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

🌱 kcp: allow unsetting etcd.local, etcd.external and dns #12065

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chrischdi
Copy link
Member

@chrischdi chrischdi commented Apr 7, 2025

What this PR does / why we need it:

All fields for these structs are already mutable. This PR allows unsetting the fields too.

Context: https://kubernetes.slack.com/archives/C8TSNPY4T/p1743103086684469

Note: unsetting one of the following paths using SSA might fail with the following error, when SSA apply uses v1beta1 instead of v1beta2, client side apply works just fine:

  • .spec.kubeadmConfigSpec.clusterConfiguration.dns
  • .spec.kubeadmConfigSpec.clusterConfiguration.etcd.local

Error example from using ClusterClass and the topology controller hitting this error:

    - lastTransitionTime: "2025-04-10T12:36:00Z"
      message: 'error reconciling the Cluster topology: failed to create patch helper
        for KubeadmControlPlane default/in-memory-10996-jvvxg: server side apply dry-run
        failed for modified object: KubeadmControlPlane.controlplane.cluster.x-k8s.io
        "in-memory-10996-jvvxg" is invalid: spec.kubeadmConfigSpec.clusterConfiguration.etcd:
        Invalid value: "null": spec.kubeadmConfigSpec.clusterConfiguration.etcd in
        body must be of type object: "null"'
      observedGeneration: 4
      reason: ReconcileFailed
      status: "False"
      type: TopologyReconciled```

This is because of dns and etcd structs being optional but not a pointer and kube-apiserver falsely converting it to an invalid state (null).

Workaround: use the latest api version (here v1beta2) in clusterclass.

TODO(chrischdi): open up an issue for the above

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

/area provider/control-plane-kubeadm

@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. area/provider/control-plane-kubeadm Issues or PRs related to KCP cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 7, 2025
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 7, 2025
@chrischdi
Copy link
Member Author

Note: have to take a look at the following issue when using ClusterClass:

  1. Create Cluster with kubeadmControlPlaneTemplate:
spec:
  kubeadmConfigSpec:
    clusterConfiguration:
      ...
      etcd:
        local:
          extraArgs:
            v: "4"
...
  1. Create new KubeadmControlPlaneTemplate and refer CC to it and try to unset etcd or etcd.local:
spec:
  kubeadmConfigSpec:
    clusterConfiguration:
      ...
      etcd:
        local: {}
...

Results in the following TopologyReconciled condition:

    - lastTransitionTime: "2025-04-07T11:20:20Z"
      message: 'error reconciling the Cluster topology: failed to create patch helper
        for KubeadmControlPlane default/in-memory-21179-td88v: server side apply dry-run
        failed for modified object: KubeadmControlPlane.controlplane.cluster.x-k8s.io
        "in-memory-21179-td88v" is invalid: spec.kubeadmConfigSpec.clusterConfiguration.etcd:
        Invalid value: "null": spec.kubeadmConfigSpec.clusterConfiguration.etcd in
        body must be of type object: "null"'
      observedGeneration: 4
      reason: ReconcileFailed
      status: "False"
      type: TopologyReconciled

@chrischdi chrischdi force-pushed the pr-allow-unsetting-etcd-local-extnal-dns branch from eea6864 to ab6c287 Compare April 10, 2025 13:52
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign neolit123 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@chrischdi chrischdi changed the title [WIP] 🌱 kcp: allow unsetting etcd.local, etcd.external and dns 🌱 kcp: allow unsetting etcd.local, etcd.external and dns Apr 10, 2025
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 10, 2025
@chrischdi chrischdi force-pushed the pr-allow-unsetting-etcd-local-extnal-dns branch from ab6c287 to 43ce145 Compare April 10, 2025 15:06
@chrischdi
Copy link
Member Author

/test help

@k8s-ci-robot
Copy link
Contributor

@chrischdi: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-cluster-api-build-main
/test pull-cluster-api-e2e-blocking-main
/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-32-1-33-main
/test pull-cluster-api-test-main
/test pull-cluster-api-test-mink8s-main
/test pull-cluster-api-verify-main

The following commands are available to trigger optional jobs:

/test pull-cluster-api-apidiff-main

Use /test all to run the following jobs that were automatically triggered:

pull-cluster-api-apidiff-main
pull-cluster-api-build-main
pull-cluster-api-e2e-blocking-main
pull-cluster-api-test-main
pull-cluster-api-verify-main

In response to this:

/test help

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.

@chrischdi
Copy link
Member Author

/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-32-1-33-main

@chrischdi
Copy link
Member Author

/assign @fabriziopandini

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/provider/control-plane-kubeadm Issues or PRs related to KCP cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants