Adapt flags of control plane components #83

erkanerol · 2022-05-20T11:41:56Z

This PR tries to adapt the configuration of api-server, kubelet, controller-manager, etcd, scheduler and kube-proxy for CAPO to keep it consistent with other providers such as AWS and KVM.

Upgrade

While developing this PR, I noticed that changes in KubeadmConfigTemplate don't trigger any rollout for worker nodes. Then I found these:

Then added kubeAdmConfigTemplateRevision as suffix to KubeadmConfigTemplate to trigger upgrades. Unfortunately, we are not allowed to delete old templates since capi controllers cannot finish the upgrade without them. We are going to use https://github.com/giantswarm/deletion-blocker-operator to block the deletion of templates.

The invisible decisions in this PR:

api-server

PodSecurityPolicy is not added to --feature-gates. It will be handled with another PR. See Support PodSecurityPolicies in CAPO roadmap#1148
RemoveSelfLink is not added to --feature-gates.
--authorization-mode is Node,RBAC by default in CAPO. When Node is removed, pods in kube-system cannot be listed. Didn't touch this flag.
--requestheader-allowed-names was front-proxy-client before this PR but it is updated as in the PR.
--tls-cipher-suites is updated as it is in https://github.com/giantswarm/giantnetes-terraform/pull/585/files
--anonymous-auth is false in vintage clusters but setting it false breaks kubeadm and nodes cannot join the cluster in CAPI clusters.

etcd

Flags of etcd in CAPO are almost the same as in other environments.
The only diff is --snapshot-count=10000. Didn't touch that flag.

kubelet

Eviction policies are copied from vintage clusters's kubelet configuration.
Some flags are not set explicitly in this PR and in our current kubelet configuration but they are overridden by the configuration file /var/lib/kubelet/config.yaml in our image.
- --anonymous-auth is already true by default but I saw it is set as false in /var/lib/kubelet/config.yaml in our image. It is better to set it explicitly.
- --cgroup-driver is systemd in /var/lib/kubelet/config.yaml in our image whereas it is cgroupfs by default in kubelet. Didn't touch it.
- --resolv-conf is /run/systemd/resolve/resolv.conf in /var/lib/kubelet/config.yaml whereas it is /etc/resolv.conf by default. Didn't touch it.
kernelMemcgNotification is true in vintage AWS but it is going to be deleted in 1.24. Therefore, didn't add to this PR.
I found these configuration in vintage AWS but I don't see any reason to set them for CAPO by default:
- serializeImagePulls=false
- registryBurst=3
- registryPullQPS=2
- maxPods=32

controller-manager

CAPO has startupProbe. Others don't. Didn't change.
AWS has a memory request. Others don't. Didn't add.
--controllers is *,bootstrapsigner,tokencleaner for CAPO. My understanding is that those are necessary for kubeadm. https://kubernetes.io/docs/reference/access-authn-authz/bootstrap-tokens/
AWS has --terminated-pod-gc-threshold=10 flag but didn't see any particular reason to decrease this value for CAPO.
CAPO has --port=0. It is OK. See kubeadm: add --port=0 for kube-controller-manager and kube-scheduler kubernetes/kubernetes#92720
AWS has --v=2 but I think it is OK to go with the default value.

scheduler

Scheduler pod in AWS has a memory request. The one in KVM doesn't. Now, we don't have it for OpenStack as well. It is the single diff in scheduler configuration.

kube-proxy

Everything is the same.

Testing

Fresh install works.
Upgrade from previous version works.

Checklist

Update changelog in CHANGELOG.md.
Make sure values.yaml and values.schema.json are valid.

tityosbot · 2022-05-20T11:41:58Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

helm/cluster-openstack/files/kubelet-args

helm/cluster-openstack/templates/kubeadm_control_plane.yaml

pipo02mix

I asked for more config options

bavarianbidi · 2022-06-01T14:21:59Z

helm/cluster-openstack/templates/kubeadm_config_template.yaml

@@ -3,9 +3,11 @@
 apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
 kind: KubeadmConfigTemplate
 metadata:
+  finalizers:
+  - cluster-api-cleaner-openstack.finalizers.giantswarm.io


is the future idea that cluster-api-cleaner-openstack removes the finalizer once the kubeadmconfigtemplate isn't needed anymore?

Q1: should we rethink the scope of cluster-api-cleaner-openstack sometime again?

A helper operator for CAPO to delete resources created by apps in workload clusters.

Q2: does it make sense to let cluster-api-cleaner-openstack set the finalizer as well? Most controllers i've seen so far are adding/removing the finalizers by their own.

Will this be needed in other providers as well? Would it make sense to solve it for all instead of using cluster-api-cleaner-openstack?

erkanerol · 2022-06-03T07:27:41Z

In the kaas-sync yesterday, we decided to implement a CAPI generic operator for this issue. I am going to implement it and then merge this PR.

pipo02mix

LGMT when Jose is happy

fiunchinho · 2022-06-03T10:05:08Z

I believe PodSecurityPolicies are missing from the feature flags.

erkanerol · 2022-06-03T12:03:10Z

I believe PodSecurityPolicies are missing from the feature flags.

Yeah. I mentioned it in the PR description.

PodSecurityPolicy is not added to --feature-gates. It will be handled with another PR. See giantswarm/roadmap#1148

fiunchinho · 2022-06-03T12:34:01Z

I believe PodSecurityPolicies are missing from the feature flags.

Yeah. I mentioned it in the PR description.

PodSecurityPolicy is not added to --feature-gates. It will be handled with another PR. See giantswarm/roadmap#1148

Sorry, I missed that.

kopiczko · 2022-06-13T11:08:33Z

helm/cluster-openstack/templates/_helpers.tpl

@@ -63,6 +63,10 @@ room for such suffix.
  sudo: ALL=(ALL) NOPASSWD:ALL
 {{- end -}}

+{{- define "kubeletExtraArgs" -}}
+{{- .Files.Get "files/kubelet-args" -}}


Why not inline?

Isn't it better to have those flags in a separate valid yaml file instead of embedding a shitty templating code?

We will use https://github.com/giantswarm/deletion-blocker-operator to block deletion of templates. Necessary finalizers will be added by the operator during runtime. We don't need this hack here anymore.

erkanerol force-pushed the apiserver-flags branch 2 times, most recently from 9258122 to 56c9eed Compare May 25, 2022 09:52

erkanerol changed the title ~~Adapt apiserver and kubelet flags~~ Adapt flags of control plane components May 25, 2022

erkanerol force-pushed the apiserver-flags branch from 56c9eed to 57c1725 Compare May 30, 2022 07:15

erkanerol marked this pull request as ready for review May 30, 2022 12:09

erkanerol requested a review from a team as a code owner May 30, 2022 12:09

bavarianbidi approved these changes May 31, 2022

View reviewed changes