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

Remove --release-kind and --federation from push-build.sh #19321

Merged
merged 1 commit into from
Sep 24, 2020
Merged

Remove --release-kind and --federation from push-build.sh #19321

merged 1 commit into from
Sep 24, 2020

Conversation

saschagrunert
Copy link
Member

@saschagrunert saschagrunert commented Sep 23, 2020 •
edited
Loading

The kubernetes_build scenario was referencing two more or less
deprecated flags --release-kind and --federation. Since the scripts
to execute those build does not exist any more inside k/k we can safely
remove them.

/assign @dims @justaugustus
/cc @kubernetes/release-managers

related to #19328

@saschagrunert
Remove --release-kind and --federation from push-build.sh
7ca00cb 
The kubernetes_build scenario was referencing two more or less
deprecated flags `--release-kind` and `--federation`. Since the scripts
to execute those build does not exist any more inside k/k we can safely
remove them.

Signed-off-by: Sascha Grunert <[email protected]>
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/scenarios sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Sep 23, 2020
@dims
Copy link
Member

dims commented Sep 23, 2020

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 23, 2020
@saschagrunert
Copy link
Member Author

/assign @spiffxp

@justaugustus
Copy link
Member

/lgtm

spiffxp
spiffxp reviewed Sep 23, 2020
View reviewed changes
Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

/hold
I'm OK with this in principle but I'm wary of creating more dead code. Is there an issue or PR covering its removal?

scenarios/kubernetes_build.py
if args.federation:
# TODO: do we need to set these?
env['PROJECT'] = args.federation
env['FEDERATION_PUSH_REPO_BASE'] = 'gcr.io/%s' % args.federation
Copy link
Member

Choose a reason for hiding this comment

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

https://cs.k8s.io/?q=FEDERATION_PUSH_REPO_BASE&i=nope&files=&repos= - this shows up in kubernetes/release/push-build.sh, is there a PR to remove this?

Copy link
Member Author

@saschagrunert saschagrunert Sep 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Not right now. I was working on finishing krel push as replacement for push-build.sh (see kubernetes/release#1459), which is now finished. The new implementation does not have both mentioned flags.

I can't remove the script until we did not move all tests using this scenario to kubetest. :-/

scenarios/kubernetes_build.py
'--federation',
help='Push federation images to the specified project')
PARSER.add_argument(
'--release-kind',
Copy link
Member

Choose a reason for hiding this comment

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

https://cs.k8s.io/?q=release_kind&i=nope&files=&repos= - same deal here

@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 Sep 23, 2020
spiffxp
spiffxp approved these changes Sep 24, 2020
View reviewed changes
Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

/hold cancel
/approve
Not a fan of dead code but I guess I'm fine pruning the surface area of this scenario (we should be striving to eliminate scenarios anyhow)

@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 Sep 24, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, saschagrunert, spiffxp

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 24, 2020
@k8s-ci-robot k8s-ci-robot merged commit 7fcf543 into kubernetes:master Sep 24, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.20 milestone Sep 24, 2020
@saschagrunert saschagrunert deleted the federation-rm branch September 24, 2020 17:31
@justaugustus
Copy link
Member

/sig release
/area release-eng

@k8s-ci-robot k8s-ci-robot added sig/release Categorizes an issue or PR as relevant to SIG Release. area/release-eng Issues or PRs related to the Release Engineering subproject labels Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spiffxp spiffxp spiffxp approved these changes

Assignees

@dims dims

@spiffxp spiffxp

@justaugustus justaugustus

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 area/scenarios cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.20
Development

Successfully merging this pull request may close these issues.
5 participants
@saschagrunert @dims @justaugustus @k8s-ci-robot @spiffxp