Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

Kata webhook mutates pods to kata runtime even for pods scheduled to run on nodes without Kata #3550

Closed
eadamsintel opened this issue May 20, 2021 · 4 comments
Labels
bug Incorrect behaviour

Comments

@eadamsintel
Copy link

The kata-deploy runtimeclass was recently updated with a fix to prevent it from being installed on a master node set to "noschedule". However, if you have a pod yaml like below it where it is told to run on master then if the webhoook is running it will add the kata runtime to the pod spec and then schedule it on the noSchedule master. This leaves the pod stuck in pending because there is no Kata runtime or Kata artifacts on the master node, but its own Pod yaml is specified to run on the noSchedule master. This is a peculiar situation because likely those pods forced to run on noSchedule master are likely management pods for the cluster and probably are not intended to be Kata containers anyways. You can't just add in runc as a RuntimeClass to the pod yaml because there is no runc RuntimeClass unless you create one and install to the cluster. It would be better if the webhook had a way to check if Kata artifacts were on the node being scheduled before mutating it to be a kata container.

To fix the situation you have to 1) remove the webhook, 2) remove the pod 3) reschedule the pod. It isn't enough to remove the webhook and re-apply the pod config. The pod has to be physically deleted and recreated.

kind: Pod
apiVersion: v1
metadata:
  name: nginx-schedule-on-master
spec:
  containers:
    - name: nginx
      image: nginx
      command: [ "sleep", "100000" ]
  nodeSelector:
    node-role.kubernetes.io/master: ""
  tolerations:
    - key: "node-role.kubernetes.io/master"
      operator: "Equal"
      value: ""
      effect: "NoSchedule"

Here is what ends up happening.

# kubectl get pods
NAME                                    READY   STATUS    RESTARTS   AGE
nginx-schedule-on-master                0/1     Pending   0          10s
pod-annotate-webhook-75657bfb5f-2c26z   1/1     Running   0          9d

# kubectl describe pod nginx-schedule-on-master
Name:         nginx-schedule-on-master
Namespace:    default
Priority:     0
Node:         <none>
Labels:       <none>
Annotations:  kubernetes.io/psp: privileged
Status:       Pending
IP:
IPs:          <none>
Containers:
  nginx:
    Image:      nginx
    Port:       <none>
    Host Port:  <none>
    Command:
      sleep
      100000
    Environment:  <none>
    Mounts:
      /var/run/secrets/kubernetes.io/serviceaccount from default-token-bkc54 (ro)
Conditions:
  Type           Status
  PodScheduled   False
Volumes:
  default-token-bkc54:
    Type:        Secret (a volume populated by a Secret)
    SecretName:  default-token-bkc54
    Optional:    false
QoS Class:       BestEffort
Node-Selectors:  katacontainers.io/kata-runtime=true
                 node-role.kubernetes.io/master=
Tolerations:     node-role.kubernetes.io/master:NoSchedule
                 node.kubernetes.io/not-ready:NoExecute for 300s
                 node.kubernetes.io/unreachable:NoExecute for 300s
Events:
  Type     Reason            Age   From               Message
  ----     ------            ----  ----               -------
  Warning  FailedScheduling  20s   default-scheduler  0/2 nodes are available: 2 node(s) didn't match node selector.
  Warning  FailedScheduling  20s   default-scheduler  0/2 nodes are available: 2 node(s) didn't match node selector.
@eadamsintel eadamsintel added bug Incorrect behaviour needs-review Needs to be assessed by the team. labels May 20, 2021
@chavafg
Copy link
Contributor

chavafg commented May 21, 2021

@wainersm @fidencio have you experience this scenario in your OpenShift testing?

Not really sure what would be the best way to detect kata-artifacts are installed as they can be placed in different directories.

@ariel-adam ariel-adam added bug Incorrect behaviour and removed bug Incorrect behaviour needs-review Needs to be assessed by the team. labels May 25, 2021
@fidencio
Copy link
Member

@eadamsintel, @chavafg,

First of all, let me say sorry it took so long for me to get to this one.

The issue is real and I don't think we ran into that because we're, theoretically, forcing the webhook to exclude some namespaces. It's a cheap and dirty workaround, but that prevents this issue.

Not exactly related to this, but I'd like to get the webhook out of the test repo and make it on its own kata-containers/webhook, where we can grow this tool together. I know @snir911 and @dmaizel have some fixes that would help a lot when upstreamed.

@snir911
Copy link
Member

snir911 commented May 26, 2021

If there's some annotation which specified whether kata is installed on a node i believe it would be easy to check that with the webhook, maybe it's also possible to allow it to apply kata only on workers, although it's not very nice i guess...
@eadamsintel I'd also suggest you to check out the kyverno.io project which could be an alternative for the webhook and i think it allows you to define the adding-kata-runtimeclass rule only on a subset of nodes

@eadamsintel
Copy link
Author

@snir911 When you install with kata-deploy then it labels the nodes that have Kata installed with katacontainers.io/kata-runtime=true My master doesn't have this label but the worker nodes do.

That would solve the issue for my use case, but that might only work for kata-deploy scenarios. I don't think you can count on this label being there if Kata is installed through other means like snap or manual install.

surajssd added a commit to surajssd/tests that referenced this issue Jul 12, 2023
Fixes: kata-containers#3550

Signed-off-by: Suraj Deshmukh <suraj.deshmukh@microsoft.com>
surajssd added a commit to surajssd/tests that referenced this issue Jul 12, 2023
Fixes: kata-containers#3550

Signed-off-by: Suraj Deshmukh <suraj.deshmukh@microsoft.com>
@GabyCT GabyCT closed this as completed in 53b9302 Jul 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Incorrect behaviour
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants