Ability to ignore some containers (sidecars) when evaluating if a pod is evictable or not

[ldemailly]

Right now if you have a sidecar like istio's (istio/istio#19395) with an EmptyDir and the main service without it; the pod is excluded

A work around is to set the annotation

   cluster-autoscaler.kubernetes.io/safe-to-evict: "true"

but that applies at the whole pod level and one would need to duplicate the logic checking for volumes to decide true/false

Can we instead have an annotation like

cluster-autoscaler.kubernetes.io/ignore-container: istio-proxy

(or maybe a pattern, ignored-containers: pattern)

That would skip said container(s) [volumes] for the determination of whether a pod is evictable or not ?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[thesuperzapper]

I think this feature is important.

Not sure If I have permission to remove stale, but here goes:
/remove-lifecycle stale

[davidspek]

This indeed is an important feature that should be added.

[artificial-aidan]

This is a show stopper for workloads that use a mix of local storage and HA services. You can't use the skip-local-storage option, because some workloads need it. The ignored container pattern would work well, because it could be used for anything injected, or specifically called out.

[thesuperzapper]

@bskiba or @jbartosik what are your thoughts on a feature to ignore some sidecars for autoscaling?

[fvogl]

I would love to see this feature implemented.

[thesuperzapper]

@marwanad or @bskiba is on your roadmap for cluster-autoscaler, or does someone else needs to drive this contribution?

The ability to ignore some volumes (based on an annotation) is a very important feature to prevent people from using the cluster-autoscaler.kubernetes.io/safe-to-evict safe-hatch on almost every Pod.

[thesuperzapper]

@ldemailly alternatively, we could implement this as cluster-autoscaler.kubernetes.io/ignore-volume: my-volume-name, rather than ignoring a specific container name (which might be more complex to implement), as I am not sure the cluster-autoscaler understands the container <-> volume relationship).

[ldemailly]

whichever way gets us the feature sounds good to me

[mbyio]

I'm trying to understand why this feature is needed. Couldn't you set skip-nodes-with-local-storage: false so the default is to evict regardless of empty dir usage, and then set cluster-autoscaler.kubernetes.io/safe-to-evict: false on the pods that you want to protect?

[mfilocha]

@mbyio On GKE, for example, you cannot change autoscaler configuration, afaik: https://cloud.google.com/kubernetes-engine/docs/concepts/cluster-autoscaler#scheduling-and-disruption

[sneko]

As mentioned by @mfilocha it seems we are stuck for now until it's customisable on the main existing autoscalers?

Better to go with a manual annotation patch in the meantime, no?

[mfilocha]

According to https://cloud.google.com/kubernetes-engine/docs/release-notes#October_27_2021: "In GKE version 1.22 and later, GKE cluster autoscaler (...) will support scaling down nodes with pods requesting local storage."

[ldemailly]

I'm trying to understand why this feature is needed. Couldn't you set skip-nodes-with-local-storage: false so the default is to evict regardless of empty dir usage, and then set cluster-autoscaler.kubernetes.io/safe-to-evict: false on the pods that you want to protect?

Because to do so you would need to repeat/reimplement the logic of checking the other containers for volumes. the use case is side-car injection through admission controller for instance.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[thesuperzapper]

We should keep tracking this issue. I still believe adding a way to ignore specific volumes (possibly with an annotation like cluster-autoscaler.kubernetes.io/ignore-volume: my-volume-name) is important.

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[eraac]

I think the issue is still relevant and must be stay open

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[sovitacharya]

Much Needed Feature as this block the optimal use of istio alongside , the Kubernetes Platform.
/remove-lifecycle stale

[jbartosik]

I guess

/remove-lifecycle rotten

I don't work on Cluster Autoscaler. So I don't plan to work on this. I'd suggest asking about this on the SIG meeting. I think this is much more likely to happen if someone volunteers to contribute this

[vadasambar]

It sounds like we just want to ignore local storage during node scale down for certain volumes.

This is kind of putting the horse before the cart (I am in the middle of trying to reproduce the issue) but I just whipped a reference implementation PR which I think should work: #5594

[vadasambar]

I can reproduce this issue.

Steps to reproduce:

1. Overprovision nodes using scale-up/dummy workload (check below for sample workload)
2. Taint the newly provisioned nodes with kubectl taint node <node-name> test=true:NoSchedule
3. Install Istio from https://istio.io/latest/docs/setup/getting-started/
4. Enable Istio injection for the test namespace (I am using default) e.g., kubectl label namespace default istio-injection=enabled
5. Create test workload which gets scheduled on and tolerates the tainted node e.g., kubectl create deployment nginx --image=nginx --replicas=1, edit the deployment to tolerate the tainted node (add node selector to match tainted node if required)
6. Scale down the scale-up/dummy workload (so that cluster-autoscaler thinks the tainted node is under-utilized and attempts to scale it down)
7. You should see an error like this in the cluster-autoscaler logs
   

Click here to see sample scale-up/dummy workload

apiVersion: apps/v1
kind: Deployment
metadata:
  name: node-scale-up-with-pod-anti-affinity
  namespace: default
spec:
  selector:
    matchLabels:
      app: node-scale-up-with-pod-anti-affinity
  replicas: 1
  template:
    metadata:
      labels:
        app: node-scale-up-with-pod-anti-affinity
    spec:
      nodeSelector:
        cloud.google.com/gke-nodepool: pool-1
     # tolerations:
     #   - effect: NoSchedule
     #     key: test
     #     operator: Equal
     #     value: "true"
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
          - labelSelector:
              matchExpressions:
              - key: app
                operator: In
                values:
                - node-scale-up-with-pod-anti-affinity
            topologyKey: "kubernetes.io/hostname"
      containers:
      - name: node-scale-up-with-pod-anti-affinity
        image: registry.k8s.io/pause:2.0

Click here to see what the istio sidecar injected test workload pod looks like

apiVersion: v1
kind: Pod
metadata:
  annotations:
    kubectl.kubernetes.io/default-container: nginx
    kubectl.kubernetes.io/default-logs-container: nginx
    prometheus.io/path: /stats/prometheus
    prometheus.io/port: "15020"
    prometheus.io/scrape: "true"
    sidecar.istio.io/status: '{"initContainers":["istio-init"],"containers":["istio-proxy"],"volumes":["workload-socket","credential-socket","workload-certs","istio-envoy","istio-data","istio-podinfo","istio-token","istiod-ca-cert"],"imagePullSecrets":null,"revision":"default"}'
  creationTimestamp: "2023-03-14T06:16:01Z"
  generateName: nginx-7ddf8d98f9-
  labels:
    app: nginx
    pod-template-hash: 7ddf8d98f9
    security.istio.io/tlsMode: istio
    service.istio.io/canonical-name: nginx
    service.istio.io/canonical-revision: latest
  name: nginx-7ddf8d98f9-ngq4k
  namespace: default
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: true
    controller: true
    kind: ReplicaSet
    name: nginx-7ddf8d98f9
    uid: 60ba242d-8bdc-469c-91b1-1f274164795b
  resourceVersion: "27326"
  uid: e45e2546-8e8d-4afc-905c-e8b32db3860b
spec:
  containers:
  - image: nginx
    imagePullPolicy: Always
    name: nginx
    resources: {}
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access-fl78v
      readOnly: true
  - args:
    - proxy
    - sidecar
    - --domain
    - $(POD_NAMESPACE).svc.cluster.local
    - --proxyLogLevel=warning
    - --proxyComponentLogLevel=misc:error
    - --log_output_level=default:info
    - --concurrency
    - "2"
    env:
    - name: JWT_POLICY
      value: third-party-jwt
    - name: PILOT_CERT_PROVIDER
      value: istiod
    - name: CA_ADDR
      value: istiod.istio-system.svc:15012
    - name: POD_NAME
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: metadata.name
    - name: POD_NAMESPACE
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: metadata.namespace
    - name: INSTANCE_IP
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: status.podIP
    - name: SERVICE_ACCOUNT
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: spec.serviceAccountName
    - name: HOST_IP
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: status.hostIP
    - name: PROXY_CONFIG
      value: |
        {}
    - name: ISTIO_META_POD_PORTS
      value: |-
        [
        ]
    - name: ISTIO_META_APP_CONTAINERS
      value: nginx
    - name: ISTIO_META_CLUSTER_ID
      value: Kubernetes
    - name: ISTIO_META_NODE_NAME
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: spec.nodeName
    - name: ISTIO_META_INTERCEPTION_MODE
      value: REDIRECT
    - name: ISTIO_META_WORKLOAD_NAME
      value: nginx
    - name: ISTIO_META_OWNER
      value: kubernetes://apis/apps/v1/namespaces/default/deployments/nginx
    - name: ISTIO_META_MESH_ID
      value: cluster.local
    - name: TRUST_DOMAIN
      value: cluster.local
    image: docker.io/istio/proxyv2:1.17.1
    imagePullPolicy: IfNotPresent
    name: istio-proxy
    ports:
    - containerPort: 15090
      name: http-envoy-prom
      protocol: TCP
    readinessProbe:
      failureThreshold: 30
      httpGet:
        path: /healthz/ready
        port: 15021
        scheme: HTTP
      initialDelaySeconds: 1
      periodSeconds: 2
      successThreshold: 1
      timeoutSeconds: 3
    resources:
      limits:
        cpu: "2"
        memory: 1Gi
      requests:
        cpu: 10m
        memory: 40Mi
    securityContext:
      allowPrivilegeEscalation: false
      capabilities:
        drop:
        - ALL
      privileged: false
      readOnlyRootFilesystem: true
      runAsGroup: 1337
      runAsNonRoot: true
      runAsUser: 1337
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /var/run/secrets/workload-spiffe-uds
      name: workload-socket
    - mountPath: /var/run/secrets/credential-uds
      name: credential-socket
    - mountPath: /var/run/secrets/workload-spiffe-credentials
      name: workload-certs
    - mountPath: /var/run/secrets/istio
      name: istiod-ca-cert
    - mountPath: /var/lib/istio/data
      name: istio-data
    - mountPath: /etc/istio/proxy
      name: istio-envoy
    - mountPath: /var/run/secrets/tokens
      name: istio-token
    - mountPath: /etc/istio/pod
      name: istio-podinfo
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access-fl78v
      readOnly: true
  dnsPolicy: ClusterFirst
  enableServiceLinks: true
  initContainers:
  - args:
    - istio-iptables
    - -p
    - "15001"
    - -z
    - "15006"
    - -u
    - "1337"
    - -m
    - REDIRECT
    - -i
    - '*'
    - -x
    - ""
    - -b
    - '*'
    - -d
    - 15090,15021,15020
    - --log_output_level=default:info
    image: docker.io/istio/proxyv2:1.17.1
    imagePullPolicy: IfNotPresent
    name: istio-init
    resources:
      limits:
        cpu: "2"
        memory: 1Gi
      requests:
        cpu: 10m
        memory: 40Mi
    securityContext:
      allowPrivilegeEscalation: false
      capabilities:
        add:
        - NET_ADMIN
        - NET_RAW
        drop:
        - ALL
      privileged: false
      readOnlyRootFilesystem: false
      runAsGroup: 0
      runAsNonRoot: false
      runAsUser: 0
    terminationMessagePath: /dev/termination-log
    terminationMessagePolicy: File
    volumeMounts:
    - mountPath: /var/run/secrets/kubernetes.io/serviceaccount
      name: kube-api-access-fl78v
      readOnly: true
  nodeName: gke-cluster-1-pool-1-45d8490d-twxf
  preemptionPolicy: PreemptLowerPriority
  priority: 0
  restartPolicy: Always
  schedulerName: default-scheduler
  securityContext: {}
  serviceAccount: default
  serviceAccountName: default
  terminationGracePeriodSeconds: 30
  tolerations:
  - effect: NoSchedule
    key: test
    value: "true"
  - effect: NoExecute
    key: node.kubernetes.io/not-ready
    operator: Exists
    tolerationSeconds: 300
  - effect: NoExecute
    key: node.kubernetes.io/unreachable
    operator: Exists
    tolerationSeconds: 300
  volumes:
  - emptyDir: {}
    name: workload-socket
  - emptyDir: {}
    name: credential-socket
  - emptyDir: {}
    name: workload-certs
  - emptyDir:
      medium: Memory
    name: istio-envoy
  - emptyDir: {}
    name: istio-data
  - downwardAPI:
      defaultMode: 420
      items:
      - fieldRef:
          apiVersion: v1
          fieldPath: metadata.labels
        path: labels
      - fieldRef:
          apiVersion: v1
          fieldPath: metadata.annotations
        path: annotations
    name: istio-podinfo
  - name: istio-token
    projected:
      defaultMode: 420
      sources:
      - serviceAccountToken:
          audience: istio-ca
          expirationSeconds: 43200
          path: istio-token
  - configMap:
      defaultMode: 420
      name: istio-ca-root-cert
    name: istiod-ca-cert
  - name: kube-api-access-fl78v
    projected:
      defaultMode: 420
      sources:
      - serviceAccountToken:
          expirationSeconds: 3607
          path: token
      - configMap:
          items:
          - key: ca.crt
            path: ca.crt
          name: kube-root-ca.crt
      - downwardAPI:
          items:
          - fieldRef:
              apiVersion: v1
              fieldPath: metadata.namespace
            path: namespace
status:
  conditions:
  - lastProbeTime: null
    lastTransitionTime: "2023-03-14T06:16:09Z"
    status: "True"
    type: Initialized
  - lastProbeTime: null
    lastTransitionTime: "2023-03-14T06:16:11Z"
    status: "True"
    type: Ready
  - lastProbeTime: null
    lastTransitionTime: "2023-03-14T06:16:11Z"
    status: "True"
    type: ContainersReady
  - lastProbeTime: null
    lastTransitionTime: "2023-03-14T06:16:02Z"
    status: "True"
    type: PodScheduled
  containerStatuses:
  - containerID: containerd://95a275e1ce8cbfa55df2aa872dc6313706fbb9672a5290b1342e7ac1892563e7
    image: docker.io/istio/proxyv2:1.17.1
    imageID: docker.io/istio/proxyv2@sha256:2152aea5fbe2de20f08f3e0412ad7a4cd54a492240ff40974261ee4bdb43871d
    lastState: {}
    name: istio-proxy
    ready: true
    restartCount: 0
    started: true
    state:
      running:
        startedAt: "2023-03-14T06:16:10Z"
  - containerID: containerd://f1aa9db4d8cbdadb0fd6125cd1dd94914d697a752cfbf1903ebcec1c05205bd9
    image: docker.io/library/nginx:latest
    imageID: docker.io/library/nginx@sha256:aa0afebbb3cfa473099a62c4b32e9b3fb73ed23f2a75a65ce1d4b4f55a5c2ef2
    lastState: {}
    name: nginx
    ready: true
    restartCount: 0
    started: true
    state:
      running:
        startedAt: "2023-03-14T06:16:10Z"
  hostIP: 10.128.0.61
  initContainerStatuses:
  - containerID: containerd://688d48287ced31d3bff4dd8ef7d20279b7875255c3b5a7bafec93ce30f0027fc
    image: docker.io/istio/proxyv2:1.17.1
    imageID: docker.io/istio/proxyv2@sha256:2152aea5fbe2de20f08f3e0412ad7a4cd54a492240ff40974261ee4bdb43871d
    lastState: {}
    name: istio-init
    ready: true
    restartCount: 0
    state:
      terminated:
        containerID: containerd://688d48287ced31d3bff4dd8ef7d20279b7875255c3b5a7bafec93ce30f0027fc
        exitCode: 0
        finishedAt: "2023-03-14T06:16:07Z"
        reason: Completed
        startedAt: "2023-03-14T06:16:07Z"
  phase: Running
  podIP: 10.4.3.4
  podIPs:
  - ip: 10.4.3.4
  qosClass: Burstable
  startTime: "2023-03-14T06:16:02Z"

Extra info

Kubernetes and istioctl version I used:

suraj@suraj:~/Downloads$ k version --short
...
Client Version: v1.24.4
Kustomize Version: v4.5.4
Server Version: v1.24.9-gke.3200

suraj@suraj:~/Downloads$ istioctl version 
...
1.17.1

1.17.1 is compatible with 1.24.x version of Kubernetes based on https://istio.io/latest/docs/releases/supported-releases/#support-status-of-istio-releases

Note that I deployed my own cluster-autoscaler in a GKE cluster to test this issue.

This is a trimmed down version of my own notes. Let me know if you want me to add more info.

[thesuperzapper]

For those watching, it looks like cluster-autoscaler 1.27.0 implemented the cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes annotation, you can read about it in the FAQ.

I think we can still improve the cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes annotation (especially for istio), by allowing it to be set as "*", which would ignore ALL local volumes rather than requiring us to specify the full list.

I have also commented this idea here: #5594 (comment)

[vadasambar]

For those watching, it looks like cluster-autoscaler 1.27.0 implemented the cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes annotation, you can read about it in the FAQ.

I think we can still improve the cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes annotation (especially for istio), by allowing it to be set as "*", which would ignore ALL local volumes rather than requiring us to specify the full list.

I have also commented this idea here: #5594 (comment)

Quoting my comment from the PR #5594 (comment):

I think there is a room for improvement in the annotation. "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes": "*" will have the same effect as "cluster-autoscaler.kubernetes.io/safe-to-evict": "true" (ref). There are 2 cases here:

1. You have set "cluster-autoscaler.kubernetes.io/safe-to-evict": "false" (pod is not safe to evict)
2. You have set "cluster-autoscaler.kubernetes.io/safe-to-evict": "true" (pod is safe to evict; if this is set "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes" annotation will be ignored)

In 1, if you add "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes": "*", it will have the same effect as setting 2
In 2, if you add "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes": "*", it will have the same effect as setting 2

Unless there is a specific usecase, I don't see a lot of value in adding "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes": "*" because we already support "cluster-autoscaler.kubernetes.io/safe-to-evict": "true".

I guess what you want is "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes": "<container-name>:*" e.g., "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes": "istio:*" which makes sense because it provides better user experience than specifying the entire list.

[vadasambar]

For those watching, it looks like cluster-autoscaler 1.27.0 implemented the cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes annotation, you can read about it in the FAQ.

Note that we are patching this feature back to 1.24, 1.25 and 1.26 as well. Expect the patch to be released this week: #5774 (comment)

[vadasambar]

@thesuperzapper if you have any thoughts around #3947 (comment) let me know. I will create a separate ticket to enhance the annotation so that it supports "cluster-autoscaler.kubernetes.io/safe-to-evict-local-volumes": "<container-name>:*" (or feel free to create it yourself and link it here :))