diff --git a/internal/controller/platform/standalone/nimservice.go b/internal/controller/platform/standalone/nimservice.go index 62dc4dc4b..1b3b61707 100644 --- a/internal/controller/platform/standalone/nimservice.go +++ b/internal/controller/platform/standalone/nimservice.go @@ -28,7 +28,7 @@ import ( "github.com/go-logr/logr" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" appsv1 "k8s.io/api/apps/v1" - autoscalingv1 "k8s.io/api/autoscaling/v1" + autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -125,7 +125,7 @@ func (r *NIMServiceReconciler) reconcileNIMService(ctx context.Context, nimServi // Sync HPA if nimService.IsAutoScalingEnabled() { - err = r.renderAndSyncResource(ctx, nimService, &renderer, &autoscalingv1.HorizontalPodAutoscaler{}, func() (client.Object, error) { + err = r.renderAndSyncResource(ctx, nimService, &renderer, &autoscalingv2.HorizontalPodAutoscaler{}, func() (client.Object, error) { return renderer.HPA(nimService.GetHPAParams()) }, "hpa", conditions.ReasonHPAFailed) if err != nil { diff --git a/internal/controller/platform/standalone/nimservice_test.go b/internal/controller/platform/standalone/nimservice_test.go index 6c1cf49b6..3ea9d89fe 100644 --- a/internal/controller/platform/standalone/nimservice_test.go +++ b/internal/controller/platform/standalone/nimservice_test.go @@ -32,7 +32,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" - autoscalingv1 "k8s.io/api/autoscaling/v1" + autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -79,7 +79,7 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() { Expect(appsv1alpha1.AddToScheme(scheme)).To(Succeed()) Expect(appsv1.AddToScheme(scheme)).To(Succeed()) Expect(rbacv1.AddToScheme(scheme)).To(Succeed()) - Expect(autoscalingv1.AddToScheme(scheme)).To(Succeed()) + Expect(autoscalingv2.AddToScheme(scheme)).To(Succeed()) Expect(networkingv1.AddToScheme(scheme)).To(Succeed()) Expect(corev1.AddToScheme(scheme)).To(Succeed()) @@ -99,6 +99,7 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() { renderer: render.NewRenderer(path.Join(strings.TrimSuffix(cwd, "internal/controller/platform/standalone"), "manifests")), } pvcName := "test-pvc" + minReplicas := int32(1) nimService = &appsv1alpha1.NIMService{ ObjectMeta: metav1.ObjectMeta{ Name: "test-nimservice", @@ -172,7 +173,31 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() { Effect: corev1.TaintEffectNoSchedule, }, }, - Scale: appsv1alpha1.Autoscaling{Enabled: ptr.To[bool](false)}, + Scale: appsv1alpha1.Autoscaling{ + Enabled: ptr.To[bool](true), + HPA: appsv1alpha1.HorizontalPodAutoscalerSpec{ + MinReplicas: &minReplicas, + MaxReplicas: 10, + Metrics: []autoscalingv2.MetricSpec{ + { + Type: autoscalingv2.ResourceMetricSourceType, + Resource: &autoscalingv2.ResourceMetricSource{ + Target: autoscalingv2.MetricTarget{ + Type: autoscalingv2.UtilizationMetricType, + }, + }, + }, + { + Type: autoscalingv2.PodsMetricSourceType, + Pods: &autoscalingv2.PodsMetricSource{ + Target: autoscalingv2.MetricTarget{ + Type: autoscalingv2.UtilizationMetricType, + }, + }, + }, + }, + }, + }, ReadinessProbe: appsv1alpha1.Probe{ Enabled: &boolTrue, Probe: &corev1.Probe{ @@ -323,6 +348,15 @@ var _ = Describe("NIMServiceReconciler for a standalone platform", func() { Expect(service.Name).To(Equal(nimService.GetName())) Expect(service.Namespace).To(Equal(nimService.GetNamespace())) + // HPA should be deployed + hpa := &autoscalingv2.HorizontalPodAutoscaler{} + err = client.Get(context.TODO(), namespacedName, hpa) + Expect(err).NotTo(HaveOccurred()) + Expect(hpa.Name).To(Equal(nimService.GetName())) + Expect(hpa.Namespace).To(Equal(nimService.GetNamespace())) + Expect(*hpa.Spec.MinReplicas).To(Equal(int32(1))) + Expect(hpa.Spec.MaxReplicas).To(Equal(int32(10))) + // Deployment should be created deployment := &appsv1.Deployment{} err = client.Get(context.TODO(), namespacedName, deployment) diff --git a/internal/render/render.go b/internal/render/render.go index 6b7d4a642..bfa140c3a 100644 --- a/internal/render/render.go +++ b/internal/render/render.go @@ -348,6 +348,8 @@ func (r *textTemplateRenderer) HPA(params *types.HPAParams) (*autoscalingv2.Hori if err != nil { return nil, fmt.Errorf("error converting unstructured object to HPA: %w", err) } + // Sorting to avoid unnecessary object updates + hpa.Spec.Metrics = utils.SortHPAMetricsSpec(hpa.Spec.Metrics) return hpa, nil } diff --git a/internal/render/render_test.go b/internal/render/render_test.go index b40d62d3b..c67bebcdd 100644 --- a/internal/render/render_test.go +++ b/internal/render/render_test.go @@ -360,5 +360,50 @@ var _ = Describe("K8s Resources Rendering", func() { Expect(*hpa.Spec.MinReplicas).To(Equal(int32(1))) Expect(hpa.Spec.MaxReplicas).To(Equal(int32(10))) }) + + It("should render HPA template and sort metrics spec correctly", func() { + minRep := int32(1) + params := types.HPAParams{ + Enabled: true, + Name: "test-hpa", + Namespace: "default", + HPASpec: autoscalingv2.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv2.CrossVersionObjectReference{ + Name: "test-deployment", + Kind: "Deployment", + APIVersion: "apps/v1", + }, + MinReplicas: &minRep, + MaxReplicas: 10, + Metrics: []autoscalingv2.MetricSpec{ + { + Type: autoscalingv2.ResourceMetricSourceType, + Resource: &autoscalingv2.ResourceMetricSource{ + Target: autoscalingv2.MetricTarget{ + Type: autoscalingv2.UtilizationMetricType, + }, + }, + }, + { + Type: autoscalingv2.PodsMetricSourceType, + Pods: &autoscalingv2.PodsMetricSource{ + Target: autoscalingv2.MetricTarget{ + Type: autoscalingv2.UtilizationMetricType, + }, + }, + }, + }, + }, + } + r := render.NewRenderer(templatesDir) + hpa, err := r.HPA(¶ms) + Expect(err).NotTo(HaveOccurred()) + Expect(hpa.Name).To(Equal("test-hpa")) + Expect(hpa.Namespace).To(Equal("default")) + Expect(*hpa.Spec.MinReplicas).To(Equal(int32(1))) + Expect(hpa.Spec.MaxReplicas).To(Equal(int32(10))) + Expect(hpa.Spec.Metrics[0].Type).To(Equal(autoscalingv2.PodsMetricSourceType)) + Expect(hpa.Spec.Metrics[1].Type).To(Equal(autoscalingv2.ResourceMetricSourceType)) + }) }) }) diff --git a/internal/utils/utils.go b/internal/utils/utils.go index 2b1622eb4..c8dd1229b 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -23,12 +23,13 @@ import ( "hash/fnv" "os" "path/filepath" + "reflect" "sort" "strings" + autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/rand" - "reflect" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -131,13 +132,30 @@ func SortKeys(obj interface{}) interface{} { } return sortedMap case []interface{}: - // Check if the slice contains maps and sort them if so + // Check if the slice contains maps and sort them by the "name" field or the first available field if len(obj) > 0 { + if _, ok := obj[0].(map[string]interface{}); ok { - sort.Slice(obj, func(i, j int) bool { - iName := obj[i].(map[string]interface{})["name"].(string) - jName := obj[j].(map[string]interface{})["name"].(string) - return iName < jName + sort.SliceStable(obj, func(i, j int) bool { + iMap, iOk := obj[i].(map[string]interface{}) + jMap, jOk := obj[j].(map[string]interface{}) + if iOk && jOk { + // Try to sort by "name" if present + iName, iNameOk := iMap["name"].(string) + jName, jNameOk := jMap["name"].(string) + if iNameOk && jNameOk { + return iName < jName + } + + // If "name" is not available, sort by the first key in each map + if len(iMap) > 0 && len(jMap) > 0 { + iFirstKey := firstKey(iMap) + jFirstKey := firstKey(jMap) + return iFirstKey < jFirstKey + } + } + // If no valid comparison is possible, maintain the original order + return false }) } } @@ -148,6 +166,16 @@ func SortKeys(obj interface{}) interface{} { return obj } +// Helper function to get the first key of a map (alphabetically sorted) +func firstKey(m map[string]interface{}) string { + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys[0] +} + // GetResourceHash returns a consistent hash for the given object spec func GetResourceHash(obj client.Object) string { // Convert obj to a map[string]interface{} @@ -242,3 +270,12 @@ func IsEqual[T client.Object](existing, desired T, fieldsToCompare ...string) bo return true } + +func SortHPAMetricsSpec(metrics []autoscalingv2.MetricSpec) []autoscalingv2.MetricSpec { + sort.Slice(metrics, func(i, j int) bool { + iMetricsType := metrics[i].Type + jMetricsType := metrics[j].Type + return iMetricsType < jMetricsType + }) + return metrics +}