scality / metalk8s

An opinionated Kubernetes distribution with a focus on long-term on-prem deployments
Apache License 2.0
363 stars 45 forks source link

`metalk8s_kubernetes.node_drain` incorrectly warns about evicting static Pods #1731

Open sayf-eddine-scality opened 5 years ago

sayf-eddine-scality commented 5 years ago

Component: salt

What happened:

[WARNING ] WARNING: Deleting pods with local storage: alertmanager-prometheus-operator-alertmanager-0, prometheus-operator-grafana-69965b54c5-9pd4c, prometheus-prometheus-operator-prometheus-0; Deleting pods not managed by ReplicationController, ReplicaSet, Job, DaemonSet or StatefulSet: etcd-bootstrap, kube-apiserver-bootstrap, kube-controller-manager-bootstrap, kube-scheduler-bootstrap, repositories-bootstrap, salt-master-bootstrap; Ignoring DaemonSet-managed pods: calico-node-6nlw8, kube-proxy-2d4mj, nginx-ingress-controller-gcxbd, prometheus-operator-prometheus-node-exporter-z8r9c

What was expected: No warning about all static Pods (names ending in `-bootstrap).

Steps to reproduce Run either an upgrade or downgrade

Resolution proposal (optional): See comment below.

gdemonet commented 5 years ago

A short investigation lead to the conclusion that the warning is misleading. There is, in our drain implementation, a _mirrorpod_filter, which should filter out any static Pod (based on the presence of the kubernetes.io/config.mirror annotation, set by Kubelet). Even if the annotation was not set, and the Pod was deleted by the drain, it would only be deleted in the API Server. Kubelet does not care about such objects, only about the manifest stored on the machine.

Now, we should fix this warning (in salt/_modules/metalk8s_drain.py#L379-L404):

         for pod in api_response.items:
-            is_deletable = True
             for pod_filter in (
                     _mirrorpod_filter,
                     self.localstorage_filter,
                     self.unreplicated_filter,
                     self.daemonset_filter
             ):
                 try:
                     filter_deletable, warning = pod_filter(pod)
                 except DrainException as exc:
                     failures.setdefault(
                         exc.message, []).append(pod.metadata.name)

+                if filter_deletable:
+                    # TODO: also clear warnings for this Pod name
+                    break
+
                 if warning:
                     warnings.setdefault(
                         warning, []).append(pod.metadata.name)
-                is_deletable &= filter_deletable
-            if is_deletable:
+            else:
                pods.append(pod)

        if failures:
            raise DrainException(_message_from_pods_dict(failures))
        if warnings:
            log.warning("WARNING: %s", _message_from_pods_dict(warnings))
        return pods