*: setup AWS kola job & move aws replication to release job #128

arithx · 2019-08-26T15:58:00Z

Creates a new job specifically for running the kola tests on AWS. As a
result moves the AWS replication out of the main Jenkinsfile into the
Jenkinsfile.release.

We'll likely have to move the AWS replication to it's own job down the
line if we want to start signing the meta.json / release.json files.

cgwalters

Overall LGTM, though I think templating is going to be very important for making it easy to hack on the pipeline, as well as for supporting branches.

cgwalters · 2019-08-26T23:40:14Z

Jenkinsfile

+            stage('Kola Runs') {
+                container('jnlp') {
+                    utils.shwrap("""
+                        oc start-build --wait fedora-coreos-pipeline-kola-aws \


Let's template the name of the buildconfig to aid forking and branching?

The job itself (similar to the release job) is only meant to have one of it existing and developer pipelines can just re-use it. Did you have something else in mind?

cgwalters · 2019-08-26T23:42:13Z

Jenkinsfile.kola.aws

+
+        stage('AWS Kola Run') {
+          utils.shwrap("""
+          kola run -p aws --aws-ami ${ami} --aws-region ${ami_region} -b fcos || :


Also an interesting topic here - should we e.g. store the kola results in the builddir too?

That might be something that could become useful in the future but for now I don't think it hurts to just store them in jenkins.

jlebon · 2019-09-05T19:52:09Z

Jenkinsfile.release

+            s3_stream_dir = "${s3_bucket}/prod/streams/${params.STREAM}"
+            utils.shwrap("""
+            coreos-assembler buildprep s3://${s3_stream_dir}/builds
+            coreos-assembler aws-replicate --build=${params.VERSION}


This is OK, though it's making the assumption that the VERSION is the latest one. Which to be fair is likely almost always the case, though it'd be nice to have something a bit stronger here. Perhaps something like buildprep --build=${params.VERSION} and then buildupload --build=${params.VERSION}?

@jlebon: buildprep doesn't seem to accept a specific build as a parameter just the URL; does upstream work there need to happen first?

Right, yeah. Was just spitballing a new UX in those cosa commands. Definitely doesn't need to block this.

jlebon · 2019-09-05T19:54:02Z

Jenkinsfile.release

+            coreos-assembler buildprep s3://${s3_stream_dir}/builds
+            coreos-assembler aws-replicate --build=${params.VERSION}
+            git clone https://github.com/coreos/fedora-coreos-releng-automation /var/tmp/fcos-releng
+            /var/tmp/fcos-releng/coreos-meta-translator/trans.py --workdir .


Ahh yeah, hmm we should probably just bake that into the cosa container, or have a sidecar container in the pod with utilities we need?

Yeah, it doesn't fit in cosa directly so I didn't have a good idea for where to fit it. Maybe just abstracting it into a generic function?

We can punt on this for now. Once scripts from that repo are used some more, we can think about how to avoid git cloneing all over the place.

jlebon · 2019-09-05T19:57:12Z

Jenkinsfile.kola.aws

+properties([
+    pipelineTriggers([]),
+    parameters([
+      string(name: 'BUILDID',


Let's call this VERSION for consistency? (Even though they're not exactly the same thing, but for our use case they are).

jlebon · 2019-09-05T19:59:39Z

Jenkinsfile.kola.aws

+             description: 'Fedora CoreOS Build ID to test',
+             defaultValue: '',
+             trim: true),
+      string(name: 'S3_STREAM_DIR',


Why not choice(STREAM) to be consistent?

Because this is a generic job it doesn't have a developer split so it cannot construct the S3_STREAM_DIR, as such my intention was to be able to still support running AWS tests in developer pipelines.

jlebon · 2019-09-05T21:28:16Z

Jenkinsfile.kola.aws

+podTemplate(cloud: 'openshift', label: 'coreos-assembler', yaml: pod, defaultContainer: 'jnlp') {
+    node('coreos-assembler') { container('coreos-assembler') {
+        def ami, ami_region
+        stage('Fetching Build Details') {


How about just "Fetch Metadata" ?

jlebon · 2019-09-05T21:29:44Z

Jenkinsfile.kola.aws

+            if (meta.amis.size() > 0) {
+                ami = meta['amis'][0]['hvm']
+                ami_region = meta['amis'][0]['name']
+            }


Should there be an early exit or maybe better error out if there are no AMIs in the metadata?

jlebon · 2019-09-05T21:30:20Z

Jenkinsfile.kola.aws

+
+        stage('AWS Kola Run') {
+          utils.shwrap("""
+          kola run -p aws --aws-ami ${ami} --aws-region ${ami_region} -b fcos || :


Don't we want some --parallel value here?

Probably but the value itself highly depends on the account it's being ran in. I don't remember the exact numbers on the default resource limits so we probably want to keep it low, maybe 5 tests at a time which shouldn't ever hit more than 15 concurrent instances?

Ahh gotcha. Hmm, I guess this gets us back to https://github.com/coreos/mantle/issues/1037#issuecomment-517299593.

Yeah, let's go with 10 for now and go from there? The Fedora testing account at least doesn't barf on that, and e.g. my personal account doesn't either. :)

jlebon · 2019-09-05T21:36:47Z

Jenkinsfile

+            stage('Kola Runs') {
+                container('jnlp') {
+                    utils.shwrap("""
+                        oc start-build --wait fedora-coreos-pipeline-kola-aws \


Suggestion: let's not --wait for this? Essentially, consider kola AWS testing as a follow-up job, not something that's necessarily part of the build process itself.

hmm interesting - i would have figured we'd wanted it that way.

One of the reasons why I suggested this is that kola AWS tests can take a while, and there's no reason to delay building other things while that's ongoing (though related: #68). I also think errorring the build job if the AWS test fails isn't practical.

jlebon · 2019-09-05T21:38:01Z

Jenkinsfile.release

@@ -30,6 +30,23 @@ pod = pod.replace("COREOS_ASSEMBLER_IMAGE", "coreos-assembler:master")

 podTemplate(cloud: 'openshift', label: 'coreos-assembler', yaml: pod, defaultContainer: 'jnlp') {
    node('coreos-assembler') { container('coreos-assembler') {
+        // Replicate the newly uploaded AMI to other regions. Intentionally


We should then remove the aws-replicate bit from the Jenkinsfile, no?

Ah yeah, I haven't refreshed this PR in a while so I think that commit got lost.

arithx · 2019-09-09T18:29:15Z

Rebased & updated to support comments. Note that this shouldn't be merged until the upstream account has the credentials needed for replication added.

dustymabe · 2019-09-09T18:48:04Z

Rebased & updated to support comments. Note that this shouldn't be merged until the upstream account has the credentials needed for replication added.

👍 - i'm looking at permissions now for the aws replication issue, so can consider this too.

dustymabe

LGTM

dustymabe · 2019-09-11T02:35:07Z

I tested out #130 extensively today, which builds on top of this PR. We should be able to merge them both around the same time and update creds in prod to finally have aws replication and aws kola tests enabled.

jlebon · 2019-09-11T15:55:31Z

Jenkinsfile

@@ -298,6 +286,19 @@ podTemplate(cloud: 'openshift', label: 'coreos-assembler', yaml: pod, defaultCon
            }
        }

+
+        if (!params.MINIMAL) {


Should also be conditional on s3_stream_dir

jlebon · 2019-09-11T15:56:14Z

Jenkinsfile.kola.aws

+                ami_region = meta['amis'][0]['name']
+            } else {
+                // no AMI found
+                return


IIRC return inside a stage doesn't actually do anything. Also, thinking more on this now, I think we should just error out? It'd be pretty easy to mistake a job ending successfully because of tests passing vs actually a no-op.

Creates a new job specifically for running the kola tests on AWS. As a result moves the AWS replication out of the main `Jenkinsfile` into the `Jenkinsfile.release`. We'll likely have to move the AWS replication to it's own job down the line if we want to start signing the `meta.json` / `release.json` files.

arithx · 2019-09-11T17:30:15Z

Update pushed addressing comments ⬆️

jlebon

LGTM! Will let you or @dustymabe time the final merge.

cgwalters reviewed Aug 26, 2019

View reviewed changes

jlebon reviewed Sep 5, 2019

View reviewed changes

arithx force-pushed the replicate_and_test branch from 26ad4a3 to bfb040f Compare September 9, 2019 18:27

dustymabe mentioned this pull request Sep 11, 2019

Enable running kola AWS tests using separate credentials #130

Merged

dustymabe approved these changes Sep 11, 2019

View reviewed changes

jlebon reviewed Sep 11, 2019

View reviewed changes

arithx force-pushed the replicate_and_test branch from bfb040f to 2ab8fac Compare September 11, 2019 17:29

jlebon approved these changes Sep 11, 2019

View reviewed changes

dustymabe merged commit d59f42d into coreos:master Sep 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: setup AWS kola job & move aws replication to release job #128

*: setup AWS kola job & move aws replication to release job #128

arithx commented Aug 26, 2019

cgwalters left a comment

cgwalters Aug 26, 2019

arithx Aug 27, 2019

cgwalters Aug 26, 2019

arithx Aug 27, 2019

jlebon Sep 5, 2019

arithx Sep 5, 2019

arithx Sep 6, 2019

jlebon Sep 6, 2019

jlebon Sep 5, 2019

arithx Sep 5, 2019

jlebon Sep 6, 2019

jlebon Sep 5, 2019

jlebon Sep 5, 2019

arithx Sep 5, 2019

jlebon Sep 5, 2019

jlebon Sep 5, 2019

jlebon Sep 5, 2019

arithx Sep 5, 2019

jlebon Sep 6, 2019 •

edited

Loading

jlebon Sep 5, 2019

dustymabe Sep 10, 2019

jlebon Sep 11, 2019

jlebon Sep 5, 2019

arithx Sep 5, 2019

arithx commented Sep 9, 2019

dustymabe commented Sep 9, 2019

dustymabe left a comment

dustymabe commented Sep 11, 2019

jlebon Sep 11, 2019

jlebon Sep 11, 2019

arithx commented Sep 11, 2019

jlebon left a comment

*: setup AWS kola job & move aws replication to release job #128

*: setup AWS kola job & move aws replication to release job #128

Conversation

arithx commented Aug 26, 2019

cgwalters left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlebon Sep 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arithx commented Sep 9, 2019

dustymabe commented Sep 9, 2019

dustymabe left a comment

Choose a reason for hiding this comment

dustymabe commented Sep 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arithx commented Sep 11, 2019

jlebon left a comment

Choose a reason for hiding this comment

jlebon Sep 6, 2019 •

edited

Loading