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.

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

Kubelet does not sync pod updates for static pods #116597

Open
vinaykul opened this issue Mar 14, 2023 · 7 comments
Open

Kubelet does not sync pod updates for static pods #116597

vinaykul opened this issue Mar 14, 2023 · 7 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/node Categorizes an issue or PR as relevant to SIG Node. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@vinaykul
Copy link
Contributor

What happened?

When a static pod is created on a node, it's mirror pod is created on the apiserver. Patching the mirror pod on the apiserver triggers HandlePodUpdate function in the kubelet:

func (kl *Kubelet) HandlePodUpdates(pods []*v1.Pod) {
start := kl.clock.Now()
for _, pod := range pods {
kl.podManager.UpdatePod(pod)
if kubetypes.IsMirrorPod(pod) {
kl.handleMirrorPod(pod, start)
continue
}
mirrorPod, _ := kl.podManager.GetMirrorPodByPod(pod)
kl.dispatchWork(pod, kubetypes.SyncPodUpdate, mirrorPod, start)
}
}

However, handleMirrorPod sets up SyncPodUpdate for the original pod instead of the updated pod.
// TODO: handle mirror pods in a separate component (issue #17251)
func (kl *Kubelet) handleMirrorPod(mirrorPod *v1.Pod, start time.Time) {
// Mirror pod ADD/UPDATE/DELETE operations are considered an UPDATE to the
// corresponding static pod. Send update to the pod worker if the static
// pod exists.
if pod, ok := kl.podManager.GetPodByMirrorPod(mirrorPod); ok {
kl.dispatchWork(pod, kubetypes.SyncPodUpdate, mirrorPod, start)
}
}

As a result, SyncPod does not really process the updates for static pods.

What did you expect to happen?

Pod updates should be processed for static pods in the same manner as they are processed for normal pods.

How can we reproduce it (as minimally and precisely as possible)?

Start local cluster w/ latest master (as of Mar 14, 2023):

# echo > /tmp/kubelet.log && DBG=1 ./hack/local-up-cluster.sh
...
...
+++ [0314 14:50:37] WARNING: linux/arm will no longer be built/shipped by default, please build it explicitly if needed.
+++ [0314 14:50:37]          support for linux/arm will be removed in a subsequent release.
make: Entering directory '/root/go/src/k8s.io/master_k8s'
+++ [0314 14:50:37] WARNING: linux/arm will no longer be built/shipped by default, please build it explicitly if needed.
+++ [0314 14:50:37]          support for linux/arm will be removed in a subsequent release.
go version go1.20.2 linux/arm64
+++ [0314 14:50:38] Building go targets for linux/arm64
    k8s.io/kubernetes/cmd/kubectl (static)
    k8s.io/kubernetes/cmd/kube-apiserver (static)
...
...
To start using your cluster, you can open up another terminal/tab and run:

  export KUBECONFIG=/var/run/kubernetes/admin.kubeconfig
  cluster/kubectl.sh

Alternatively, you can write to the default kubeconfig:

  export KUBERNETES_PROVIDER=local

  cluster/kubectl.sh config set-cluster local --server=https://localhost:6443 --certificate-authority=/var/run/kubernetes/server-ca.crt
  cluster/kubectl.sh config set-credentials myself --client-key=/var/run/kubernetes/client-admin.key --client-certificate=/var/run/kubernetes/client-admin.crt
  cluster/kubectl.sh config set-context local --cluster=local --user=myself
  cluster/kubectl.sh config use-context local
  cluster/kubectl.sh

Create static pod:

# cat <<EOF > /var/run/kubernetes/static-pods/bbox.yaml
apiVersion: v1
kind: Pod
metadata:
  name: bbox
spec:
  restartPolicy: OnFailure
  terminationGracePeriodSeconds: 2
  containers:
  - name: ctr
    image: busybox:1.33
    imagePullPolicy: IfNotPresent
    command: ["tail", "-f", "/dev/null"]
EOF

Verify image in container status of the static pod:

# ./cluster/kubectl.sh get pod bbox-127.0.0.1 -ojson | jq .status.containerStatuses[0].image
"docker.io/library/busybox:1.33"

Patch container image:

# ./cluster/kubectl.sh patch pod bbox-127.0.0.1 --patch '{"spec":{"containers":[{"name":"ctr", "image":"busybox:1.34"}]}}'
pod/bbox-127.0.0.1 patched

Check pod spec of static pod and verify image has been updated:

# ./cluster/kubectl.sh get pod bbox-127.0.0.1 -ojson | jq .spec.containers[0].image
"busybox:1.34"

After some time, check container status of static pod:

# ./cluster/kubectl.sh get pod bbox-127.0.0.1 -ojson | jq .status.containerStatuses[0].image
"docker.io/library/busybox:1.33"

Repeat exercise with normal pod to verify that container restarts and image has been updated.

Anything else we need to know?

No response

Kubernetes version

$ kubectl version
# paste output here
# ./cluster/kubectl.sh version
WARNING: This version information is deprecated and will be replaced with the output from kubectl version --short.  Use --output=yaml|json to get the full version.
Client Version: version.Info{Major:"1", Minor:"27+", GitVersion:"v1.27.0-alpha.3.573+a9008b502d32a5", GitCommit:"a9008b502d32a5dbacf835adb040419bf31f5c04", GitTreeState:"clean", BuildDate:"2023-03-14T14:50:38Z", GoVersion:"go1.20.2", Compiler:"gc", Platform:"linux/arm64"}
Kustomize Version: v4.5.7
Server Version: version.Info{Major:"1", Minor:"27+", GitVersion:"v1.27.0-alpha.3.573+a9008b502d32a5", GitCommit:"a9008b502d32a5dbacf835adb040419bf31f5c04", GitTreeState:"clean", BuildDate:"2023-03-14T14:50:38Z", GoVersion:"go1.20.2", Compiler:"gc", Platform:"linux/arm64"}

Cloud provider

Local ubuntu VM on mac M1

OS version

# On Linux:
$ cat /etc/os-release
# paste output here
$ uname -a
# paste output here

# On Windows:
C:\> wmic os get Caption, Version, BuildNumber, OSArchitecture
# paste output here
# uname -a
Linux vbuild 5.4.0-144-generic #161-Ubuntu SMP Fri Feb 3 14:48:57 UTC 2023 aarch64 aarch64 aarch64 GNU/Linux

Install tools

Container runtime (CRI) and version (if applicable)

Related plugins (CNI, CSI, ...) and versions (if applicable)

@vinaykul vinaykul added the kind/bug Categorizes issue or PR as related to a bug. label Mar 14, 2023
@k8s-ci-robot k8s-ci-robot added 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. labels Mar 14, 2023
@vinaykul
Copy link
Contributor Author

/sig node
/cc @smarterclayton @bobbypage @liggitt

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Mar 14, 2023
@SergeyKanzhelev SergeyKanzhelev added this to Triage in SIG Node Bugs Mar 14, 2023
@SergeyKanzhelev
Copy link
Member

/triage accepted
/priority backlog

this is not a regression, is it?

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 22, 2023
@SergeyKanzhelev SergeyKanzhelev moved this from Triage to Triaged in SIG Node Bugs Mar 22, 2023
@vinaykul
Copy link
Contributor Author

IIUC, this is not even a bug, in light of info that static pods are not expected to be resized via API , which I was unsure of when I created this issue. @smarterclayton please confirm.

That said, Imho it might be desirable to resize under/over provisioned static pods based on observed usage for better cluster utilization.

@bobbypage
Copy link
Member

bobbypage commented Mar 22, 2023

hmm, in this case is thekubectl patch running against the mirror pod?

If that's the case, I think it's expected that the image will not change. The mirror pod is suppose to be read only and reflect the static pod manifest on disk which is the source of truth. So I would expect that it would not be possible to change the static pod via updating the mirror pod.

I have #116725 open which is similar issue, but slightly different -- if the static pod manifest is updated on disk, then it is expected that the mirror pod spec should be updated as well.

@smarterclayton
Copy link
Contributor

Yes, the source of truth for spec of a static pod is the config on the node, not the mirror pod. There is no component today that reconciles the state of the pod spec on mirror pods, which would be part of #16627 through David's issue. The HandlePodUpdate is triggered to indicate that the status of the pod has changed, which is part of the flow of status info from kubelet -> api -> back to pod worker.

@Vandit1604
Copy link
Contributor

Hey, I searched for the above code in the master branch and it has been removed by this PR #102884.
Also here, it states that you can only update something in a static pod if the configured directory has some changes in it. Through the api-server. It's not possible to update via changing something in mirror-pod.

Also, I'm new to k8s and finding my first issue to work on, if my deduction in wrong in anyway. Please feel free to point it out (Happy to take brutal feedback)

@tallclair
Copy link
Member

We need to be very careful about handling any updates from the apiserver for mirror pods, as there is potential for a security issues compromising node isolation here. Mirror pods are the only type of pods that nodes are allowed to create, and in the past we've had some close calls that were mitigated by the fact that Kubelets basically ignore mirror pods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/node Categorizes an issue or PR as relevant to SIG Node. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Development

No branches or pull requests

7 participants