Merge pull request kubernetes#4979 from sibucan/sort-vpa-containerRec…

…ommendations

Sort the list of container resource recommendations in VPA status

vertical-pod-autoscaler/pkg/recommender/routines/recommender.go

@@ -19,6 +19,7 @@ package routines
 import (
 	"context"
 	"flag"
+	"sort"
 	"time"
 
 	vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
@@ -144,13 +145,22 @@ func (r *recommender) UpdateVPAs() {
 func getCappedRecommendation(vpaID model.VpaID, resources logic.RecommendedPodResources,
 	policy *vpa_types.PodResourcePolicy) *vpa_types.RecommendedPodResources {
 	containerResources := make([]vpa_types.RecommendedContainerResources, 0, len(resources))
-	for containerName, res := range resources {
+	// Sort the container names from the map. This is because maps are an
+	// unordered data structure, and iterating through the map will return
+	// a different order on every call.
+	containerNames := make([]string, 0, len(resources))
+	for containerName := range resources {
+		containerNames = append(containerNames, containerName)
+	}
+	sort.Strings(containerNames)
+	// Create the list of recommendations for each container.
+	for _, name := range containerNames {
 		containerResources = append(containerResources, vpa_types.RecommendedContainerResources{
-			ContainerName:  containerName,
-			Target:         model.ResourcesAsResourceList(res.Target),
-			LowerBound:     model.ResourcesAsResourceList(res.LowerBound),
-			UpperBound:     model.ResourcesAsResourceList(res.UpperBound),
-			UncappedTarget: model.ResourcesAsResourceList(res.Target),
+			ContainerName:  name,
+			Target:         model.ResourcesAsResourceList(resources[name].Target),
+			LowerBound:     model.ResourcesAsResourceList(resources[name].LowerBound),
+			UpperBound:     model.ResourcesAsResourceList(resources[name].UpperBound),
+			UncappedTarget: model.ResourcesAsResourceList(resources[name].Target),
 		})
 	}
 	recommendation := &vpa_types.RecommendedPodResources{

vertical-pod-autoscaler/pkg/recommender/routines/recommender_test.go

@@ -0,0 +1,78 @@
+/*
+Copyright 2022 The Kubernetes 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 routines
+
+import (
+	"testing"
+	"time"
+
+	labels "k8s.io/apimachinery/pkg/labels"
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/logic"
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/recommender/model"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSortedRecommendation(t *testing.T) {
+	cases := []struct {
+		name         string
+		resources    logic.RecommendedPodResources
+		expectedLast []string
+	}{
+		{
+			name: "All recommendations sorted",
+			resources: logic.RecommendedPodResources{
+				"a-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}},
+				"b-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}},
+				"c-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}},
+				"d-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}},
+			},
+			expectedLast: []string{
+				"a-container",
+				"b-container",
+				"c-container",
+				"d-container",
+			},
+		},
+		{
+			name: "All recommendations unsorted",
+			resources: logic.RecommendedPodResources{
+				"b-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}},
+				"a-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}},
+				"d-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}},
+				"c-container": logic.RecommendedContainerResources{Target: model.Resources{model.ResourceCPU: model.CPUAmountFromCores(1), model.ResourceMemory: model.MemoryAmountFromBytes(1000)}},
+			},
+			expectedLast: []string{
+				"a-container",
+				"b-container",
+				"c-container",
+				"d-container",
+			},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			namespace := "test-namespace"
+			vpa := model.NewVpa(model.VpaID{Namespace: namespace, VpaName: "my-vpa"}, labels.Nothing(), time.Unix(0, 0))
+			vpa.UpdateRecommendation(getCappedRecommendation(vpa.ID, tc.resources, nil))
+			// Check that the slice is in the correct order.
+			for i := range vpa.Recommendation.ContainerRecommendations {
+				assert.Equal(t, tc.expectedLast[i], vpa.Recommendation.ContainerRecommendations[i].ContainerName)
+			}
+		})
+	}
+}