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

Declare explicit roles in procedures #378

Merged
merged 1 commit into from
Jun 23, 2021

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Jun 15, 2021

In Foreman Release there were no explicit roles so those were added.

It also aligns Foreman and Katello naming for the Release Engineer role.

This implements point 1 and 3 as laid out in the communication RFC. The second point is something humans should adopt.

Also includes an unrelated fix to use the tarballs_release script.

Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

Comments/questions, but full ack on the explicit documentation of the ownership.

Aligns well with the changes @pcreech @zjhuntin and @Odilhao been planning, where we assign an rel-eng person to the release, instead of switching if the release happens to span multiple sprints.


* Release Owner: <%= owner %>
* Release Engineer: <%= engineer %>
* Installer Maintainer: @
Copy link
Member

Choose a reason for hiding this comment

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

Is this intentionally left empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it was sort of a one-off and I wasn't entirely sure if we should add it to the branch command.

# Manual updates: <%= target_date %>

## Release Engineer
Copy link
Member

Choose a reason for hiding this comment

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

Should these headers repeat the username?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting idea.

- [ ] Update release version similar to [here](https://github.com/theforeman/theforeman-rel-eng/commit/2029a9688da00d9c385c3438dd71b594ba5f728e)
- [ ] Run the Jenkins [Tarballs Release](https://ci.theforeman.org/job/tarballs-release/) to create tarballs using [release_tarballs](https://github.com/theforeman/theforeman-rel-eng/blob/master/release_tarballs)
Copy link
Member

Choose a reason for hiding this comment

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

Why moving this off to rel-eng? This step happens directly after tagging, and could be already finished the moment rel-eng sees the ping if started by rel-owner (as it is right now).

No hard blocker, more curious why :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I sort of assumed it was done by rel-eng. Also there's a script in theforeman-rel-eng for it.

Copy link
Member

Choose a reason for hiding this comment

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

So far it was always @tbrisker who kicked that off. No idea if by script or not.

Copy link
Member

Choose a reason for hiding this comment

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

I manually trigger it when I'm done tagging. I guess I see it as the last step in getting to "Here are the artifacts you need, now build them into packages please", but it could also be done by rel-eng just as well when pinged.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps we should also make sure the release owner can use the rel-eng scripts to make this easier? There is also tag_project which we today document as separate steps for example.

My idea is that we can further smooth the process: rather than long instructions just point to scripts. At that point, it matters less who does it.

Another idea: verifying if Jenkins jobs are green can also be done in a script and the exit code can indicate if they are.

In practice what you can already do is:

./release_tarballs && ./download_tarballs && ./inspect_tarballs

I also think we should also qualify the inspect step. What I always manually did was verify the version is correct in VERSION. Perhaps we can automate that too using tar and grep. It could also be a place to prevent supply chain attacks (if Jenkins would be compromised) but I'm not sure how we would achieve that.

However, maybe we're getting a bit off-topic here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh and to be clear, the real reason I moved it down was to put it below the updating of theforeman-rel-eng to be able to use the script and I considered updating that part of the release engineer's role.

Copy link
Member

Choose a reason for hiding this comment

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

In general I'm all for automating as much as possible. I think the key points that need a human to do something (be it run a script or trigger a job) are:

  • The release is tagged and ready for building, start the train
  • Verify that this is the real and correct files/code/package/whatever and sign everything. Ideally this could also be automated but if we're thinking of supply chain security we probably can't trust jenkins to verify the code is correct or hold the keys to signing, that would have to be done on some separate, hopefully secure system.

Anything else could potentially be done by a machine, especially if all the human right now does is run a script. Let me know if i'm missing some step that is crucial to have human involvement in.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've removed this change and limited it to just adding roles. I can open a separate PR for more scripting.

In Foreman Release there were no explicit roles so those were added.

It also aligns Foreman and Katello naming for the Release Engineer role.

This implements point 1 and 3 as laid out in the communication RFC
https://community.theforeman.org/t/branching-release-process-communication/23965
The second point is something humans should adopt.
Copy link
Member

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

Thanks @ekohl !

@tbrisker tbrisker merged commit d116109 into theforeman:master Jun 23, 2021
@ekohl ekohl deleted the update-procedures branch June 23, 2021 08:50
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.

3 participants