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

kubernetes-1.26: fix cloud provider condition #2876

Merged
merged 1 commit into from
Mar 10, 2023

Conversation

gthao313
Copy link
Member

@gthao313 gthao313 commented Mar 10, 2023

Issue number:

Closes #

Description of changes:

Author: Tianhao Geng <tianhg@amazon.com>
Date:   Fri Mar 10 10:09:08 2023 +0000

    kubernetes-1.26: fix cloud provider condition

    Bottlerocket settings model for cloud_provider converts the empty string to
    "\"\"", so we should improve the if condition to eq to it.

Testing done:

  • - aws-* conformance test
  • - metal-* conformance test
  • - vmware-* conformance test

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Copy link
Contributor

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

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

Overall, makes sense - I'm just abit confused what the context on this is. Also curious to hear what failures you were seein with this.

Looks like in other Kubernetes packages, we were using a default "external" block to always get that. Which is similar to what Sean suggested.

ExecStart=/usr/bin/kubelet \
{{#unless settings.kubernetes.standalone-mode}}
--cloud-provider {{default "external" settings.kubernetes.cloud-provider}} \
--kubeconfig /etc/kubernetes/kubelet/kubeconfig \
{{#if (eq settings.kubernetes.authentication-mode "tls")}}
--bootstrap-kubeconfig /etc/kubernetes/kubelet/bootstrap-kubeconfig \
{{/if}}
{{else}}
--cloud-provider "" \
{{/unless}}

Can we just do something similar to what we do in the Kubernetes 1.25 package?

@gthao313
Copy link
Member Author

Can we just do something similar to what we do in the Kubernetes 1.25 package?

@jpmcb We can't. Ideally we should migrate it to cloud_provider=external but metal-* needs empty string as value here so wen can't directly set this to external for all. If we still use {{default "external" settings.kubernetes.cloud-provider}} \ here, for aws-* variants, they default the value to aws which will break the change here.

Bottlerocket settings model for cloud_provider converts the empty string to
"\"\"", so we should improve the if condition to eq to it.
@gthao313
Copy link
Member Author

Push above improve the condition

settings model for cloud_provider converts the empty string to \"\", so we should improve it to equal to "\"\"".

// Kubelet expects the empty string to be double-quoted when be passed to `--cloud-provider`
let cloud_provider = if input.is_empty() { "\"\"" } else { input };

Test:
launched metal cluster

✅ Tinkerbell Provider setup is valid
✅ Validate certificate for registry mirror
✅ Validate authentication for git provider
✅ Create preflight validations pass
Creating new bootstrap cluster
Provider specific pre-capi-install-setup on bootstrap cluster
Installing cluster-api providers on bootstrap cluster
Provider specific post-setup
Creating new workload cluster
Installing networking on workload cluster
Creating EKS-A namespace
Installing cluster-api providers on workload cluster
Installing EKS-A secrets on workload cluster
Installing resources on management cluster
Moving cluster management from bootstrap to workload cluster
Installing EKS-A custom components (CRD and controller) on workload cluster
Installing EKS-D components on workload cluster
Creating EKS-A CRDs instances on workload cluster
Installing GitOps Toolkit on workload cluster
GitOps field not specified, bootstrap flux skipped
Writing cluster config file
Deleting bootstrap cluster
🎉 Cluster created!

@gthao313 gthao313 merged commit faa43f1 into bottlerocket-os:develop Mar 10, 2023
@gthao313 gthao313 deleted the k8s-1.26 branch March 10, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants