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

Add IgnorePreflightErrors to KubeadmConfig types #4581

Closed
sbueringer opened this issue May 6, 2021 · 5 comments · Fixed by #4905
Closed

Add IgnorePreflightErrors to KubeadmConfig types #4581

sbueringer opened this issue May 6, 2021 · 5 comments · Fixed by #4905
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Milestone

Comments

@sbueringer
Copy link
Member

What steps did you take and what happened:

Based on Slack thread: https://kubernetes.slack.com/archives/C8TSNPY4T/p1619121490147500

We saw that there were some fields missing which are in upstream kubeadm v1beta2. At a first glance we're missing:

  • CertificateKey
  • IgnorePreflightErrors

The theory in Slack was that these fields were added after we forked the types

What did you expect to happen:
I would have expected to be able to confgure (at least) IgnorePreflightErrors.

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

  • Cluster-api version:
  • Minikube/KIND version:
  • Kubernetes version: (use kubectl version):
  • OS (e.g. from /etc/os-release):

/kind bug
[One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels]

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label May 6, 2021
@sbueringer sbueringer changed the title Missing kubeadm fields Missing kubeadm type fields May 6, 2021
@fabriziopandini
Copy link
Member

Hopefully I can help with some context here:

  1. Current version of Cluster API KubeadmConfig is derived from kubeadm API v1 beta1
  2. Starting from v1alpha4, Cluster API KubeadmConfig types are decoupled from a specific version of the kubeadm API
  3. Building on top of 2, it is now possible to implement some changes/cleanups in KubeadmConfig to improve usability

Accordingly

  • IgnorePreflightErrors does not exists yet because this field was not present in v1beta1; I'm +1 to get this added, but we should be aware that this field will be no-op on older Kubernetes (this is not a problem IMO)
  • CertificateKey was not present in v1beta1 too, however I don't think that we should add it to KubeadmConfig because we are not using automatic copy of certificates in Cluster API.

@sbueringer
Copy link
Member Author

Makes sense to me. Is there already an issue open for v1alpha4 changes to the KubeadmConfig types or do we want to keep the current issue open to add the IgnorePreflightErrors field?

@fabriziopandini
Copy link
Member

AFAIK there is no issue. It is ok to use this one for now.

@vincepri
Copy link
Member

vincepri commented Jul 6, 2021

/retitle Add IgnorePreflightErrors to KubeadmConfig types
/assign @sbueringer
/milestone v0.4
/priority important-longterm

We should add validation that this field should only be used with Kubernetes v1.22+

@k8s-ci-robot k8s-ci-robot changed the title Missing kubeadm type fields Add IgnorePreflightErrors to KubeadmConfig types Jul 6, 2021
@k8s-ci-robot k8s-ci-robot added this to the v0.4 milestone Jul 6, 2021
@k8s-ci-robot k8s-ci-robot added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Jul 6, 2021
@fabriziopandini
Copy link
Member

NOTE: Validation can be tricky for Machine deployments because the Kubernetes version to use might be on a different resource (while on KCP instead everything is on the same resource)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants