graduate kubeadm patch support to GA

[neolit123]

some projects in the ecosystem are moving away from kustomize:

• kind (completed)
• cluster-api (maybe?)
• kustomize being part of kubectl, while kubectl is still in k/k has caused issues:
  WIP: Upgrade kubectl to sigs.k8s.io/kustomize/[email protected] kubernetes#85146 (comment)

i personally question some of the approaches the project has taken WRT support of API types.

i think a goal for us should be to continue supporting the same mechanic for user "customization" in kubeadm, but possibly change the backend.

---

• update 18.05.2020:
  a proposal for using patches is tracked as alpha for 1.19:
  https://github.com/kubernetes/enhancements/tree/master/keps/sig-cluster-lifecycle/kubeadm/1739-customization-with-patches
  kubeadm: customization with patches enhancements#1739

• update 07.10.2022
  latest status about tracking
  graduate kubeadm patch support to GA #2046 (comment)

---

TODO:

• support multiple API versions - e.g. kubelet v1beta1 / v1.

ALPHA (1.19)

• k/k PR: kubeadm: introduce --experimental-patches and deprecate --experimental-kustomize kubernetes#92017
  • marks --experimental-kustomize as deprecated
  • adds --experimental-patches
• e2e tests:
  • test-infra: kubeadm: add e2e test jobs for the --experimental-patches feature test-infra#17934
  • k/kubeadm: kinder: support and test the --experimental-patches flag for kubeadm #2184

---

1.20:

• mark the kustomize KEP as "replaced":
  keps/sig-cluster-lifecycle/kubeadm: mark the kustomization KEP as replaced enhancements#1991
• remove --experimental-kustomize from kubeadm:
  kubeadm: remove the --experimental-kustomize feature kubernetes#94871
• remove --exprimental-kustomize e2e tests from:
  • k/kubeadm/kinder/ci
    kinder: remove -k / -kustomize-dir support and related test jobs #2295
  • test-infra
    sig-cluster-lifecycle: remove kustomize related test jobs test-infra#19299
• PSA to the ML:
  https://groups.google.com/g/kubernetes-sig-cluster-lifecycle/c/bv8ZbpmO5Q8

---

1.22

• graduate the functionality:
  • add --patches
  • mark --experimental-patches as deprecated
  • add Init|JoinConfiguration.patches.directory to v1beta3
    kubeadm: add support for patches in v1beta3; deprecate --experimental-patches kubernetes#103063
• add k/website docs about this at https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/control-plane-flags/

---

1.23

• update patches KEP (retroactively):
  kubeadm: customization with patches enhancements#1739
  https://github.com/kubernetes/enhancements/tree/master/keps/sig-cluster-lifecycle/kubeadm/1739-customization-with-patches
• adapt kinder / kubeadm e2e to use --config for "init|join" and --patches for "upgrade"
  https://github.com/kubernetes/kubeadm/tree/master/kinder/pkg/kubeadm
• remove --experimental-patches & disallow mixture of --patches and --config for "init/join"?
  kubeadm: disallow the mixture of --config and --patches & remove deprecated --experimental-patches kubernetes#104065

---

/kind design
/priority important-longterm

[timothysc]

I'm almost thinking to have a plug-able template approach might be the best option, but we would need to spec this out.

/cc @frapposelli

[BenTheElder]

I think @neolit123 is already quite familiar with this but for others I want to add a little context regarding kind's move FWIW:

kind moved off of kustomize both to make it cheaper for projects like cluster-api to embed kind (need to follow up on that) ... it seriously dropped the binary weight of kind (down to around 9MB total) and avoids issues like the "k8sdep" packages that have the kubernetes API types at some version ...

We hope that it will be easier to embed in test-runners, kinder, etc. as the API improves and the dependencies are kept minimal. kustomize is significantly larger and more dependency heavy.

The deps are probably reasonable enough for the kustomize primary use case (kubernetes API object patching...), but since we were / are only patching objects for which kustomize is NOT API aware I.E. just plain merge patch and 6902 patch on kubeadm types, we could just use the underlying patch libraries with a pretty small custom package to implement patch <-> target matching etc.

This also enabled us to add TOML patching which has proven handy (for containerd config) and doesn't make sense for tools like kustomize.

We also wanted to enable looser matching requirements, given that kubeadm's API types are nearly the same across some versions. We support matching on only the kind field between a patch and a target object if apiVersion is unset in the patch. That way you can write trivial patches that target multiple versions instead of repeating the same patch for every version.

All of those points may or may or may not make sense in other contexts such as kubeadm.

I think the semantics are not quite complete yet (in particular kubernetes-sigs/kind#1332) but it could make sense to export for re-use if others are interested in the same behavior. My main concern is that other users would probably want to drag back in the k8s runtime schemes, and we explicitly do not want kubernetes/kubernetes as a kind binary dependency.

https://github.com/kubernetes-sigs/kind/tree/268d5057533333c6f84434a29f1824d44f621b04/pkg/cluster/internal/patch

---

i personally question some of the approaches the project has taken WRT support of API types.

... +1. this was one of the biggest blockers for us. The API also just is much more complicated than the naive merge-patch / 6902 patch runtime we needed.

[neolit123]

@pacoxu
looks like the recent changes broke the patches e2e test jobs for 1.22 and latest:
https://testgrid.k8s.io/sig-cluster-lifecycle-kubeadm#kubeadm-kinder-patches-latest
https://testgrid.k8s.io/sig-cluster-lifecycle-kubeadm#kubeadm-kinder-patches-1-22

both show errors such as:

ERROR: /etc/kubernetes/manifests/etcd.yaml is not patched
ERROR: /etc/kubernetes/manifests/kube-apiserver.yaml is not patched
ERROR: /etc/kubernetes/manifests/kube-controller-manager.yaml is not patched
ERROR: /etc/kubernetes/manifests/kube-scheduler.yaml is not patched

this means the patches were not applied.

looking at the output of kubeadm init here:
https://storage.googleapis.com/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-kinder-patches-latest/1425504851701272576/build-log.txt

i don't see the patches field present in the config:

time="17:16:13" level=debug msg="generated config:\napiServer:\n  certSANs:\n  - localhost\n  - 172.17.0.2\napiVersion: kubeadm.k8s.io/v1beta3\nclusterName: kinder-patches\ncontrolPlaneEndpoint: 172.17.0.5:6443\ncontrollerManager:\n  extraArgs:\n    enable-hostpath-provisioner: \"true\"\nkind: ClusterConfiguration\nkubernetesVersion: v1.23.0-alpha.0.408+53d24cd0b4fc45\nmetadata:\n  name: config\nnetworking:\n  podSubnet: 192.168.0.0/16\n  serviceSubnet: \"\"\nscheduler:\n  extraArgs: null\n---\napiVersion: kubeadm.k8s.io/v1beta3\nbootstrapTokens:\n- token: abcdef.0123456789abcdef\nkind: InitConfiguration\nlocalAPIEndpoint:\n  advertiseAddress: 172.17.0.2\n  bindPort: 6443\nmetadata:\n  name: config\nnodeRegistration:\n  criSocket: /var/run/dockershim.sock\n  kubeletExtraArgs:\n    node-ip: 172.17.0.2\n---\napiVersion: kubelet.config.k8s.io/v1beta1\ncgroupDriver: systemd\nevictionHard:\n  imagefs.available: 0%\n  nodefs.available: 0%\n  nodefs.inodesFree: 0%\nfailSwapOn: false\nimageGCHighThresholdPercent: 100\nkind: KubeletConfiguration\nmetadata:\n  name: config\n---\napiVersion: kubeproxy.config.k8s.io/v1alpha1\nkind: KubeProxyConfiguration\nmetadata:\n  name: config\n"

this means our kinder logic for adding the "patches patch" is not working - e.g. this part:
https://github.com/kubernetes/kubeadm/pull/2537/files#diff-062b4ae3122d567dd3ac6e475bdf4b16739710f584ef380706728745fae94c75R279-R285

i think the problem is here:
https://github.com/kubernetes/kubeadm/pull/2537/files#diff-733eb3862c9c82106d657cb00e06246a0881a564637307ed45dd2329b078ade5R113-R119

during "init" the function must return an InitConfiguration patch.
and during "join" a JoinConfiguration patch.
currently it always returns a JoinConfiguration patch.

i think we should also move the function to a new file e.g. componentpatches.go because "discovery" is only a concept during join, while patches also work on init and upgrade.

there could be other problems lurking too...

[pacoxu]

let me check

[pacoxu]

I tried to fix it in #2545.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[neolit123]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[pacoxu]

There seem to be no bugs or further feature requests for patch support now.

[neolit123]

we discussed with Fabrizio to tie the graduation to the kubeadm api which is still beta. i think we can keep this issue open in case new features are requested for the patches.

[chendave]

we discussed with Fabrizio to tie the graduation to the kubeadm api which is still beta. i think we can keep this issue open in case new features are requested for the patches.

this doesn't seems like a candidate for v1beta4, right? if not, I will remove this from the v1bata4 candidate list. @pacoxu @neolit123

[neolit123]

we discussed with Fabrizio to tie the graduation to the kubeadm api which is still beta. i think we can keep this issue open in case new features are requested for the patches.

this doesn't seems like a candidate for v1beta4, right? if not, I will remove this from the v1bata4 candidate list. @pacoxu @neolit123

yes

[neolit123]

this has been a working feature in the kubeadm API and the CLI for a number of releases.
"graduation to GA" at this point seems artificial.
the feature is stable...
the API field can graduate to GA once the API graduates to GA.

i think we can keep this issue open in case new features are requested for the patches.

it seems better to open new issues for such change requests.