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

krel: make promote-images work for other k8s and k8s sigs projects #2280

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

CecileRobertMichon
Copy link
Member

@CecileRobertMichon CecileRobertMichon commented Oct 5, 2021

What type of PR is this?

/kind feature

What this PR does / why we need it:

Makes krel promote-images work with other projects so it can be used by other maintainers to promote images in k8s/k8s.io.

The default values are set to the Kubernetes project to preserve existing behavior.

Example usage:

krel promote-images --project cluster-api-azure --tag v0.5.4 --reviewers "@nader-ziada @devigned" --fork CecileRobertMichon

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

krel: make promote-images work for other k8s and k8s sigs projects

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. 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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 5, 2021
@k8s-ci-robot k8s-ci-robot added the sig/release Categorizes an issue or PR as relevant to SIG Release. label Oct 5, 2021
@CecileRobertMichon
Copy link
Member Author

/cc @puerco

trying to find a good way to test this. Pushed a fake tag but currently, it's failing with FATA Growing manifest with tag v0.5.4: no images survived filtering; double-check your --filter_* flag(s) for typos

@CecileRobertMichon
Copy link
Member Author

Pushed a fake tag but currently, it's failing with FATA Growing manifest with tag v0.5.4: no images survived filtering; double-check your --filter_* flag(s) for typos

fixed this issue

but now I'm getting

FATA pushing userfork to CecileRobertMichon/k8s.io: command /usr/local/bin/git push --set-upstream userfork cluster-api-azure-v0.5.4-image-promotion did not succeed: [email protected]: Permission denied (publickey).

checking if it's an issue with my github token...

@CecileRobertMichon
Copy link
Member Author

IT WORKS 🎉

kubernetes/k8s.io#2882

I actually had an issue with my github ssh key, fixed it by following https://docs.github.com/en/authentication/troubleshooting-ssh/error-permission-denied-publickey

@CecileRobertMichon
Copy link
Member Author

CecileRobertMichon commented Oct 5, 2021

Few things remaining:

  1. need to fix the integration test
  2. could we somehow sort the images by tag instead of by SHA (maybe as an option)? IMO that's more human friendly... @puerco thoughts?
  3. should the k8s.io PR not have the releng prefix when the PR isn't for kubernetes? does the release team use that for triage?

@CecileRobertMichon
Copy link
Member Author

/retest

not sure if it was a flake?

time="2021-10-05T19:08:38Z" level=info msg="Hit the rate limit on try 1, sleeping for 49m58.453160242s" err="GET https://api.github.com/repos/kubernetes/kubernetes/tags?page=10&per_page=50: 403 API rate limit of 60 still exceeded until 2021-10-05 19:58:37 +0000 UTC, not making remote request. [rate reset in 49m58s]"

@saschagrunert
Copy link
Member

not sure if it was a flake?

Yeah, that's a flake :/

Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

Code lgtm

/hold

@CecileRobertMichon please remove the WIP when you're ready

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 6, 2021
@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 6, 2021
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

/lgtm
thanks Cecile!

@CecileRobertMichon
Copy link
Member Author

@saschagrunert @cpanato thanks for the review! @justaugustus pointed out in Slack that we should put this in https://github.com/kubernetes-sigs/promo-tools instead. Do we want to merge this PR first and then add the functionality to promo-tools or add it there directly? Seems like promo-tools doesn't have functionality for opening an image promotion PR currently so might be a bigger chunk of work.

@saschagrunert
Copy link
Member

Seems like promo-tools doesn't have functionality for opening an image promotion PR currently so might be a bigger chunk of work.

If that's the case then moving forward iteratively seems fine to me.

@justaugustus
Copy link
Member

Seems like promo-tools doesn't have functionality for opening an image promotion PR currently so might be a bigger chunk of work.

If that's the case then moving forward iteratively seems fine to me.

Agreed!
Thanks for this awesome improvement, Cecile!
/lgtm
/approve
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2021
@justaugustus justaugustus changed the title [WIP] krel: make promote-images work for other k8s and k8s sigs projects krel: make promote-images work for other k8s and k8s sigs projects Oct 7, 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 Oct 7, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, cpanato, justaugustus, saschagrunert

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:
  • OWNERS [cpanato,justaugustus,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. needs-priority 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/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants