Use kubectl for kube-proxy upgrader calls #5609

g-gaston · 2023-04-12T14:12:08Z

Description of changes

Before the kube-proxy upgrader (#5345) all API calls to the management cluster came either from kubectl or clusterctl, which both happened to be run in a docker container in the admin machine. This was the first piece of code that introduced the use of Kubernetes Go client directly from the CLI binary. This means that if a user was relying on this internal implementation (explicit interface vs implicit interface), their system could break if it wasn't setup to give the CLI network connectivity to the kind cluster.

This PR "reverts" the addition of that new paradigm by changing the underlying client implementation to use kubectl commands.

Testing

Ran TestCloudStackKubernetes123ManagementClusterUpgradeFromLatest manually upgrading from v0.14.6 to make sure the kube-proxy upgrader still works as expected. (in progress)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov · 2023-04-12T14:18:28Z

Codecov Report

Merging #5609 (9ba81c7) into main (dc16138) will increase coverage by 0.08%.
The diff coverage is 87.72%.

❗ Current head 9ba81c7 differs from pull request most recent head ffc3ec0. Consider uploading reports for the commit ffc3ec0 to get more accurate results

@@            Coverage Diff             @@
##             main    #5609      +/-   ##
==========================================
+ Coverage   73.22%   73.30%   +0.08%     
==========================================
  Files         447      449       +2     
  Lines       37029    37244     +215     
==========================================
+ Hits        27113    27301     +188     
- Misses       8291     8311      +20     
- Partials     1625     1632       +7

Impacted Files	Coverage Δ
pkg/clients/kubernetes/scheme.go	`50.00% <ø> (ø)`
pkg/providers/snow/snow.go	`71.36% <75.00%> (+0.27%)`	⬆️
pkg/executables/kubectl.go	`57.20% <80.57%> (+1.48%)`	⬆️
pkg/clients/kubernetes/unauth.go	`91.17% <86.04%> (-8.83%)`	⬇️
pkg/clients/kubernetes/client.go	`100.00% <100.00%> (ø)`
pkg/clients/kubernetes/kubeconfig.go	`100.00% <100.00%> (ø)`
pkg/clients/kubernetes/kubectl.go	`100.00% <100.00%> (ø)`
pkg/clustermanager/kube_proxy.go	`79.93% <100.00%> (+0.59%)`	⬆️
pkg/controller/clientutil/kubernetes.go	`100.00% <100.00%> (ø)`
pkg/controller/clusterapi.go	`100.00% <100.00%> (ø)`
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

eks-distro-bot · 2023-04-12T17:03:17Z

@g-gaston: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
eks-anywhere-docs-presubmit	`ef77f76`	link	true	`/test eks-anywhere-docs-presubmit`
eks-anywhere-cluster-controller-tooling-presubmit	`ef77f76`	link	true	`/test eks-anywhere-cluster-controller-tooling-presubmit`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

g-gaston · 2023-04-12T17:08:51Z

/hold while we run some manual tests

pkg/clients/kubernetes/unauth.go

jiayiwang7 · 2023-04-12T21:23:59Z

pkg/clients/kubernetes/unauth.go

+	return c.kubectl.Get(ctx, resourceType, kubeconfig, list)
+}
+
+// Create saves the object obj in the Kubernetes cluster.


Suggested change

// Create saves the object obj in the Kubernetes cluster.

// Create creates the object obj in the Kubernetes cluster.

I would prefer to keep save since it avoids using the same word contained in the method name. If not, the description becomes a bit redundant.

Also:
https://github.com/kubernetes-sigs/controller-runtime/blob/main/pkg/client/interfaces.go#L64

jiayiwang7 · 2023-04-12T21:32:49Z

pkg/controller/clusterapi.go

 		return nil, err
 	}
 	return kubeadmControlPlane, nil
 }

+// CAPIKubeadmControlPlaneKey generates an ObjectKey for the CAPI Kubeadm control plane owned by


shouldn't this method belong to clusterapi package?

That's not a bad idea
fine if I take a follow up?

jiayiwang7 · 2023-04-12T22:12:00Z

pkg/executables/kubectl.go

+	return apiObj.GetName()
+}
+
+func newNotFoundErrorForTypeAndName(resourceType, name string) error {


does this work for x.y.apiversion?

yeah. In fact that's the case where it works best

Before the kube-proxy upgrader (aws#5345) all API calls to the management cluster came either from kubectl or clusterctl, which both happened to be run in a docker container in the admin machine. This was the first piece of code that introduced the use of Kubernetes Go client directly from the CLI binary. This means that if a user was relying on this internal implementation (explicit interface vs implicit interface), their system could break if it wasn't setup to give the CLI network connectivity to the kind cluster. This PR "reverts" the addition of that new paradigm byt changing the underlying client implementation to use kubectl commands.

g-gaston · 2023-04-12T22:35:06Z

/unhold

jiayiwang7 · 2023-04-12T22:37:09Z

/lgtm

g-gaston · 2023-04-12T22:37:31Z

/approve

eks-distro-bot · 2023-04-12T22:37:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: g-gaston

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~~ [g-gaston]

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

g-gaston · 2023-04-13T19:24:36Z

/cherry-pick release-0.15

eks-distro-pr-bot · 2023-04-13T19:25:30Z

@g-gaston: new pull request created: #5631

In response to this:

/cherry-pick release-0.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

eks-distro-pr-bot · 2023-04-13T19:26:07Z

@g-gaston: new pull request could not be created: failed to create pull request against aws/eks-anywhere#release-0.15 from head eks-distro-pr-bot:cherry-pick-5609-to-release-0.15: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for eks-distro-pr-bot:cherry-pick-5609-to-release-0.15."}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

In response to this:

/cherry-pick release-0.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

eks-distro-pr-bot · 2023-04-13T19:26:20Z

@g-gaston: new pull request could not be created: failed to create pull request against aws/eks-anywhere#release-0.15 from head eks-distro-pr-bot:cherry-pick-5609-to-release-0.15: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for eks-distro-pr-bot:cherry-pick-5609-to-release-0.15."}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

In response to this:

/cherry-pick release-0.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Before the kube-proxy upgrader (aws#5345) all API calls to the management cluster came either from kubectl or clusterctl, which both happened to be run in a docker container in the admin machine. This was the first piece of code that introduced the use of Kubernetes Go client directly from the CLI binary. This means that if a user was relying on this internal implementation (explicit interface vs implicit interface), their system could break if it wasn't setup to give the CLI network connectivity to the kind cluster. This PR "reverts" the addition of that new paradigm byt changing the underlying client implementation to use kubectl commands.

eks-distro-bot added do-not-merge/work-in-progress size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 12, 2023

g-gaston force-pushed the kube-proxy-to-kubectl branch from 2713785 to ef77f76 Compare April 12, 2023 17:01

github-actions bot added area/docs Documentation documentation labels Apr 12, 2023

g-gaston force-pushed the kube-proxy-to-kubectl branch from ef77f76 to a8681e0 Compare April 12, 2023 17:06

github-actions bot removed area/docs Documentation documentation labels Apr 12, 2023

g-gaston changed the title ~~[WIP] Use kubectl for kube-proxy upgrader calls~~ Use kubectl for kube-proxy upgrader calls Apr 12, 2023

eks-distro-bot removed the do-not-merge/work-in-progress label Apr 12, 2023

eks-distro-bot added the do-not-merge/hold label Apr 12, 2023

jiayiwang7 self-requested a review April 12, 2023 20:28

jiayiwang7 approved these changes Apr 12, 2023

View reviewed changes

eks-distro-bot assigned jiayiwang7 Apr 12, 2023

eks-distro-bot added the lgtm label Apr 12, 2023

jiayiwang7 reviewed Apr 12, 2023

View reviewed changes

eks-distro-bot removed the lgtm label Apr 12, 2023

g-gaston force-pushed the kube-proxy-to-kubectl branch from 9ba81c7 to e4900c6 Compare April 12, 2023 22:32

github-actions bot added area/docs Documentation documentation labels Apr 12, 2023

g-gaston force-pushed the kube-proxy-to-kubectl branch from e4900c6 to ffc3ec0 Compare April 12, 2023 22:32

github-actions bot removed documentation area/docs Documentation labels Apr 12, 2023

eks-distro-bot removed the do-not-merge/hold label Apr 12, 2023

eks-distro-bot added the lgtm label Apr 12, 2023

eks-distro-bot added the approved label Apr 12, 2023

eks-distro-bot merged commit 334b084 into aws:main Apr 12, 2023

eks-distro-pr-bot mentioned this pull request Apr 13, 2023

[release-0.15] Use kubectl for kube-proxy upgrader calls #5631

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use kubectl for kube-proxy upgrader calls #5609

Use kubectl for kube-proxy upgrader calls #5609

g-gaston commented Apr 12, 2023 •

edited by jiayiwang7

Loading

codecov bot commented Apr 12, 2023 •

edited

Loading

eks-distro-bot commented Apr 12, 2023 •

edited

Loading

g-gaston commented Apr 12, 2023

jiayiwang7 Apr 12, 2023

g-gaston Apr 12, 2023

jiayiwang7 Apr 12, 2023

g-gaston Apr 12, 2023

jiayiwang7 Apr 12, 2023

g-gaston Apr 12, 2023

g-gaston commented Apr 12, 2023

jiayiwang7 commented Apr 12, 2023

g-gaston commented Apr 12, 2023

eks-distro-bot commented Apr 12, 2023

g-gaston commented Apr 13, 2023

eks-distro-pr-bot commented Apr 13, 2023

eks-distro-pr-bot commented Apr 13, 2023

eks-distro-pr-bot commented Apr 13, 2023

	// Create saves the object obj in the Kubernetes cluster.
	// Create creates the object obj in the Kubernetes cluster.

Use kubectl for kube-proxy upgrader calls #5609

Use kubectl for kube-proxy upgrader calls #5609

Conversation

g-gaston commented Apr 12, 2023 • edited by jiayiwang7 Loading

Description of changes

Testing

codecov bot commented Apr 12, 2023 • edited Loading

Codecov Report

eks-distro-bot commented Apr 12, 2023 • edited Loading

g-gaston commented Apr 12, 2023

jiayiwang7 Apr 12, 2023

Choose a reason for hiding this comment

g-gaston Apr 12, 2023

Choose a reason for hiding this comment

jiayiwang7 Apr 12, 2023

Choose a reason for hiding this comment

g-gaston Apr 12, 2023

Choose a reason for hiding this comment

jiayiwang7 Apr 12, 2023

Choose a reason for hiding this comment

g-gaston Apr 12, 2023

Choose a reason for hiding this comment

g-gaston commented Apr 12, 2023

jiayiwang7 commented Apr 12, 2023

g-gaston commented Apr 12, 2023

eks-distro-bot commented Apr 12, 2023

g-gaston commented Apr 13, 2023

eks-distro-pr-bot commented Apr 13, 2023

eks-distro-pr-bot commented Apr 13, 2023

eks-distro-pr-bot commented Apr 13, 2023

g-gaston commented Apr 12, 2023 •

edited by jiayiwang7

Loading

codecov bot commented Apr 12, 2023 •

edited

Loading

eks-distro-bot commented Apr 12, 2023 •

edited

Loading