Filter pod statuses reported back to containers which exist in proxy pod

pkg/controllers/feedback/controller.go

@@ -24,6 +24,7 @@ import (
 
 	"admiralty.io/multicluster-scheduler/pkg/config/agent"
 	"github.com/go-test/deep"
+
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -198,11 +199,11 @@ func (c *reconciler) Handle(obj interface{}) (requeueAfter *time.Duration, err e
 
 			// we can't group annotation and status updates into an update,
 			// because general update ignores status
-
-			needStatusUpdate := deep.Equal(proxyPod.Status, delegate.Status) != nil
+			filteredDelegateStatus := filterContainerStatus(&proxyPod.Spec, delegate.Status)
+			needStatusUpdate := deep.Equal(proxyPod.Status, filteredDelegateStatus) != nil
 			if needStatusUpdate {
 				podCopy := proxyPod.DeepCopy()
-				podCopy.Status = delegate.Status
+				podCopy.Status = filteredDelegateStatus
 
 				var err error
 				if proxyPod, err = c.kubeclientset.CoreV1().Pods(namespace).UpdateStatus(ctx, podCopy, metav1.UpdateOptions{}); err != nil {
@@ -229,6 +230,64 @@ func (c *reconciler) Handle(obj interface{}) (requeueAfter *time.Duration, err e
 	return nil, nil
 }
 
+// filterContainerStatus returns a shallow copy of delegateStatus with container / initContainer / ephemeralContainer
+// statuses filtered to containers which actually exist in proxyPodSpec.
+// Nothing in the original container status lists in delegateStatus is mutated.
+func filterContainerStatus(proxyPodSpec *corev1.PodSpec, delegateStatus corev1.PodStatus) corev1.PodStatus {
+	delegateStatus.ContainerStatuses = filterContainerStatuses(delegateStatus.ContainerStatuses, hasContainerName(proxyPodSpec.Containers))
+	delegateStatus.InitContainerStatuses = filterContainerStatuses(delegateStatus.InitContainerStatuses, hasContainerName(proxyPodSpec.InitContainers))
+	delegateStatus.EphemeralContainerStatuses = filterContainerStatuses(delegateStatus.EphemeralContainerStatuses, hasEphemeralContainerName(proxyPodSpec.EphemeralContainers))
+	return delegateStatus
+}
+
+// filterContainerStatuses returns a list of the container statuses for which hasName returns true.
+// If hasName returns true for all, statuses is returned as-is.
+// If hasName returns false for any, a filtered copy is returned.
+// statuses is never modified.
+func filterContainerStatuses(statuses []corev1.ContainerStatus, hasName func(name string) bool) []corev1.ContainerStatus {
+	copied := false
+	retval := statuses
+	for i, s := range statuses {
+		if hasName(s.Name) {
+			if copied {
+				// if we're working with a copy, append to our copy
+				retval = append(retval, s)
+			}
+			continue
+		}
+
+		if !copied {
+			// copy statuses up to i
+			retval = make([]corev1.ContainerStatus, i, len(statuses)-1)
+			copy(retval, statuses[0:i])
+			copied = true
+		}
+	}
+	return retval
+}
+
+func hasContainerName(containers []corev1.Container) func(name string) bool {
+	return func(name string) bool {
+		for _, c := range containers {
+			if c.Name == name {
+				return true
+			}
+		}
+		return false
+	}
+}
+
+func hasEphemeralContainerName(containers []corev1.EphemeralContainer) func(name string) bool {
+	return func(name string) bool {
+		for _, c := range containers {
+			if c.Name == name {
+				return true
+			}
+		}
+		return false
+	}
+}
+
 func (c *reconciler) removeFinalizer(ctx context.Context, pod *corev1.Pod, j int) (*corev1.Pod, error) {
 	podCopy := pod.DeepCopy()
 	podCopy.Finalizers = append(podCopy.Finalizers[:j], podCopy.Finalizers[j+1:]...)

pkg/controllers/feedback/controller_test.go

@@ -0,0 +1,117 @@
+/*
+ * Copyright 2024 The Multicluster-Scheduler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package feedback
+
+import (
+	"reflect"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+)
+
+func TestFilterContainerStatuses(t *testing.T) {
+	testcases := []struct {
+		Name   string
+		In     []string
+		Has    []string
+		Expect []string
+	}{
+		{
+			Name:   "empty",
+			In:     []string{},
+			Has:    []string{},
+			Expect: []string{},
+		},
+		{
+			Name:   "has_all",
+			In:     []string{"a", "b", "c"},
+			Has:    []string{"a", "b", "c"},
+			Expect: []string{"a", "b", "c"},
+		},
+		{
+			Name:   "has_none",
+			In:     []string{"a", "b", "c"},
+			Has:    []string{},
+			Expect: []string{},
+		},
+		{
+			Name:   "has_first",
+			In:     []string{"a", "b", "c"},
+			Has:    []string{"a"},
+			Expect: []string{"a"},
+		},
+		{
+			Name:   "has_last",
+			In:     []string{"a", "b", "c"},
+			Has:    []string{"c"},
+			Expect: []string{"c"},
+		},
+		{
+			Name:   "missing_first",
+			In:     []string{"a", "b", "c"},
+			Has:    []string{"b", "c"},
+			Expect: []string{"b", "c"},
+		},
+		{
+			Name:   "missing_last",
+			In:     []string{"a", "b", "c"},
+			Has:    []string{"a", "b"},
+			Expect: []string{"a", "b"},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.Name, func(t *testing.T) {
+			in := []corev1.ContainerStatus{}
+			for _, name := range tc.In {
+				in = append(in, corev1.ContainerStatus{Name: name})
+			}
+
+			has := func(name string) bool {
+				for _, n := range tc.Has {
+					if name == n {
+						return true
+					}
+				}
+				return false
+			}
+
+			out := filterContainerStatuses(in, has)
+			outNames := []string{}
+			for _, s := range out {
+				outNames = append(outNames, s.Name)
+			}
+			if !reflect.DeepEqual(outNames, tc.Expect) {
+				t.Fatalf("expected %v, got %v", tc.Expect, outNames)
+			}
+
+			// test if input was returned as-is or copied
+			if len(in) > 0 && len(out) > 0 {
+				copied := &in[0] != &out[0]
+				if reflect.DeepEqual(tc.In, tc.Expect) {
+					if copied {
+						t.Fatalf("expected input to be returned as-is when no filtering occurred")
+					}
+				} else {
+					if !copied {
+						t.Fatalf("expected input to be copied when filtering occurred")
+					}
+				}
+			}
+		})
+	}
+}