Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lower FCOS S3 bucket prefix into config.yaml #737

Merged
merged 3 commits into from
Nov 4, 2022

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Nov 3, 2022

The prod/streams/${params.STREAM} is an FCOS thing. Let's have the s3_bucket key in the config.yaml also incorporate the subpath.

This will allow the RHCOS config to specify its own subpath and is prep for hotfix builds specifying a different subpath.

Copy link
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.

I wonder if there is a better way to achieve the goal here. To me if we change the top level s3_bucket to have a bucket+path then I'd expect the pipeline to not touch anything outside of that path in the bucket, which clearly isn't true (see sync-stream-metadata and also maybe the plume call in release).

Maybe what we should do here instead is separate things out a bit more:

s3:
  bucket: fcos-builds
  builds_path: prod/streams/${STREAM}
  release_metadata_path: releases

Obviously the stuff that happens in sync-stream-metadata is FCOS only which is why we're ok breaking the cleanliness there, but it might be nice to have the relationship be clearer.

config.yaml Show resolved Hide resolved
jobs/release.Jenkinsfile Outdated Show resolved Hide resolved
@jlebon
Copy link
Member Author

jlebon commented Nov 4, 2022

I wonder if there is a better way to achieve the goal here. To me if we change the top level s3_bucket to have a bucket+path then I'd expect the pipeline to not touch anything outside of that path in the bucket, which clearly isn't true (see sync-stream-metadata and also maybe the plume call in release).

Maybe what we should do here instead is separate things out a bit more:

s3:
  bucket: fcos-builds
  builds_path: prod/streams/${STREAM}
  release_metadata_path: releases

Obviously the stuff that happens in sync-stream-metadata is FCOS only which is why we're ok breaking the cleanliness there, but it might be nice to have the relationship be clearer.

Yeah, I pretty much optimized it for the jobs which are in common between FCOS and RHCOS. It didn't seem valuable to try to extend the config schema right now for prod FCOS-specific things.

Re. builds_path, I debated indeed whether to have it split from the bucket or unified. I ended up with the latter because that's how most jobs consume it anyway, but I think splitting now with an eye towards expanding in the future makes sense. Will tweak.

utils.groovy Outdated Show resolved Hide resolved
Copy link
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 - one requested comment update

jobs/release.Jenkinsfile Outdated Show resolved Hide resolved
@jlebon jlebon force-pushed the s3-bucket-prefix branch 2 times, most recently from acb4862 to 74dd4e8 Compare November 4, 2022 18:13
The `prod/streams/${params.STREAM}` is an FCOS thing. Let's have the
`s3_bucket` key in the `config.yaml` also incorporate the subpath.

This will allow the RHCOS config to specify its own subpath and is prep
for hotfix builds specifying a different subpath.
This parameter dates from the very start of kola jobs. At that time, we
didn't pass in the stream itself as a separate parameter, so we passed
the whole S3 stream dir instead.

Nowadays, we do pass in the stream, and all kola jobs correctly query
the pipecfg for that info. So clean things up by deleting the
unnecessary parameter.
In the release job, we may not be releasing the latest build but our use
of `cosa meta` assumed so. Pass `--build` to be more strict.

We currently only call `run_cloud_tests()` from contexts where it's
reasonable to expect the target version is the latest build, but as it's
structured as a standalone function, also pass `--build` there to be
safe.
@jlebon jlebon marked this pull request as ready for review November 4, 2022 18:47
Copy link
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 6ea6506 into coreos:main Nov 4, 2022
@jlebon jlebon deleted the s3-bucket-prefix branch November 4, 2022 19:06
@jlebon jlebon restored the s3-bucket-prefix branch November 4, 2022 19:15
@jlebon jlebon deleted the s3-bucket-prefix branch April 24, 2023 01:41
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.

None yet

2 participants