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.

Sign up for GitHub

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

Jump to bottom

overwrite known targets in patch manager cache in case it changes #110442

Merged
merged 1 commit into from Jun 8, 2022
Merged

overwrite known targets in patch manager cache in case it changes #110442

merged 1 commit into from Jun 8, 2022

Conversation

pacoxu
Copy link
Member

@pacoxu pacoxu commented Jun 8, 2022 •
edited

What type of PR is this?

/kind bug

What this PR does / why we need it:

#110405 add a new calling for GetPatchManagerForPath and there is a cache patch manager map which is used in PatchStaticPod as well with different known targest.

Which issue(s) this PR fixes:

https://testgrid.k8s.io/sig-cluster-lifecycle-kubeadm#kubeadm-kinder-latest-on-1-24

Special notes for your reviewer:

[control-plane] Creating static Pod manifest for "kube-apiserver"
I0608 00:49:59.881252     176 certs.go:522] validating certificate period for CA certificate
I0608 00:49:59.881454     176 manifests.go:125] [control-plane] adding volume "ca-certs" for component "kube-apiserver"
I0608 00:49:59.881471     176 manifests.go:125] [control-plane] adding volume "etc-ca-certificates" for component "kube-apiserver"
I0608 00:49:59.881480     176 manifests.go:125] [control-plane] adding volume "k8s-certs" for component "kube-apiserver"
I0608 00:49:59.881487     176 manifests.go:125] [control-plane] adding volume "usr-local-share-ca-certificates" for component "kube-apiserver"
I0608 00:49:59.881495     176 manifests.go:125] [control-plane] adding volume "usr-share-ca-certificates" for component "kube-apiserver"
unknown patch target name "kube-apiserver", must be one of [kubeletconfiguration]
k8s.io/kubernetes/cmd/kubeadm/app/util/patches.(*PatchManager).ApplyPatchesToTarget
	cmd/kubeadm/app/util/patches/patches.go:150
k8s.io/kubernetes/cmd/kubeadm/app/util/staticpod.PatchStaticPod
	cmd/kubeadm/app/util/staticpod/utils.go:188
k8s.io/kubernetes/cmd/kubeadm/app/phases/controlplane.CreateStaticPodFiles
	cmd/kubeadm/app/phases/controlplane/manifests.go:142
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/init.runControlPlaneSubphase.func1
	cmd/kubeadm/app/cmd/phases/init/controlplane.go:149
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).Run.func1
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:234
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).visitAll
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:421
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).Run
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:207
k8s.io/kubernetes/cmd/kubeadm/app/cmd.newCmdInit.func1
	cmd/kubeadm/app/cmd/init.go:153
github.com/spf13/cobra.(*Command).execute
	vendor/github.com/spf13/cobra/command.go:856
github.com/spf13/cobra.(*Command).ExecuteC
	vendor/github.com/spf13/cobra/command.go:974
github.com/spf13/cobra.(*Command).Execute
	vendor/github.com/spf13/cobra/command.go:902
k8s.io/kubernetes/cmd/kubeadm/app.Run
	cmd/kubeadm/app/kubeadm.go:50
main.main
	cmd/kubeadm/kubeadm.go:25
runtime.main
	/usr/local/go/src/runtime/proc.go:250
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1571
failed to patch static Pod manifest file for "kube-apiserver"
k8s.io/kubernetes/cmd/kubeadm/app/phases/controlplane.CreateStaticPodFiles
	cmd/kubeadm/app/phases/controlplane/manifests.go:144
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/init.runControlPlaneSubphase.func1
	cmd/kubeadm/app/cmd/phases/init/controlplane.go:149
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).Run.func1
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:234
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).visitAll
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:421
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).Run
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:207
k8s.io/kubernetes/cmd/kubeadm/app/cmd.newCmdInit.func1
	cmd/kubeadm/app/cmd/init.go:153
github.com/spf13/cobra.(*Command).execute
	vendor/github.com/spf13/cobra/command.go:856
github.com/spf13/cobra.(*Command).ExecuteC
	vendor/github.com/spf13/cobra/command.go:974
github.com/spf13/cobra.(*Command).Execute
	vendor/github.com/spf13/cobra/command.go:902
k8s.io/kubernetes/cmd/kubeadm/app.Run
	cmd/kubeadm/app/kubeadm.go:50
main.main
	cmd/kubeadm/kubeadm.go:25
runtime.main
	/usr/local/go/src/runtime/proc.go:250
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1571
error execution phase control-plane/apiserver
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).Run.func1
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:235
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).visitAll
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:421
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).Run
	cmd/kubeadm/app/cmd/phases/workflow/runner.go:207
k8s.io/kubernetes/cmd/kubeadm/app/cmd.newCmdInit.func1
	cmd/kubeadm/app/cmd/init.go:153
github.com/spf13/cobra.(*Command).execute
	vendor/github.com/spf13/cobra/command.go:856
github.com/spf13/cobra.(*Command).ExecuteC
	vendor/github.com/spf13/cobra/command.go:974
github.com/spf13/cobra.(*Command).Execute
	vendor/github.com/spf13/cobra/command.go:902
k8s.io/kubernetes/cmd/kubeadm/app.Run
	cmd/kubeadm/app/kubeadm.go:50
main.main
	cmd/kubeadm/kubeadm.go:25
runtime.main
	/usr/local/go/src/runtime/proc.go:250
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1571
Error: failed to exec action kubeadm-init: exit status 1
 exit status 1

Does this PR introduce a user-facing change?

None

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 8, 2022
@k8s-ci-robot k8s-ci-robot requested a review from RA489 June 8, 2022 01:49
@k8s-ci-robot k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jun 8, 2022
@pacoxu
Copy link
Member Author

pacoxu commented Jun 8, 2022

/priority important-soon
/assign neolit123

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jun 8, 2022
@neolit123
Copy link
Member

/triage accepted
/approve

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 8, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neolit123, pacoxu

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:

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 8, 2022
@neolit123
Copy link
Member

thanks!
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2022
@k8s-ci-robot k8s-ci-robot merged commit abba149 into kubernetes:master Jun 8, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.25 milestone Jun 8, 2022
@neolit123
Copy link
Member

looks like the kinder join is failing:
https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-kinder-latest/1534547708759511040

problem seems to be we are passing a JoinConfiguration.Patches but the directory does not exist on worker nodes:

I0608 14:54:53.016736 337 kubelet.go:171] [kubelet-start] Stopping the kubelet
[patches] Reading patches from path "/kinder/patches"
lstat /kinder/patches: no such file or directory

@neolit123
Copy link
Member

neolit123 commented Jun 8, 2022 •
edited

there are still problems in:
https://k8s-testgrid.appspot.com/sig-cluster-lifecycle-kubeadm#kubeadm-kinder-patches-latest
https://storage.googleapis.com/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-kinder-patches-latest/1534639862936768512/build-log.txt

[kubelet-start] Writing kubelet environment file with flags to file "/var/lib/kubelet/kubeadm-flags.env"
[patches] Reading patches from path "/kinder/patches"
[patches] Ignoring file "etcd+json.json": unknown target, must be one of [kubeletconfiguration]
[patches] Ignoring file "kube-apiserver.yaml": unknown target, must be one of [kubeletconfiguration]
[patches] Ignoring file "kube-controller-manager+merge.yaml": unknown target, must be one of [kubeletconfiguration]
[patches] Ignoring file "kube-scheduler0+strategic.yaml": unknown target, must be one of [kubeletconfiguration]
[patches] Ignored the following files: [etcd+json.json kube-apiserver.yaml kube-controller-manager+merge.yaml kube-scheduler0+strategic.yaml]

time="21:01:54" level=debug msg="Running: docker exec kinder-patches-control-plane-1 /kinder/verify-patches.sh"
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

i think what happens is that during the kubelet start phase only the kubeletconfiguration patch target is loaded and the rest are not when patches are to be applied to control plane manifests. one fix would be to store all known targets in a single place (e.g. patches.GetKnownTargets() ) and use them for both kubelet start phase and control plane phase when obtaining a patch manager.

alternatively make patch managers re-read patch files via a new function on demand (e.g. manager.ReadPatches()). i think this is less preferred.

@pacoxu
Copy link
Member Author

pacoxu commented Jun 9, 2022

I make static pod known targets public and change app phases.

		target := "kubeletconfiguration"
		var knownTargets = append(utilstaticpod.KnownTargets, target)

I will test locally first.

@pacoxu pacoxu mentioned this pull request Jun 9, 2022
Merged
@pacoxu
Copy link
Member Author

pacoxu commented Jun 9, 2022

https://testgrid.k8s.io/sig-cluster-lifecycle-kubeadm#kubeadm-kinder-patches-latest
test grid is green now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neolit123 neolit123 neolit123 left review comments

@RA489 RA489 Awaiting requested review from RA489

Assignees

@neolit123 neolit123

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubeadm cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.25
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@pacoxu @neolit123 @k8s-ci-robot