Skip to content

Convert kola test jobs to vanilla pipeline jobs#270

Merged
jlebon merged 1 commit intocoreos:masterfrom
jlebon:pr/tests-to-jobs
Oct 22, 2020
Merged

Convert kola test jobs to vanilla pipeline jobs#270
jlebon merged 1 commit intocoreos:masterfrom
jlebon:pr/tests-to-jobs

Conversation

@jlebon
Copy link
Copy Markdown
Member

@jlebon jlebon commented Aug 26, 2020

This is part of the push to move away from buildconfig-based jobs
towards the same model as what upstream CI does. For more details, see:

#228

As a first victim, convert the kola tests since they're mostly
straight-forward. A huge part of this is re-using coreos-ci-lib the same
way upstream CI does. And this helps save us a lot of boilerplate (see
diffstat for this patch).

Converting to vanilla jobs also means we can now use the build step as
intended, instead of hacking around it with oc start-build.

@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented Aug 26, 2020

Requires: coreos/coreos-ci-lib#36
Requires: coreos/coreos-ci-lib#37

@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented Aug 26, 2020

(I'll admit I didn't test this. Since it's not the main pipeline itself, was thinking of just doing fixups as needed on top.)

Comment thread jobs/kola-gcp.Jenkinsfile
@miabbott
Copy link
Copy Markdown
Member

Looks pretty straight-forward to me. 👍

Comment thread Jenkinsfile Outdated
Comment thread jobs/kola-aws.Jenkinsfile Outdated
Comment thread jobs/kola-aws.Jenkinsfile Outdated
Comment thread jobs/kola-gcp.Jenkinsfile Outdated
Comment thread jobs/kola-aws.Jenkinsfile Outdated
@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented Sep 25, 2020

Updated this now, but I'd like to sanity-check those kola run invocations locally.

Comment thread jobs/kola-aws.Jenkinsfile Outdated
Comment on lines +41 to +42
def AWS_FCOS_BUILDS_BOT_CONFIG = "/run/kubernetes/secrets/aws-fcos-builds-bot-config/config"
def AWS_FCOS_KOLA_BOT_CONFIG = "/run/kubernetes/secrets/aws-fcos-kola-bot-config/config"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm. That's not exactly what I had in mind. I think I'm bumping into a limitation in my knowledge on how things work.

I was thinking we'd be somehow re-using

- name: AWS_FCOS_BUILDS_BOT_CONFIG
value: /.aws-fcos-builds-bot-config/config

rather than having to define it here again.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer use that pod definition in those jobs. Instead, we're telling cosaPod that we need access to the specific secrets (see coreos/coreos-ci-lib#37). That way, pods only get the secrets they need mounted in.

I guess an improvement we could make here is to expose the path to the secret in environment variables as part of that wrapper.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g. in this file, notice:

cosaPod(image: params.COREOS_ASSEMBLER_IMAGE, memory: "256Mi",
        secrets: ["aws-fcos-builds-bot-config", "aws-fcos-kola-bot-config"]) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, sorry for the noise on this. I probably wouldn't have made the other comment if I had realized how it worked.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, I think a good idea came out of it regardless: coreos/coreos-ci-lib#39.

Copy link
Copy Markdown
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not an expert in this pipeline, but the high level direction looks really good to me, and superficially code looks good too.

@darkmuggle
Copy link
Copy Markdown
Contributor

LGTM....any thing hold up the merge?

@dustymabe
Copy link
Copy Markdown
Member

Maybe let's hold just a day or two til we get the next release out the door and then merge away!

This is part of the push to move away from buildconfig-based jobs
towards the same model as what upstream CI does. For more details, see:

coreos#228

As a first victim, convert the kola tests since they're mostly
straight-forward. A huge part of this is re-using coreos-ci-lib the same
way upstream CI does. And this helps save us a lot of boilerplate (see
diffstat for this patch).

Converting to vanilla jobs also means we can now use the `build` step as
intended, instead of hacking around it with `oc start-build`.
@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented Oct 22, 2020

OK, manually tested this locally now and fixed a bunch of issues. Also realized there's a gap in fcosKola we need to address first: coreos/coreos-ci-lib#41. Once that PR is merged, I think we can merge this.

Copy link
Copy Markdown
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jlebon jlebon merged commit 8c28e25 into coreos:master Oct 22, 2020
@jlebon jlebon deleted the pr/tests-to-jobs branch October 22, 2020 20:45
jlebon added a commit to jlebon/fedora-coreos-streams that referenced this pull request Nov 3, 2020
jlebon added a commit to coreos/fedora-coreos-streams that referenced this pull request Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants