Fix VPA for latest prom-operator (#1497)

* Fix VPA for latest prom-operator * fix forgotten verticalpodautoscaler.Config (#1506) * fix forgotten verticalpodautoscaler.Config * move a const to a more suitable place --------- Co-authored-by: Herve Nicol <[email protected]> * cleanup VPA test * Fix VPA for latest prom-operator (#1511) * Fix VPA for latest prom-operator * Update CHANGELOG.md --------- Co-authored-by: Hervé Nicol <[email protected]> Co-authored-by: Herve Nicol <[email protected]>
giantswarm · Feb 7, 2024 · 7367dca · 7367dca
1 parent b935237
commit 7367dca
Show file tree

Hide file tree

Showing 5 changed files with 126 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix VPA to support latest Prometheus-operator version (based on observability-bundle 1.2.0) as the latest version of the Prometheus CR now supports the `scale` subresource which causes issues with VPA.
+
 ## [4.66.0] - 2024-02-06
 
 ### Changed

diff --git a/service/controller/clusterapi/resource.go b/service/controller/clusterapi/resource.go
@@ -267,9 +267,11 @@ func New(config Config) ([]resource.Interface, error) {
 	var verticalPodAutoScalerResource resource.Interface
 	{
 		c := verticalpodautoscaler.Config{
-			Logger:    config.Logger,
-			K8sClient: config.K8sClient,
-			VpaClient: config.VpaClient,
+			Logger:       config.Logger,
+			K8sClient:    config.K8sClient,
+			VpaClient:    config.VpaClient,
+			Installation: config.Installation,
+			Provider:     config.Provider,
 		}
 
 		verticalPodAutoScalerResource, err = verticalpodautoscaler.New(c)

diff --git a/service/controller/managementcluster/resource.go b/service/controller/managementcluster/resource.go
@@ -197,9 +197,11 @@ func newResources(config resourcesConfig) ([]resource.Interface, error) {
 	var verticalPodAutoScalerResource resource.Interface
 	{
 		c := verticalpodautoscaler.Config{
-			Logger:    config.Logger,
-			K8sClient: config.K8sClient,
-			VpaClient: config.VpaClient,
+			Logger:       config.Logger,
+			K8sClient:    config.K8sClient,
+			VpaClient:    config.VpaClient,
+			Installation: config.Installation,
+			Provider:     config.Provider,
 		}
 
 		verticalPodAutoScalerResource, err = verticalpodautoscaler.New(c)

diff --git a/service/controller/resource/monitoring/verticalpodautoscaler/resource.go b/service/controller/resource/monitoring/verticalpodautoscaler/resource.go
@@ -2,35 +2,48 @@ package verticalpodautoscaler
 
 import (
 	"context"
+	"fmt"
 
+	"github.com/blang/semver"
+	appsv1alpha1 "github.com/giantswarm/apiextensions-application/api/v1alpha1"
 	"github.com/giantswarm/k8sclient/v7/pkg/k8sclient"
 	"github.com/giantswarm/microerror"
 	"github.com/giantswarm/micrologger"
 	autoscaling "k8s.io/api/autoscaling/v1"
 	v1 "k8s.io/api/core/v1"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
 	vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned"
 
+	"github.com/giantswarm/prometheus-meta-operator/v2/pkg/cluster"
 	"github.com/giantswarm/prometheus-meta-operator/v2/service/key"
 )
 
 const (
-	Name = "verticalpodautoscaler"
+	Name                              = "verticalpodautoscaler"
+	unknownObservabilityBundleVersion = "0.0.0"
 )
 
 type Config struct {
 	K8sClient k8sclient.Interface
 	VpaClient vpa_clientset.Interface
 	Logger    micrologger.Logger
+
+	Installation string
+	Provider     cluster.Provider
 }
 
 type Resource struct {
 	k8sClient k8sclient.Interface
 	vpaClient vpa_clientset.Interface
 	logger    micrologger.Logger
+
+	installation string
+	provider     cluster.Provider
 }
 
 func New(config Config) (*Resource, error) {
@@ -44,10 +57,16 @@ func New(config Config) (*Resource, error) {
 		return nil, microerror.Maskf(invalidConfigError, "%T.Logger must not be empty", config)
 	}
 
+	if config.Installation == "" {
+		return nil, microerror.Maskf(invalidConfigError, "%T.Installation must not be empty", config)
+	}
+
 	r := &Resource{
-		k8sClient: config.K8sClient,
-		vpaClient: config.VpaClient,
-		logger:    config.Logger,
+		k8sClient:    config.K8sClient,
+		vpaClient:    config.VpaClient,
+		logger:       config.Logger,
+		installation: config.Installation,
+		provider:     config.Provider,
 	}
 
 	return r, nil
@@ -103,11 +122,6 @@ func (r *Resource) getObject(ctx context.Context, v interface{}) (*vpa_types.Ver
 	vpa := &vpa_types.VerticalPodAutoscaler{
 		ObjectMeta: objectMeta,
 		Spec: vpa_types.VerticalPodAutoscalerSpec{
-			TargetRef: &autoscaling.CrossVersionObjectReference{
-				Kind:       "StatefulSet",
-				Name:       key.PrometheusSTSName(cluster),
-				APIVersion: "apps/v1",
-			},
 			UpdatePolicy: &vpa_types.PodUpdatePolicy{
 				UpdateMode: &updateModeAuto,
 			},
@@ -131,9 +145,65 @@ func (r *Resource) getObject(ctx context.Context, v interface{}) (*vpa_types.Ver
 		},
 	}
 
+	mcObservabilityBundleAppVersion, err := r.getManagementClusterObservabilityBundleAppVersion(ctx, cluster)
+	if err != nil {
+		return nil, microerror.Mask(err)
+	}
+
+	version, err := semver.Parse(mcObservabilityBundleAppVersion)
+	if err != nil {
+		return nil, microerror.Mask(err)
+	}
+	if version.LT(semver.MustParse("1.2.0")) {
+		// Set target reference to Prometheus StatefulSet
+		vpa.Spec.TargetRef = &autoscaling.CrossVersionObjectReference{
+			Kind:       "StatefulSet",
+			Name:       key.PrometheusSTSName(cluster),
+			APIVersion: "apps/v1",
+		}
+	} else {
+		// Set target reference to Prometheus CR
+		vpa.Spec.TargetRef = &autoscaling.CrossVersionObjectReference{
+			Kind:       "Prometheus",
+			Name:       key.ClusterID(cluster),
+			APIVersion: "monitoring.coreos.com/v1",
+		}
+
+	}
+
 	return vpa, nil
 }
 
+// We get the management cluster observability bundle app version for the management cluster to know if we need to configure the VPA CR to target the StatefulSet or the Prometheus CR.
+// We need to do this because VPA uses the scale subresource to scale the target object, and the Prometheus CR has a scale subresource since the observability-bundle 1.2.0.
+func (r *Resource) getManagementClusterObservabilityBundleAppVersion(ctx context.Context, cluster metav1.Object) (string, error) {
+	var appName string
+	var appNamespace string
+	if key.IsCAPIManagementCluster(r.provider) {
+		appName = fmt.Sprintf("%s-observability-bundle", r.installation)
+		appNamespace = "org-giantswarm"
+	} else {
+		appName = "observability-bundle"
+		appNamespace = "giantswarm"
+	}
+
+	app := &appsv1alpha1.App{}
+	objectKey := types.NamespacedName{Namespace: appNamespace, Name: appName}
+	err := r.k8sClient.CtrlClient().Get(ctx, objectKey, app)
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			return unknownObservabilityBundleVersion, nil
+		}
+		return "", err
+	}
+
+	if app.Status.Version != "" {
+		return app.Status.Version, nil
+	}
+	// This avoids a race condition where the app is created for the cluster but not deployed.
+	return unknownObservabilityBundleVersion, nil
+}
+
 func (r *Resource) listWorkerNodes(ctx context.Context) (*v1.NodeList, error) {
 	// Selects only worker nodes
 	selector := "node-role.kubernetes.io/control-plane!="

diff --git a/service/controller/resource/monitoring/verticalpodautoscaler/resource_test.go b/service/controller/resource/monitoring/verticalpodautoscaler/resource_test.go
@@ -6,15 +6,18 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/giantswarm/micrologger"
-
+	appsv1alpha1 "github.com/giantswarm/apiextensions-application/api/v1alpha1"
 	"github.com/giantswarm/k8sclient/v7/pkg/k8sclient"
 	k8sclientfake "github.com/giantswarm/k8sclient/v7/pkg/k8sclient/fake"
+	"github.com/giantswarm/micrologger"
 	v1 "k8s.io/api/core/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
 	vpa_clientsetfake "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned/fake"
+	"k8s.io/client-go/kubernetes/scheme"
 
+	"github.com/giantswarm/prometheus-meta-operator/v2/pkg/cluster"
 	"github.com/giantswarm/prometheus-meta-operator/v2/pkg/unittest"
 )
 
@@ -46,34 +49,49 @@ func TestVerticalPodAutoScaler(t *testing.T) {
 
 	var k8sClient k8sclient.Interface
 	{
+		schemeBuilder := runtime.SchemeBuilder(k8sclient.SchemeBuilder{
+			apiextensionsv1.AddToScheme,
+			appsv1alpha1.AddToScheme,
+		})
+
+		err = schemeBuilder.AddToScheme(scheme.Scheme)
+		if err != nil {
+			t.Fatal(err)
+		}
 		c := k8sclient.ClientsConfig{
 			Logger:        logger,
-			SchemeBuilder: k8sclient.SchemeBuilder(v1.SchemeBuilder),
+			SchemeBuilder: k8sclient.SchemeBuilder(schemeBuilder),
 		}
 		k8sClient, err = k8sclientfake.NewClients(c, node)
 		if err != nil {
 			t.Fatal(err)
 		}
 	}
-	testFunc := func(v interface{}) (interface{}, error) {
-		c := Config{
-			Logger:    logger,
-			K8sClient: k8sClient,
-			VpaClient: vpa_clientsetfake.NewSimpleClientset(),
-		}
-		r, err := New(c)
-		if err != nil {
-			return nil, err
-		}
-		return r.getObject(context.Background(), v)
-	}
 
 	for _, flavor := range unittest.ProviderFlavors {
 		outputDir, err := filepath.Abs("./test/" + flavor)
 		if err != nil {
 			t.Fatal(err)
 		}
 
+		testFunc := func(v interface{}) (interface{}, error) {
+			c := Config{
+				Logger:    logger,
+				K8sClient: k8sClient,
+				VpaClient: vpa_clientsetfake.NewSimpleClientset(),
+				Provider: cluster.Provider{
+					Kind:   "aws",
+					Flavor: flavor,
+				},
+				Installation: "test-installation",
+			}
+			r, err := New(c)
+			if err != nil {
+				return nil, err
+			}
+			return r.getObject(context.Background(), v)
+		}
+
 		c := unittest.Config{
 			Flavor:    flavor,
 			OutputDir: outputDir,