Remove cloud-config and cloud-provider from 1.33 kubelet #17412
Conversation
|
Hi @Swizzmaster. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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-sigs/prow repository. |
|
/ok-to-test |
|
It seems like some of the tests are failing, please look into them and fix it. |
nodeup/pkg/model/kubelet.go
Outdated
| @@ -327,7 +327,9 @@ func (b *KubeletBuilder) buildSystemdEnvironmentFile(ctx context.Context, kubele | |||
| // We build this flag differently because it depends on CloudConfig, and to expose it directly | |||
| // would be a degree of freedom we don't have (we'd have to write the config to different files) | |||
| // We can always add this later if it is needed. | |||
| flags += " --cloud-config=" + InTreeCloudConfigFilePath | |||
| if b.IsKubernetesLT("1.33") { | |||
| flags = append(flags, fmt.Sprintf("--cloud-config=%s", InTreeCloudConfigFilePath)) | |||
There was a problem hiding this comment.
flags here is a string due to the return value from here
kops/pkg/flagbuilder/build_flags.go
Line 34 in 5a25815
You need a slice to append hence the tests are failing - https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kops/17412/pull-kops-build/1925033895468732416 .
There was a problem hiding this comment.
Yeah, I fixed that already the larger problem is this change breaks several tests.
FAIL: TestSecretBuilder (0.00s)
kubelet_test.go:380: error loading model "tests/golden/minimal": unexpected error from mockedPopulateClusterSpec: completed cluster failed validation: [spec.kubelet.cloudProvider: Forbidden: Did not match cluster cloudProvider, spec.controlPlaneKubelet.cloudProvider: Forbidden: Did not match cluster cloudProvider]
I think I know what happening. testing it now
There was a problem hiding this comment.
Ah, prolly you might have fixed few mins after comment was made ? as I was able to still comment on that code at that time.
Thanks for fixing it.
|
/ok-to-test |
|
@Swizzmaster Thank you for fixing this. Do you mind squashing the commits? |
Absolutely, squashed it down. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman 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 |
|
/retest |
Several periodic tests are failing due to the removal of the --cloud-provider flag
kubernetes/kubernetes#130161
Failing tests:
kubernetes/kubernetes#130849
Code change modeled after the removal of the flag from the api-server
1f6ea4f#diff-f81768b75ea2816be1479eac878f905b34b772f08c233164966f7fbe237d91b1