Skip to content

🐛 Fix SSA merge conflict for []Arg fields by removing composite listMapKey#13340

Open
MaxRink wants to merge 2 commits intokubernetes-sigs:mainfrom
MaxRink:fix/remove-listmapkey-value-from-extraargs
Open

🐛 Fix SSA merge conflict for []Arg fields by removing composite listMapKey#13340
MaxRink wants to merge 2 commits intokubernetes-sigs:mainfrom
MaxRink:fix/remove-listmapkey-value-from-extraargs

Conversation

@MaxRink
Copy link
Contributor

@MaxRink MaxRink commented Feb 16, 2026

What this PR does / why we need it:

The []Arg fields in kubeadm_types.go (e.g. kubeletExtraArgs, extraArgs on APIServer, ControllerManager, Scheduler, LocalEtcd) had composite list map keys defined via both +listMapKey=name and +listMapKey=value. This caused server-side apply (SSA) to use the tuple (name, value) as the merge key.

When a user updates the value of an existing arg (same name, different value), SSA treats it as a new entry rather than an update, because the composite key (name, newValue) differs from the original (name, oldValue). This results in duplicate entries with the same name, which then triggers the XValidation rule "kubeletExtraArgs name must be unique" / "extraArgs name must be unique", causing the apply to be rejected.

This PR removes +listMapKey=value from all five affected []Arg fields, keeping only +listMapKey=name as the sole merge key. This ensures SSA correctly identifies entries by name alone and performs in-place value updates instead of creating duplicates.

Affected fields:

  • APIServer.ExtraArgs
  • ControllerManager.ExtraArgs
  • Scheduler.ExtraArgs
  • NodeRegistrationOptions.KubeletExtraArgs
  • LocalEtcd.ExtraArgs

Which issue(s) this PR fixes:
Fixes #13339

/area provider/control-plane-kubeadm
/kind bug

The []Arg fields (extraArgs, kubeletExtraArgs) used a composite list map
key of (name, value). This caused server-side apply to treat entries with
different values as distinct map entries. When updating the value for an
existing arg name, SSA would merge both old and new entries into the
result, creating duplicates. The XValidation uniqueness rule then
rejected the object with 'kubeletExtraArgs name must be unique'.

Fix: Remove +listMapKey=value from all []Arg fields so the list is keyed
by name only. This allows SSA to correctly identify entries by name and
replace values in-place.

Affected fields:
- APIServer.ExtraArgs
- ControllerManager.ExtraArgs
- Scheduler.ExtraArgs
- NodeRegistration.KubeletExtraArgs
- LocalEtcd.ExtraArgs
@k8s-ci-robot k8s-ci-robot added area/provider/control-plane-kubeadm Issues or PRs related to KCP kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 16, 2026
@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.

Details 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

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 16, 2026
@MaxRink
Copy link
Contributor Author

MaxRink commented Feb 16, 2026

This propably also needs a backport to 1.11 and 1.12

@sbueringer
Copy link
Member

We cannot only use name as a key because name is not unique. It is unique via CEL as long as we have to be able to roundtrip to v1beta1 but it won't be afterwards.

When a user updates the value of an existing arg (same name, different value), SSA treats it as a new entry rather than an update, because the composite key (name, newValue) differs from the original (name, oldValue).

This should not happen. Can you share more details about the case where you encountered this?

@sbueringer
Copy link
Member

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 16, 2026
@MaxRink
Copy link
Contributor Author

MaxRink commented Feb 16, 2026

Can you share more details about the case where you encountered this

Its complicated.
Basically we were still applying v1beta1 resources with flux and changed to v1beta2 via flux. In that process some field values change as our tooling also evolved and that with SSA leads to field duplication

        kubeletExtraArgs:
          cloud-provider: external
          config-dir: /var/lib/kubelet/config.d/
          container-log-max-files: "5"
          container-log-max-size: "10Mi"
          event-qps: "0"
          feature-gates: UserNamespacesSupport=true
          node-ip: "{{ ds.meta_data.local_ipv4  }}"
          protect-kernel-defaults: "true"
          tls-cipher-suites: "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_128_GCM_SHA256"

to

       kubeletExtraArgs:
         - name: cloud-provider
           value: external
         - name: config-dir
           value: /var/lib/kubelet/config.d/
         - name: container-log-max-files
           value: "5"
         - name: container-log-max-size
           value: "10Mi"
         - name: event-qps
           value: "0"
         - name: feature-gates
           value: UserNamespacesSupport=true
         - name: node-ip
           value: "{% set eth0 = ds.meta_data.network.config.ethernets.values() | selectattr(\"set-name\", \"equalto\", \"eth0\") | first %}{{ ds.meta_data.network.interfaces[\"by-mac\"][eth0.match.macaddress].ipv4[0].addr }},{{ ds.meta_data.network.interfaces[\"by-mac\"][eth0.match.macaddress].ipv6[0].addr }}"
         - name: protect-kernel-defaults
           value: "true"
         - name: tls-cipher-suites
           value: "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_256_GCM_SHA384,TLS_RSA_WITH_AES_128_GCM_SHA256"

IMO the current implementation is just wrong, as even after you would remove CEL the actual args rendered would be invalid.

@sbueringer
Copy link
Member

Let's continue on the issue

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KubeadmControlPlane kubeletExtraArgs marked invalid

3 participants