From e6f9ba110db97856547213453cbf2e31f6f4e8b5 Mon Sep 17 00:00:00 2001 From: Alex Simenduev Date: Sat, 30 Dec 2023 00:12:08 +0200 Subject: [PATCH 1/2] Allow draining when DaemonSet kind has custom API Group --- .../drainability/rules/replicacount/rule.go | 10 +++++--- .../rules/replicacount/rule_test.go | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/cluster-autoscaler/simulator/drainability/rules/replicacount/rule.go b/cluster-autoscaler/simulator/drainability/rules/replicacount/rule.go index 458ee234f777..21c83789d558 100644 --- a/cluster-autoscaler/simulator/drainability/rules/replicacount/rule.go +++ b/cluster-autoscaler/simulator/drainability/rules/replicacount/rule.go @@ -21,6 +21,7 @@ import ( apiv1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/autoscaler/cluster-autoscaler/simulator/drainability" "k8s.io/autoscaler/cluster-autoscaler/utils/drain" pod_util "k8s.io/autoscaler/cluster-autoscaler/utils/pod" @@ -57,7 +58,10 @@ func (r *Rule) Drainable(drainCtx *drainability.DrainContext, pod *apiv1.Pod) dr if controllerRef == nil { return drainability.NewUndefinedStatus() } - refKind := controllerRef.Kind + + groupVersionKind := schema.FromAPIVersionAndKind(controllerRef.APIVersion, controllerRef.Kind) + refGroup := groupVersionKind.Group + refKind := groupVersionKind.Kind if refKind == "ReplicationController" { rc, err := drainCtx.Listers.ReplicationControllerLister().ReplicationControllers(controllerNamespace).Get(controllerRef.Name) @@ -71,8 +75,8 @@ func (r *Rule) Drainable(drainCtx *drainability.DrainContext, pod *apiv1.Pod) dr return drainability.NewBlockedStatus(drain.MinReplicasReached, fmt.Errorf("replication controller for %s/%s has too few replicas spec: %d min: %d", pod.Namespace, pod.Name, rc.Spec.Replicas, r.minReplicaCount)) } } else if pod_util.IsDaemonSetPod(pod) { - if refKind != "DaemonSet" { - // We don't have a listener for the other DaemonSet kind. + if refGroup != "apps" || refKind != "DaemonSet" { + // We don't have a listener for the other DaemonSet group or kind. // TODO: Use a generic client for checking the reference. return drainability.NewUndefinedStatus() } diff --git a/cluster-autoscaler/simulator/drainability/rules/replicacount/rule_test.go b/cluster-autoscaler/simulator/drainability/rules/replicacount/rule_test.go index 342a69738bb5..efe09e2072a9 100644 --- a/cluster-autoscaler/simulator/drainability/rules/replicacount/rule_test.go +++ b/cluster-autoscaler/simulator/drainability/rules/replicacount/rule_test.go @@ -145,6 +145,30 @@ func TestDrainable(t *testing.T) { wantReason: drain.ControllerNotFound, wantError: true, }, + "DS-managed pod by a custom API Group": { + pod: &apiv1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Namespace: "default", + OwnerReferences: test.GenerateOwnerReferences(ds.Name, "DaemonSet", "crd/v1", ""), + }, + Spec: apiv1.PodSpec{ + NodeName: "node", + }, + }, + }, + "DS-managed pod by a custom API Group with missing reference": { + pod: &apiv1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Namespace: "default", + OwnerReferences: test.GenerateOwnerReferences("missing", "DaemonSet", "crd/v1", ""), + }, + Spec: apiv1.PodSpec{ + NodeName: "node", + }, + }, + }, "DS-managed pod by a custom Daemonset": { pod: &apiv1.Pod{ ObjectMeta: metav1.ObjectMeta{ From ea26159b2099c63412bccd60eb796b85c2839a8c Mon Sep 17 00:00:00 2001 From: Alex Simenduev Date: Mon, 22 Jan 2024 21:39:03 +0200 Subject: [PATCH 2/2] use ParseGroupVersion in Drinable method --- .../simulator/drainability/rules/replicacount/rule.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cluster-autoscaler/simulator/drainability/rules/replicacount/rule.go b/cluster-autoscaler/simulator/drainability/rules/replicacount/rule.go index 21c83789d558..e03c6d365e58 100644 --- a/cluster-autoscaler/simulator/drainability/rules/replicacount/rule.go +++ b/cluster-autoscaler/simulator/drainability/rules/replicacount/rule.go @@ -59,9 +59,12 @@ func (r *Rule) Drainable(drainCtx *drainability.DrainContext, pod *apiv1.Pod) dr return drainability.NewUndefinedStatus() } - groupVersionKind := schema.FromAPIVersionAndKind(controllerRef.APIVersion, controllerRef.Kind) - refGroup := groupVersionKind.Group - refKind := groupVersionKind.Kind + refGroup, err := schema.ParseGroupVersion(controllerRef.APIVersion) + if err != nil { + return drainability.NewUndefinedStatus() + } + + refKind := controllerRef.Kind if refKind == "ReplicationController" { rc, err := drainCtx.Listers.ReplicationControllerLister().ReplicationControllers(controllerNamespace).Get(controllerRef.Name) @@ -75,7 +78,7 @@ func (r *Rule) Drainable(drainCtx *drainability.DrainContext, pod *apiv1.Pod) dr return drainability.NewBlockedStatus(drain.MinReplicasReached, fmt.Errorf("replication controller for %s/%s has too few replicas spec: %d min: %d", pod.Namespace, pod.Name, rc.Spec.Replicas, r.minReplicaCount)) } } else if pod_util.IsDaemonSetPod(pod) { - if refGroup != "apps" || refKind != "DaemonSet" { + if refGroup.Group != "apps" || refKind != "DaemonSet" { // We don't have a listener for the other DaemonSet group or kind. // TODO: Use a generic client for checking the reference. return drainability.NewUndefinedStatus()