-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
docs: update release.md with improvements from v3.4.34 release #18600
docs: update release.md with improvements from v3.4.34 release #18600
Conversation
Signed-off-by: Ivan Valdes <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted filessee 22 files with indirect coverage changes @@ Coverage Diff @@
## main #18600 +/- ##
==========================================
- Coverage 68.83% 68.82% -0.01%
==========================================
Files 420 420
Lines 35474 35474
==========================================
- Hits 24418 24415 -3
+ Misses 9636 9635 -1
- Partials 1420 1424 +4 Continue to review full report in Codecov by Sentry.
|
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.
LGTM - Thanks @ivanvc
7. Publish the release page on GitHub | ||
- Set the release title as the version name | ||
- Choose the correct release tag (generated from step #4) | ||
- Follow the format of previous release pages | ||
- Attach the generated binaries and signature file | ||
- Verify the historical binary size for each architecture. If there's a big difference, verify that it works for that architecture |
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.
I think longer term we could have a workflow trigger either automatically or manually during a release window which performs artifact validations.
For now let's just add this note about manual verification though.
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.
I would love to run e2e tests on the generated artifacts.
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.
I completely agree. This is a short-term solution while we implement better automation.
/retest |
@@ -78,11 +78,14 @@ which don't need to be executed before releasing each version. | |||
|
|||
It generates all release binaries under the directory `/tmp/etcd-release-${VERSION}/etcd/release/` and images. Binaries are pushed to the Google Cloud bucket | |||
under project `etcd-development`, and images are pushed to `quay.io` and `gcr.io`. | |||
|
|||
**Remove the `quay.io` login entry from `~/.docker/config.json` after pushing Docker images.** |
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.
Didn't see what's the current way of procuring quay password. Let's make sure we persist it somewhere before we instruct everyone to delete. As detailed in https://contribute.cncf.io/resources/project-services/faq/#how-do-i-share-credentials-passwords-or-other-confidential-information there are free plans from password managers for open source projects. Would be great to adopt them.
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.
@serathius, I think the issue is that no current maintainer can access the quay.io admin account. We've been using a shared password that @jmhbnz provides before the release, but we don't have a way to restrict the user's permissions, so the current safest option is to delete the login from the Docker config file.
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.
Putting the change in this pr to the side I agree as a project we should implement some project level password vault so we are not reliant on credentials being shared directly by maintainers as that assumes maintainers are always available when needed and always remember / have their own system to store the passwords.
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.
Looks like 1Password’s free open source plan is a good choice to proceed.
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.
Raised #18611
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.
LGTM
We can resolve the followups separately.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahrtr, ivanvc, jmhbnz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This pull request has the improvements noted while doing #18486.
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.