diff --git a/cluster-autoscaler/core/equivalence_groups.go b/cluster-autoscaler/core/equivalence_groups.go index e509d61ffbee..98007032b939 100644 --- a/cluster-autoscaler/core/equivalence_groups.go +++ b/cluster-autoscaler/core/equivalence_groups.go @@ -17,10 +17,10 @@ limitations under the License. package core import ( + "k8s.io/autoscaler/cluster-autoscaler/core/utils" "reflect" apiv1 "k8s.io/api/core/v1" - apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/types" "k8s.io/autoscaler/cluster-autoscaler/processors/status" "k8s.io/autoscaler/cluster-autoscaler/utils/drain" @@ -67,7 +67,7 @@ func groupPodsBySchedulingProperties(pods []*apiv1.Pod) map[equivalenceGroupId][ matchingFound := false for _, g := range equivalenceGroupsByController[controllerRef.UID] { - if reflect.DeepEqual(pod.Labels, g.representant.Labels) && apiequality.Semantic.DeepEqual(pod.Spec, g.representant.Spec) { + if reflect.DeepEqual(pod.Labels, g.representant.Labels) && utils.PodSpecSemanticallyEqual(pod.Spec, g.representant.Spec) { matchingFound = true podEquivalenceGroups[g.id] = append(podEquivalenceGroups[g.id], pod) break diff --git a/cluster-autoscaler/core/equivalence_groups_test.go b/cluster-autoscaler/core/equivalence_groups_test.go index ff71c832ba4e..51a125de010c 100644 --- a/cluster-autoscaler/core/equivalence_groups_test.go +++ b/cluster-autoscaler/core/equivalence_groups_test.go @@ -47,6 +47,16 @@ func TestGroupSchedulablePodsForNode(t *testing.T) { }, } + rc3 := apiv1.ReplicationController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rc3", + Namespace: "default", + SelfLink: "api/v1/namespaces/default/replicationcontrollers/rc3", + UID: "12345678-1234-1234-1234-12345678901e", + }, + } + + projectedSAVol := BuildServiceTokenProjectedVolumeSource("path") p1 := BuildTestPod("p1", 1500, 200000) p2_1 := BuildTestPod("p2_1", 3000, 200000) p2_1.OwnerReferences = GenerateOwnerReferences(rc1.Name, "ReplicationController", "extensions/v1beta1", rc1.UID) @@ -56,10 +66,16 @@ func TestGroupSchedulablePodsForNode(t *testing.T) { p3_1.OwnerReferences = GenerateOwnerReferences(rc2.Name, "ReplicationController", "extensions/v1beta1", rc2.UID) p3_2 := BuildTestPod("p3_2", 100, 200000) p3_2.OwnerReferences = GenerateOwnerReferences(rc2.Name, "ReplicationController", "extensions/v1beta1", rc2.UID) - unschedulablePods := []*apiv1.Pod{p1, p2_1, p2_2, p3_1, p3_2} + p4_1 := BuildTestPod("p4_1", 100, 200000) + p4_1.OwnerReferences = GenerateOwnerReferences(rc2.Name, "ReplicationController", "extensions/v1beta1", rc3.UID) + p4_1.Spec.Volumes = []apiv1.Volume{{Name: "kube-api-access-nz94b", VolumeSource: apiv1.VolumeSource{Projected: projectedSAVol}}} + p4_2 := BuildTestPod("p4_2", 100, 200000) + p4_2.OwnerReferences = GenerateOwnerReferences(rc2.Name, "ReplicationController", "extensions/v1beta1", rc3.UID) + p4_2.Spec.Volumes = []apiv1.Volume{{Name: "kube-api-access-mo25i", VolumeSource: apiv1.VolumeSource{Projected: projectedSAVol}}} + unschedulablePods := []*apiv1.Pod{p1, p2_1, p2_2, p3_1, p3_2, p4_1, p4_2} podGroups := groupPodsBySchedulingProperties(unschedulablePods) - assert.Equal(t, 3, len(podGroups)) + assert.Equal(t, 4, len(podGroups)) wantedGroups := []struct { pods []*apiv1.Pod @@ -68,6 +84,7 @@ func TestGroupSchedulablePodsForNode(t *testing.T) { {pods: []*apiv1.Pod{p1}}, {pods: []*apiv1.Pod{p2_1, p2_2}}, {pods: []*apiv1.Pod{p3_1, p3_2}}, + {pods: []*apiv1.Pod{p4_1, p4_2}}, } equal := func(a, b []*apiv1.Pod) bool { diff --git a/cluster-autoscaler/utils/test/test_utils.go b/cluster-autoscaler/utils/test/test_utils.go index 8d99198efbce..969c25fb39af 100644 --- a/cluster-autoscaler/utils/test/test_utils.go +++ b/cluster-autoscaler/utils/test/test_utils.go @@ -65,6 +65,20 @@ func BuildTestPod(name string, cpu int64, mem int64) *apiv1.Pod { return pod } +// BuildServiceTokenProjectedVolumeSource returns a ProjectedVolumeSource with SA token +// projection +func BuildServiceTokenProjectedVolumeSource(path string) *apiv1.ProjectedVolumeSource { + return &apiv1.ProjectedVolumeSource{ + Sources: []apiv1.VolumeProjection{ + { + ServiceAccountToken: &apiv1.ServiceAccountTokenProjection{ + Path: path, + }, + }, + }, + } +} + const ( // cannot use constants from gpu module due to cyclic package import resourceNvidiaGPU = "nvidia.com/gpu" diff --git a/cluster-autoscaler/utils/utils.go b/cluster-autoscaler/utils/utils.go index 8ba4c642b3d3..4996278eb31e 100644 --- a/cluster-autoscaler/utils/utils.go +++ b/cluster-autoscaler/utils/utils.go @@ -18,6 +18,7 @@ package utils import ( apiv1 "k8s.io/api/core/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" klog "k8s.io/klog/v2" "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" @@ -54,3 +55,38 @@ func FilterOutNodes(nodes []*apiv1.Node, nodesToFilterOut []*apiv1.Node) []*apiv return filtered } + +// PodSpecSemanticallyEqual returns if two pod specs are similar after dropping +// the fields we don't care about +func PodSpecSemanticallyEqual(p1 apiv1.PodSpec, p2 apiv1.PodSpec) bool { + p1Spec := sanitizedProjectedVolumesAndMounts(p1) + p2Spec := sanitizedProjectedVolumesAndMounts(p2) + return apiequality.Semantic.DeepEqual(p1Spec, p2Spec) +} + +// sanitizedProjectedVolumesAndMounts returns a pod spec with projected volumes +// and their mounts removed +// Due to the generated suffixes, a strict DeepEquals check will fail and generate +// an equivalence group per pod which is undesirable. +// Projected volumes do not impact scheduling so we should ignore them +func sanitizedProjectedVolumesAndMounts(podSpec apiv1.PodSpec) apiv1.PodSpec { + var projectedVolumeNames []string + for i, v := range podSpec.Volumes { + if v.Projected != nil { + projectedVolumeNames = append(projectedVolumeNames, v.Name) + podSpec.Volumes[i] = apiv1.Volume{} + } + } + + for _, pv := range projectedVolumeNames { + for c := range podSpec.Containers { + for v, mount := range podSpec.Containers[c].VolumeMounts { + if mount.Name == pv { + podSpec.Containers[c].VolumeMounts[v] = apiv1.VolumeMount{} + } + } + } + } + + return podSpec +}