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.

Sign up for GitHub

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

Jump to bottom

kpromo: Add krel promote-images functionality #458

Merged
merged 5 commits into from
Nov 18, 2021
Merged

kpromo: Add krel promote-images functionality #458

merged 5 commits into from
Nov 18, 2021

Conversation

justaugustus
Copy link
Contributor

@justaugustus justaugustus commented Nov 17, 2021 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

From https://github.com/kubernetes/release/tree/8742f143657ad3be35ad6c044e74e87f91cf39dc.

Slack convo: https://kubernetes.slack.com/archives/C2C40FMNF/p1636407926086000?thread_ts=1635789741.040200&cid=C2C40FMNF

  • kpromo: Add krel promote-images functionality
  • kpromo(pr): Remove dependency on k8s.io/release
  • kpromo(pr): Enable as subcommand of kpromo
  • kpromo(pr): Move image promoter constructs to separate package
  • kpromo: Build v3.3.0-beta.2-1 image

Signed-off-by: Stephen Augustus [email protected]
Co-authored-by: Adolfo García Veytia (Puerco) [email protected]
Co-authored-by: Sascha Grunert [email protected]
Co-authored-by: Carlos Panato [email protected]
Co-authored-by: Cecile Robert-Michon [email protected]
Co-authored-by: Nabarun Pal [email protected]

FYI to the previous contributors of this code: @puerco @saschagrunert @cpanato @CecileRobertMichon @palnabarun

Which issue(s) this PR fixes:

Special notes for your reviewer:

Still fixing this up locally, since it was more involved than I initially calculated, so expect test failures.

Does this PR introduce a user-facing change?

- kpromo: Add krel promote-images functionality
- kpromo(pr): Remove dependency on k8s.io/release
- kpromo(pr): Enable as subcommand of `kpromo`
- kpromo(pr): Move image promoter constructs to separate package
- kpromo: Build v3.3.0-beta.2-1 image

@justaugustus @puerco
@saschagrunert @cpanato @palnabarun
kpromo: Add krel promote-images functionality
8b4a0fb 
From k/release at 8742f143657ad3be35ad6c044e74e87f91cf39dc.

Signed-off-by: Stephen Augustus <[email protected]>
Co-authored-by: Adolfo García Veytia (Puerco) <[email protected]>
Co-authored-by: Sascha Grunert <[email protected]>
Co-authored-by: Carlos Panato <[email protected]>
Co-authored-by: Cecile Robert-Michon <[email protected]>
Co-authored-by: Nabarun Pal <[email protected]>
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 17, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justaugustus

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the area/artifacts Issues or PRs related to the hosting of release artifacts for subprojects label Nov 17, 2021
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/release-eng Issues or PRs related to the Release Engineering subproject approved Indicates a PR has been approved by an approver from all required OWNERS files. sig/release Categorizes an issue or PR as relevant to SIG Release. labels Nov 17, 2021
justaugustus
justaugustus commented Nov 17, 2021
View reviewed changes
cmd/kpromo/cmd/pr/pr.go
@@ -0,0 +1,358 @@
/*
Copyright 2020 The Kubernetes Authors.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copyright date was purposely left intact as 2020.

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 17, 2021
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 18, 2021
@justaugustus
kpromo: Build v3.3.0-beta.2-1 image
5eb42fc 
Signed-off-by: Stephen Augustus <[email protected]>
@justaugustus justaugustus changed the title [WIP] kpromo: Add krel promote-images functionality kpromo: Add krel promote-images functionality Nov 18, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 18, 2021
@justaugustus
Copy link
Contributor Author

There are some other bits I can think of fixing (like merging the functionality of and deprecating cip-mm), but I don't want to boil the ocean...

So, ready for review:
/assign @puerco @Verolop
cc: @kubernetes-sigs/release-engineering

puerco
puerco reviewed Nov 18, 2021
View reviewed changes
cmd/kpromo/cmd/pr/pr.go

prBody := fmt.Sprintf("Image promotion for %s %s\n", opts.project, strings.Join(opts.tags, " / "))
prBody += "This is an automated PR generated from `kpromo`\n"
prBody += fmt.Sprintf("```\nkpromo pr %s\n```\n\n", args)
Copy link
Member

Choose a reason for hiding this comment

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

Nice detail capturing the command here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice detail capturing the command here

Heh, I can't take credit!
That was @palnabarun in kubernetes/release#2320 and @CecileRobertMichon in kubernetes/release#2280. :)

@justaugustus justaugustus mentioned this pull request Nov 18, 2021
Merged
CecileRobertMichon
CecileRobertMichon reviewed Nov 18, 2021
View reviewed changes
cmd/kpromo/cmd/pr/pr.go
return errors.Wrap(err, "adding image manifest to staging area")
}

commitMessage := "releng: Image promotion for " + opts.project + " " + strings.Join(opts.tags, " / ")
Copy link

Choose a reason for hiding this comment

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

small nit I had from using this recently: do you think we could get rid of the releng: prefix? or maybe just add it when it's for the Kubernetes project? Not sure if it's relevant for other projects

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mind if I fix in a follow-up?
There are more things I'd like to fix about the message, but they're a little out-of-scope for this PR.

Copy link

Choose a reason for hiding this comment

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

sounds good 👌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

small nit I had from using this recently: do you think we could get rid of the releng: prefix? or maybe just add it when it's for the Kubernetes project? Not sure if it's relevant for other projects

Will be fixed in #460.

@CecileRobertMichon
Copy link

lgtm

for the non-Kubernetes project use case

@justaugustus justaugustus mentioned this pull request Nov 18, 2021
Merged
@puerco
Copy link
Member

puerco commented Nov 18, 2021

Awesome, let the promo-tools hackathon begin !
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2021
@justaugustus justaugustus mentioned this pull request Nov 18, 2021
Merged
@k8s-ci-robot k8s-ci-robot merged commit 5599f36 into kubernetes-sigs:main Nov 18, 2021
@justaugustus justaugustus mentioned this pull request Nov 18, 2021
Closed
This was referenced Nov 18, 2021
Merged
Merged
@justaugustus justaugustus mentioned this pull request Dec 13, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CecileRobertMichon CecileRobertMichon CecileRobertMichon left review comments

@puerco puerco puerco left review comments

@Verolop Verolop Awaiting requested review from Verolop

Assignees

@Verolop Verolop

@puerco puerco

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/artifacts Issues or PRs related to the hosting of release artifacts for subprojects area/release-eng Issues or PRs related to the Release Engineering subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/release Categorizes an issue or PR as relevant to SIG Release. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@justaugustus @k8s-ci-robot @CecileRobertMichon @puerco @Verolop