- 
                Notifications
    You must be signed in to change notification settings 
- Fork 62
fcos-release-notes: add job for generating release notes #261
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
base: main
Are you sure you want to change the base?
Conversation
Adds a job for generating `release-notes.yaml` from `release-notes.d/` in `testing-devel` and `next-devel` and puts the output `release-notes.yaml` file to `testing` and `next` branch correspondingly. Also adds the functionality in `Jenkinsfile` such that after building the image, replaces the premature `next-release` placeholder key with the real build id. Then it uploads the final `release-notes.yaml` to the S3 bucket as well as pushing to `fedora-coreos-config` repo. Related: coreos/fedora-coreos-tracker#194 Signed-off-by: Allen Bai <[email protected]>
| ./ s3://fcos-builds | ||
| """) | ||
| } | ||
|  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something in my mind keeps telling me this is not correct / desired. Since Jenkinsfile primarily used only used cosa, and not sure if I could use botCreds and aws-fcos-builds-bot here. The logic comes from bump-lockfile.Jenkinsfile and sync-stream-metadata.Jenkinsfile.
Could you sanity check please? @jlebon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still trying to understand how everything fits together (so the next statements might not fit with the intended design), but ISTM like we shouldn't do anything related to release notes as part of the main build pipeline. That should instead be in the release job (or a job triggered by the release job), and only on production streams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Discussed at #261 (comment).
| The PR makes it possible to: 
 | 
| shwrap("mkdir ${branch}") | ||
|  | ||
| dir(branch) { | ||
| shwrap("cosa init --branch ${branch} https://github.com/${config_repo}") | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to use cosa here? Should we just use git clone --depth=1 instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All I needed was a FCOS config repo so no hard need for a cosa init.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's just use git clone --depth=1 to keep things simple then.
| shwrap(""" | ||
| cd ${branch}/src/config | ||
| git checkout ${branch} | ||
| rm -rf release-notes.d/*.yaml | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably need to use git rm here and git commit -m below.
| // should gracefully handle race conditions here | ||
| sh("git push https://\${GHUSER}:\${GHTOKEN}@github.com/${config_repo} ${branch}") | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doing a git pull --rebase before doing the git push might help with race conditions.
| In the most recent design proposal we have (snippet): Proposal:Have a tool that runs independently of the pipeline like  In this PR the "tool" is  If  | 
| For  Git repository manipulation
 It seems like we can do these three bullet points: in a single stage and not worry about breaking it up further. From those bullets it seems like there should be 2  
 We really are just primarily operating on the  | 
| 
 @dustymabe Yes, the only portion that I depend on  
 I thought we mainly operate on  
 it seems like  | 
| 
 👍 
 The  Though, now that I think about it, we might need to leave the  
 Right. Basically  | 
| 
 That makes a lot of sense. I was concerning about  
 I thought twice about the process, and it makes sense now. Thanks for explaining! | 
| From coreos/fedora-coreos-config#550 (comment) I've reorganized the logic to reflect the workflow without a  Note: all of the  | 
| 
 Looks good. 
 Looks good 
 I'm a bit torn on the "we no longer need to aggregate yaml snippets in stable" bit. I think it might more simple to just generate them the same way as we do for  Though the more I think about it, we often do multiple  
 I think there are a few things that are important to call out to see if we are on the same page. 
 Does that match with what you are thinking? | 
Adds a job for generating
release-notes.yamlfromrelease-notes.d/intesting-develandnext-develand puts the outputrelease-notes.yamlfile totestingandnextbranch correspondingly.Also adds the functionality in
Jenkinsfilesuch that after building the image, replaces the prematurenext-releaseplaceholder key with the real build id. Then it uploads the finalrelease-notes.yamlto the S3 bucket as well as pushing tofedora-coreos-configrepo.Related: coreos/fedora-coreos-tracker#194
Signed-off-by: Allen Bai [email protected]