✨ Generate the manifests by calling the kubeadm #216

charleszheng44 · 2021-09-14T16:33:20Z

What this PR does / why we need it:
Generate the ncp manifests using the kubeadm executable

k8s-ci-robot · 2021-09-14T16:33:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: charleszheng44

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [charleszheng44]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

christopherhein

This is amazing so far, 1 comment for now and I want to pul this down and experiment with it tomorrow. 👍

christopherhein · 2021-09-16T00:27:25Z

Dockerfile

 ENTRYPOINT [ "/start.sh", "/workspace/manager" ]

 # Use distroless as minimal base image to package the manager binary
 # Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM gcr.io/distroless/static:nonroot
+# FROM gcr.io/distroless/static:nonroot
+FROM ubuntu


If we need to switch this can we use the debian from the distroless repo - https://github.com/GoogleContainerTools/distroless#docker

jzhoucliqr · 2021-10-05T16:24:16Z

controlplane/nested/controllers/controller_util.go

+		case kubeadm.APIServer:
+			ret[kubeadm.APIServer] = completeKASPodSpec(pod, clusterName)
+		case kubeadm.ControllerManager:
+			ret[kubeadm.APIServer] = completeKCMPodSpec(pod, clusterName)


Suggested change

ret[kubeadm.APIServer] = completeKCMPodSpec(pod, clusterName)

ret[kubeadm. ControllerManager] = completeKCMPodSpec(pod, clusterName)

jzhoucliqr · 2021-10-05T16:24:37Z

controlplane/nested/controllers/controller_util.go

+		case kubeadm.ControllerManager:
+			ret[kubeadm.APIServer] = completeKCMPodSpec(pod, clusterName)
+		case kubeadm.Etcd:
+			ret[kubeadm.APIServer] = completeEtcdPodSpec(pod, clusterName)


Suggested change

ret[kubeadm.APIServer] = completeEtcdPodSpec(pod, clusterName)

ret[kubeadm. Etcd] = completeEtcdPodSpec(pod, clusterName)

jzhoucliqr · 2021-10-05T16:51:49Z

Thanks @charleszheng44 for the effort! I'm trying out this PR, however, the etcd statefulset DNS resolve have some issue so it doesn't come up, not sure if you've seen this?

{"level":"warn","ts":1633452597.3438876,"caller":"netutil/netutil.go:131","msg":"failed to resolve URL Host; returning","url":"https://cluster-s1-etcd-0.cluster-s1-etcd.default.svc:2380","host":"cluster-s1-etcd-0.cluster-s1-etcd.default.svc:2380","retry-interval":1,"error":"lookup cluster-s1-etcd-0.cluster-s1-etcd.default.svc on 10.96.0.10:53: no such host"}

charleszheng44 · 2021-10-05T16:56:49Z

Thanks @charleszheng44 for the effort! I'm trying out this PR, however, the etcd statefulset DNS resolve have some issue so it doesn't come up, not sure if you've seen this?

{"level":"warn","ts":1633452597.3438876,"caller":"netutil/netutil.go:131","msg":"failed to resolve URL Host; returning","url":"https://cluster-s1-etcd-0.cluster-s1-etcd.default.svc:2380","host":"cluster-s1-etcd-0.cluster-s1-etcd.default.svc:2380","retry-interval":1,"error":"lookup cluster-s1-etcd-0.cluster-s1-etcd.default.svc on 10.96.0.10:53: no such host"}

@jzhoucliqr Thanks for finding the bug. I am working on it and will let you know once it is fixed 😃

jzhoucliqr · 2021-10-05T18:19:10Z

I think the issue is because in statefulset we use component name as service name, which ends with -nestedetcd, but the actual service object name ends with -etcd.
https://github.com/kubernetes-sigs/cluster-api-provider-nested/pull/216/files#diff-d3d34848bfdf7f6567e793bc968606d1628a0c36a69f141ecb7bf09e90c439c3R164

jzhoucliqr · 2021-10-05T18:37:38Z

controlplane/nested/kubeadm/consts.go

+
+const (
+	KubeadmExecPath          = "/kubeadm"
+	KASManifestsPath         = "/etc/kubernetes/manifests/kube-apiserver.yaml"


By using the same default file locations, could it cause conflict, for eg, when multiple clusters are created at the same time?

Yes. That could be a problem. kubeadm does not support generating static pod manifests in a directory other than /etc/kubernetes/manifests. We should figure out a way to solve it.

@christopherhein @Fei-Guo do you have any suggestions?

Can we span a new pod for each cluster and generate the manifests in the new pod then terminate after they have been created as configmaps?

Can we span a new pod for each cluster and generate the manifests in the new pod then terminate after they have been created as configmaps?

@christopherhein @jzhoucliqr

Yes. That's also the only way I can see. Let's try to merge this PR first and I will create a new issue/PR to fix it?

I think that might work the only idea I could have is tracking what cluster is currently being templated and "lock" other reconciliation from processing until the current cluster bring up finishes.

Then we need to add a big lock in the reconcile loop or reduce the ncp_controller's concurrency to one to ensure the ncp_controller is not creating more than one nested control plane at a time. Will that affect the performance?

A big lock might be a good short term solution and proposing a path override option to the kubeadm folks for a longer term solution?

as mentioned to @charleszheng44 on slack, the kubeadm commands used to generate the manifests accept --rootfs, so you can pass e.g. ./ to generate the manifest tree under ./. this requires running kubeadm as root there obviously.
i don't see kubeadm allowing a configurable /etc/kubernetes/manifests any time soon.

charleszheng44 · 2021-10-05T19:04:18Z

I think the issue is because in statefulset we use component name as service name, which ends with -nestedetcd, but the actual service object name ends with -etcd. https://github.com/kubernetes-sigs/cluster-api-provider-nested/pull/216/files#diff-d3d34848bfdf7f6567e793bc968606d1628a0c36a69f141ecb7bf09e90c439c3R164

That's right 😅 Let me try it out. Thanks

charleszheng44 · 2021-10-05T23:43:28Z

@jzhoucliqr It should work now. Would you try it out?

jzhoucliqr · 2021-10-07T05:20:57Z

@jzhoucliqr It should work now. Would you try it out?

Thanks. Will try it out.

jzhoucliqr · 2021-10-07T17:56:58Z

@jzhoucliqr It should work now. Would you try it out?

Thanks. Will try it out.

Changes work fine. Thanks @charleszheng44

christopherhein · 2021-11-30T18:44:21Z

/lgtm

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 14, 2021

k8s-ci-robot requested review from resouer and vincepri September 14, 2021 16:33

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 14, 2021

charleszheng44 force-pushed the feat/gen-manifests-using-kubeadm branch from 2949cf0 to 07575c2 Compare September 14, 2021 16:34

charleszheng44 changed the title ~~✨ Generate the manifests by calling the kubeadm~~ ✨ [WIP] Generate the manifests by calling the kubeadm Sep 14, 2021

christopherhein reviewed Sep 16, 2021

View reviewed changes

christopherhein mentioned this pull request Sep 16, 2021

🐛 update component path to /home/nonroot #204

Closed

charleszheng44 force-pushed the feat/gen-manifests-using-kubeadm branch from 07575c2 to 66c731a Compare September 21, 2021 04:03

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 21, 2021

update the crd

864d0db

charleszheng44 force-pushed the feat/gen-manifests-using-kubeadm branch from 66c731a to 966bca1 Compare September 25, 2021 19:58

jzhoucliqr reviewed Oct 5, 2021

View reviewed changes

charleszheng44 force-pushed the feat/gen-manifests-using-kubeadm branch from 966bca1 to 19f4262 Compare October 5, 2021 16:28

jzhoucliqr reviewed Oct 5, 2021

View reviewed changes

charleszheng44 force-pushed the feat/gen-manifests-using-kubeadm branch from 19f4262 to 9f0f5b7 Compare October 5, 2021 23:41

charleszheng44 force-pushed the feat/gen-manifests-using-kubeadm branch 3 times, most recently from 1e9600e to da2067d Compare October 6, 2021 19:05

charleszheng44 changed the title ~~✨ [WIP] Generate the manifests by calling the kubeadm~~ ✨ Generate the manifests by calling the kubeadm Oct 6, 2021

charleszheng44 force-pushed the feat/gen-manifests-using-kubeadm branch from e71f308 to 21e37b5 Compare October 19, 2021 18:19

generate the manifests by calling the kubeadm

00d69ef

charleszheng44 force-pushed the feat/gen-manifests-using-kubeadm branch from 21e37b5 to 00d69ef Compare October 19, 2021 18:41

k8s-ci-robot assigned christopherhein Nov 30, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 30, 2021

k8s-ci-robot merged commit 5339aab into kubernetes-retired:main Nov 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Generate the manifests by calling the kubeadm #216

✨ Generate the manifests by calling the kubeadm #216

charleszheng44 commented Sep 14, 2021

k8s-ci-robot commented Sep 14, 2021

christopherhein left a comment

christopherhein Sep 16, 2021

jzhoucliqr Oct 5, 2021

jzhoucliqr Oct 5, 2021

jzhoucliqr commented Oct 5, 2021

charleszheng44 commented Oct 5, 2021

jzhoucliqr commented Oct 5, 2021

jzhoucliqr Oct 5, 2021

charleszheng44 Oct 5, 2021

charleszheng44 Oct 5, 2021

christopherhein Oct 5, 2021

charleszheng44 Oct 6, 2021

christopherhein Oct 6, 2021

charleszheng44 Oct 6, 2021

kfox1111 Oct 7, 2021

neolit123 Oct 7, 2021

charleszheng44 commented Oct 5, 2021

charleszheng44 commented Oct 5, 2021

jzhoucliqr commented Oct 7, 2021

jzhoucliqr commented Oct 7, 2021

christopherhein commented Nov 30, 2021

	ret[kubeadm.APIServer] = completeKCMPodSpec(pod, clusterName)
	ret[kubeadm. ControllerManager] = completeKCMPodSpec(pod, clusterName)

	ret[kubeadm.APIServer] = completeEtcdPodSpec(pod, clusterName)
	ret[kubeadm. Etcd] = completeEtcdPodSpec(pod, clusterName)

✨ Generate the manifests by calling the kubeadm #216

✨ Generate the manifests by calling the kubeadm #216

Conversation

charleszheng44 commented Sep 14, 2021

k8s-ci-robot commented Sep 14, 2021

christopherhein left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jzhoucliqr commented Oct 5, 2021

charleszheng44 commented Oct 5, 2021

jzhoucliqr commented Oct 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charleszheng44 commented Oct 5, 2021

charleszheng44 commented Oct 5, 2021

jzhoucliqr commented Oct 7, 2021

jzhoucliqr commented Oct 7, 2021

christopherhein commented Nov 30, 2021