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 --ssh- options, deprecated 13 releases, that only work on GCE #102297

Merged
merged 3 commits into from Jun 5, 2021
Merged

remove --ssh- options, deprecated 13 releases, that only work on GCE #102297

merged 3 commits into from Jun 5, 2021

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented May 25, 2021

Removing --ssh-user and --ssh-key options that have been deprecated for 13 releases (since 1.9). They only ever functioned on GCE and now only function when running the in-tree cloud provider instead of the out of the out of tree provider.

The apiserver proxy feature was introduced in 1.16 and went to beta in 1.18, so a replacement has been available for some time.

I bumped into this while trying to fix an integration test that needed fine control of some versioning options and I wasn't able to unwind it in part because of the construction of the tunneler.

PR opened to see if trip dependencies in CI have been built on this.

/kind cleanup
/priority important-soon
@kubernetes/sig-api-machinery-api-reviews

--ssh-user and --ssh-key options are removed.  They only functioned on GCE, and only in-tree.  Use the apiserver network proxy instead.

@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. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels May 25, 2021
@liggitt
Copy link
Member

liggitt commented May 25, 2021

just to make sure all network proxy paths remain functional as this is dropped

/test pull-kubernetes-e2e-gce-network-proxy-grpc

@lavalamp
Copy link
Member

/assign @caesarxuchao

@fedebongio
Copy link
Contributor

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels May 25, 2021
@fedebongio
Copy link
Contributor

yay!

@cheftako cheftako mentioned this pull request Jun 1, 2021
Merged
@cheftako
Copy link
Member

cheftako commented Jun 3, 2021

/lgtm

https://testgrid.k8s.io/sig-api-machinery-network-proxy#ci-kubernetes-e2e-gci-gce-network-proxy-grpc&width=25 (the ssh tunnel replacement looks green) and #102510 has it being tested. I think we are good to go. Thanks for getting the removal handled.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 3, 2021
@deads2k
Copy link
Contributor Author

deads2k commented Jun 3, 2021

/test all

just ensuring we're still green.

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 3, 2021
@deads2k deads2k changed the title [wip] remove --ssh- options, deprecated 13 releases, that only work on GCE remove --ssh- options, deprecated 13 releases, that only work on GCE Jun 3, 2021
@k8s-ci-robot k8s-ci-robot added area/provider/gcp Issues or PRs related to gcp provider sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 3, 2021
@deads2k
Copy link
Contributor Author

deads2k commented Jun 3, 2021

/retest

@liggitt
Copy link
Member

liggitt commented Jun 3, 2021

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, liggitt

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 Jun 3, 2021
@fejta-bot
Copy link

This PR may require API review.

If so, when the changes are ready, complete the pre-review checklist and request an API review.

Status of requested reviews is tracked in the API Review project.

cheftako
cheftako reviewed Jun 3, 2021
View reviewed changes
cluster/gce/gci/configure-kubeapiserver.sh Outdated
fi
params+=" --advertise-address=${vm_external_ip}"
Copy link
Member

Choose a reason for hiding this comment

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

Sorry to nit but can we move this up to be just after where we set the vm_external_ip variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry to nit but can we move this up to be just after where we set the vm_external_ip variable?

sure, done.

@cheftako
Copy link
Member

cheftako commented Jun 3, 2021

/retest

cheftako
cheftako reviewed Jun 4, 2021
View reviewed changes
cmd/kube-apiserver/app/server.go
@@ -55,7 +54,6 @@ import (
clientgoclientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/util/keyutil"
cloudprovider "k8s.io/cloud-provider"
Copy link
Member

Choose a reason for hiding this comment

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

Woohoo!

@cheftako
Copy link
Member

cheftako commented Jun 4, 2021

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 4, 2021
@liggitt
Copy link
Member

liggitt commented Jun 4, 2021

/retest

@andrewsykim
Copy link
Member

Really great to see this happen, thanks for all the effort folks!

@k8s-ci-robot k8s-ci-robot merged commit 74af3b7 into kubernetes:master Jun 5, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.22 milestone Jun 5, 2021
@dims
Copy link
Member

dims commented Jun 5, 2021

DGEoPl3

@cheftako
Copy link
Member

cheftako commented Jun 7, 2021

😝

@cheftako
Copy link
Member

cheftako commented Jun 7, 2021

Congrats! Thanks everyone for making this happen!

@matthyx matthyx mentioned this pull request Jun 24, 2021
Closed
@liggitt liggitt mentioned this pull request Aug 18, 2021
Closed
@dims dims mentioned this pull request Dec 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cheftako cheftako cheftako left review comments

@liggitt liggitt liggitt left review comments

@logicalhan logicalhan Awaiting requested review from logicalhan

@SataQiu SataQiu Awaiting requested review from SataQiu

Assignees

@cheftako cheftako

@caesarxuchao caesarxuchao

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/apiserver area/provider/gcp Issues or PRs related to gcp provider area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.22
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@deads2k @liggitt @lavalamp @fedebongio @cheftako @k8s-ci-robot @fejta-bot @andrewsykim @dims @caesarxuchao