WIP: raise an alert when the descheduler is not correctly configured

Descheduler is an optional operator and it's
not installed by default nor as a dependency of HCO.
When installed it can work on a cluster with
KubeVirt only if configured enabling
devEnableEvictionsInBackground profileCustomization
that is disabled by default.
HCO will check if the descheduler is there,
and if so it will check its configuration.
If the descheduler is missconfigured for
the KubeVirt use case, HCO will trigger
an alert making the cluster admin aware.
HCO is not directly amending the descheduler
configuration since it's an external independent
operator and directly controlling it is not a safe
practice (it could bring to infinite loops fighting
with other operators and so on).

Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>

cmd/hyperconverged-cluster-operator/main.go

@@ -12,6 +12,7 @@ import (
 	imagev1 "github.com/openshift/api/image/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	openshiftroutev1 "github.com/openshift/api/route/v1"
+	deschedulerv1 "github.com/openshift/cluster-kube-descheduler-operator/pkg/apis/descheduler/v1"
 	csvv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	operatorsapiv2 "github.com/operator-framework/api/pkg/operators/v2"
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
@@ -121,7 +122,7 @@ func main() {
 	needLeaderElection := !ci.IsRunningLocally()
 
 	// Create a new Cmd to provide shared dependencies and start components
-	mgr, err := manager.New(cfg, getManagerOptions(operatorNamespace, needLeaderElection, ci.IsMonitoringAvailable(), ci.IsOpenshift(), scheme))
+	mgr, err := manager.New(cfg, getManagerOptions(operatorNamespace, needLeaderElection, ci.IsMonitoringAvailable(), ci.IsDeschedulerAvailable(), ci.IsOpenshift(), scheme))
 	cmdHelper.ExitOnError(err, "can't initiate manager")
 
 	// register pprof instrumentation if HCO_PPROF_ADDR is set
@@ -203,7 +204,7 @@ func main() {
 
 // Restricts the cache's ListWatch to specific fields/labels per GVK at the specified object to control the memory impact
 // this is used to completely overwrite the NewCache function so all the interesting objects should be explicitly listed here
-func getCacheOption(operatorNamespace string, isMonitoringAvailable, isOpenshift bool) cache.Options {
+func getCacheOption(operatorNamespace string, isMonitoringAvailable, isDeschedulerAvailable, isOpenshift bool) cache.Options {
 	namespaceSelector := fields.Set{"metadata.namespace": operatorNamespace}.AsSelector()
 	labelSelector := labels.Set{hcoutil.AppLabel: hcoutil.HyperConvergedName}.AsSelector()
 	labelSelectorForNamespace := labels.Set{hcoutil.KubernetesMetadataName: operatorNamespace}.AsSelector()
@@ -257,6 +258,10 @@ func getCacheOption(operatorNamespace string, isMonitoringAvailable, isOpenshift
 		},
 	}
 
+	cacheOptionsByObjectForDescheduler := map[client.Object]cache.ByObject{
+		&deschedulerv1.KubeDescheduler{}: {},
+	}
+
 	cacheOptionsByObjectForOpenshift := map[client.Object]cache.ByObject{
 		&openshiftroutev1.Route{}: {
 			Namespaces: map[string]cache.Config{
@@ -282,6 +287,9 @@ func getCacheOption(operatorNamespace string, isMonitoringAvailable, isOpenshift
 	if isMonitoringAvailable {
 		maps.Copy(cacheOptions.ByObject, cacheOptionsByObjectForMonitoring)
 	}
+	if isDeschedulerAvailable {
+		maps.Copy(cacheOptions.ByObject, cacheOptionsByObjectForDescheduler)
+	}
 	if isOpenshift {
 		maps.Copy(cacheOptions.ByObject, cacheOptionsByObjectForOpenshift)
 	}
@@ -290,7 +298,7 @@ func getCacheOption(operatorNamespace string, isMonitoringAvailable, isOpenshift
 
 }
 
-func getManagerOptions(operatorNamespace string, needLeaderElection, isMonitoringAvailable, isOpenshift bool, scheme *apiruntime.Scheme) manager.Options {
+func getManagerOptions(operatorNamespace string, needLeaderElection, isMonitoringAvailable, isDeschedulerAvailable, isOpenshift bool, scheme *apiruntime.Scheme) manager.Options {
 	return manager.Options{
 		Metrics: server.Options{
 			BindAddress: fmt.Sprintf("%s:%d", hcoutil.MetricsHost, hcoutil.MetricsPort),
@@ -305,7 +313,7 @@ func getManagerOptions(operatorNamespace string, needLeaderElection, isMonitorin
 		// "configmapsleases". Therefore, having only "leases" should be safe now.
 		LeaderElectionResourceLock: resourcelock.LeasesResourceLock,
 		LeaderElectionID:           "hyperconverged-cluster-operator-lock",
-		Cache:                      getCacheOption(operatorNamespace, isMonitoringAvailable, isOpenshift),
+		Cache:                      getCacheOption(operatorNamespace, isMonitoringAvailable, isDeschedulerAvailable, isOpenshift),
 		Scheme:                     scheme,
 	}
 }

controllers/commontestutils/testUtils.go

@@ -307,6 +307,12 @@ func (c ClusterInfoMock) IsConsolePluginImageProvided() bool {
 func (c ClusterInfoMock) IsMonitoringAvailable() bool {
 	return true
 }
+func (c ClusterInfoMock) IsDeschedulerAvailable() bool {
+	return true
+}
+func (c ClusterInfoMock) IsDeschedulerMissconfigured() bool {
+	return false
+}
 func (c ClusterInfoMock) IsSingleStackIPv6() bool {
 	return true
 }
@@ -330,6 +336,9 @@ func (ClusterInfoMock) GetTLSSecurityProfile(_ *openshiftconfigv1.TLSSecurityPro
 func (ClusterInfoMock) RefreshAPIServerCR(_ context.Context, _ client.Client) error {
 	return nil
 }
+func (ClusterInfoMock) RefreshDeschedulerCR(_ context.Context, _ client.Client) error {
+	return nil
+}
 
 // ClusterInfoSNOMock mocks Openshift SNO
 type ClusterInfoSNOMock struct{}
@@ -378,12 +387,21 @@ func (ClusterInfoSNOMock) GetTLSSecurityProfile(_ *openshiftconfigv1.TLSSecurity
 func (ClusterInfoSNOMock) RefreshAPIServerCR(_ context.Context, _ client.Client) error {
 	return nil
 }
+func (ClusterInfoSNOMock) RefreshDeschedulerCR(_ context.Context, _ client.Client) error {
+	return nil
+}
 func (ClusterInfoSNOMock) IsConsolePluginImageProvided() bool {
 	return true
 }
 func (c ClusterInfoSNOMock) IsMonitoringAvailable() bool {
 	return true
 }
+func (c ClusterInfoSNOMock) IsDeschedulerAvailable() bool {
+	return true
+}
+func (c ClusterInfoSNOMock) IsDeschedulerMissconfigured() bool {
+	return false
+}
 func (c ClusterInfoSNOMock) IsSingleStackIPv6() bool {
 	return true
 }
@@ -432,7 +450,13 @@ func (ClusterInfoSRCPHAIMock) IsConsolePluginImageProvided() bool {
 func (ClusterInfoSRCPHAIMock) IsMonitoringAvailable() bool {
 	return true
 }
-func (m ClusterInfoSRCPHAIMock) IsSingleStackIPv6() bool {
+func (ClusterInfoSRCPHAIMock) IsDeschedulerAvailable() bool {
+	return true
+}
+func (ClusterInfoSRCPHAIMock) IsDeschedulerMissconfigured() bool {
+	return false
+}
+func (ClusterInfoSRCPHAIMock) IsSingleStackIPv6() bool {
 	return true
 }
 func (ClusterInfoSRCPHAIMock) GetTLSSecurityProfile(_ *openshiftconfigv1.TLSSecurityProfile) *openshiftconfigv1.TLSSecurityProfile {
@@ -444,6 +468,9 @@ func (ClusterInfoSRCPHAIMock) GetTLSSecurityProfile(_ *openshiftconfigv1.TLSSecu
 func (ClusterInfoSRCPHAIMock) RefreshAPIServerCR(_ context.Context, _ client.Client) error {
 	return nil
 }
+func (ClusterInfoSRCPHAIMock) RefreshDeschedulerCR(_ context.Context, _ client.Client) error {
+	return nil
+}
 
 func KeysFromSSMap(ssmap map[string]string) gstruct.Keys {
 	keys := gstruct.Keys{}

controllers/hyperconverged/hyperconverged_controller.go

@@ -17,6 +17,7 @@ import (
 	consolev1 "github.com/openshift/api/console/v1"
 	imagev1 "github.com/openshift/api/image/v1"
 	routev1 "github.com/openshift/api/route/v1"
+	deschedulerv1 "github.com/openshift/cluster-kube-descheduler-operator/pkg/apis/descheduler/v1"
 	operatorhandler "github.com/operator-framework/operator-lib/handler"
 	"github.com/pkg/errors"
 	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
@@ -82,9 +83,10 @@ const (
 	systemHealthStatusWarning   = "warning"
 	systemHealthStatusError     = "error"
 
-	hcoVersionName    = "operator"
-	secondaryCRPrefix = "hco-controlled-cr-"
-	apiServerCRPrefix = "api-server-cr-"
+	hcoVersionName      = "operator"
+	secondaryCRPrefix   = "hco-controlled-cr-"
+	apiServerCRPrefix   = "api-server-cr-"
+	deschedulerCRPrefix = "descheduler-cr-"
 
 	// These group are no longer supported. Use these constants to remove unused resources
 	v2vGroup = "v2v.kubevirt.io"
@@ -233,6 +235,11 @@ func add(mgr manager.Manager, r reconcile.Reconciler, ci hcoutil.ClusterInfo) er
 		return err
 	}
 
+	deschedulerCRPlaceholder, err := getDeschedulerCRPlaceholder()
+	if err != nil {
+		return err
+	}
+
 	if ci.IsOpenshift() {
 		// Watch openshiftconfigv1.APIServer separately
 		msg := "Reconciling for openshiftconfigv1.APIServer"
@@ -255,6 +262,30 @@ func add(mgr manager.Manager, r reconcile.Reconciler, ci hcoutil.ClusterInfo) er
 			return err
 		}
 	}
+
+	if ci.IsDeschedulerAvailable() {
+		// Watch KubeDescheduler separately
+		msg := "Reconciling for KubeDescheduler"
+
+		err = c.Watch(
+			source.Kind(
+				mgr.GetCache(),
+				client.Object(&deschedulerv1.KubeDescheduler{}),
+				handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, a client.Object) []reconcile.Request {
+					// enqueue using a placeholder to signal that the change is not
+					// directly on HCO CR but on the Descheduler CR that we want to reload
+					// only if really changed
+					log.Info(msg)
+					return []reconcile.Request{
+						{NamespacedName: deschedulerCRPlaceholder},
+					}
+				}),
+			))
+		if err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 
@@ -345,6 +376,12 @@ func (r *ReconcileHyperConverged) Reconcile(ctx context.Context, request reconci
 		result.Requeue = true
 	}
 
+	// TODO: if ci.IsDeschedulerAvailable() == false,
+	// double check if the CRD appeared when HCO was already running
+	// in that case HCO pod should restart to correctly configure the client caache,
+	// see:
+	// https://github.com/kubernetes-sigs/controller-runtime/issues/2456
+
 	return result, err
 }
 
@@ -384,6 +421,21 @@ func (r *ReconcileHyperConverged) resolveReconcileRequest(ctx context.Context, l
 		return resolvedRequest, hcoTriggered, nil
 	}
 
+	deschedulerCRTriggered, err := isTriggeredByDeschedulerCR(originalRequest)
+	if err != nil {
+		return reconcile.Request{}, hcoTriggered, err
+	}
+	if deschedulerCRTriggered {
+		logger.Info("Triggered by Descheduler CR, refreshing it")
+		err = hcoutil.GetClusterInfo().RefreshAPIServerCR(ctx, r.client)
+		if err != nil {
+			return reconcile.Request{}, hcoTriggered, err
+		}
+		metrics.SetHCOMetricMissconfiguredDescheduler(hcoutil.GetClusterInfo().IsDeschedulerMissconfigured())
+		hcoTriggered = false
+		return resolvedRequest, hcoTriggered, nil
+	}
+
 	logger.Info("The reconciliation got triggered by a secondary CR object")
 	return resolvedRequest, hcoTriggered, nil
 }
@@ -411,6 +463,15 @@ func isTriggeredByAPIServerCR(request reconcile.Request) (bool, error) {
 	return request.NamespacedName == placeholder, nil
 }
 
+func isTriggeredByDeschedulerCR(request reconcile.Request) (bool, error) {
+	placeholder, err := getDeschedulerCRPlaceholder()
+	if err != nil {
+		return false, err
+	}
+
+	return request.NamespacedName == placeholder, nil
+}
+
 func (r *ReconcileHyperConverged) doReconcile(req *common.HcoRequest) (reconcile.Result, error) {
 
 	valid, err := r.validateNamespace(req)
@@ -1451,6 +1512,20 @@ func getAPIServerCRPlaceholder() (types.NamespacedName, error) {
 	return fakeHco, nil
 }
 
+func getDeschedulerCRPlaceholder() (types.NamespacedName, error) {
+	fakeHco := types.NamespacedName{
+		Name: deschedulerCRPrefix + randomConstSuffix,
+	}
+
+	namespace, err := hcoutil.GetOperatorNamespaceFromEnv()
+	if err != nil {
+		return fakeHco, err
+	}
+	fakeHco.Namespace = namespace
+
+	return fakeHco, nil
+}
+
 func drop(slice []string, s string) ([]string, bool) {
 	var newSlice []string
 	dropped := false

deploy/cluster_role.yaml

@@ -984,6 +984,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - operator.openshift.io
+  resources:
+  - kubedeschedulers
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - config.openshift.io
   resources:

deploy/index-image/community-kubevirt-hyperconverged/1.14.0/manifests/kubevirt-hyperconverged-operator.v1.14.0.clusterserviceversion.yaml

@@ -454,6 +454,14 @@ spec:
           - get
           - list
           - watch
+        - apiGroups:
+          - operator.openshift.io
+          resources:
+          - kubedeschedulers
+          verbs:
+          - get
+          - list
+          - watch
         - apiGroups:
           - config.openshift.io
           resources:

deploy/olm-catalog/community-kubevirt-hyperconverged/1.14.0/manifests/kubevirt-hyperconverged-operator.v1.14.0.clusterserviceversion.yaml

@@ -9,7 +9,7 @@ metadata:
     certified: "false"
     console.openshift.io/disable-operand-delete: "true"
     containerImage: quay.io/kubevirt/hyperconverged-cluster-operator:1.14.0-unstable
-    createdAt: "2024-09-12 05:05:22"
+    createdAt: "2024-09-17 19:01:48"
     description: A unified operator deploying and controlling KubeVirt and its supporting
       operators with opinionated defaults
     features.operators.openshift.io/cnf: "false"
@@ -454,6 +454,14 @@ spec:
           - get
           - list
           - watch
+        - apiGroups:
+          - operator.openshift.io
+          resources:
+          - kubedeschedulers
+          verbs:
+          - get
+          - list
+          - watch
         - apiGroups:
           - config.openshift.io
           resources:

docs/metrics.md

@@ -9,6 +9,9 @@ Monitors resources for potential problems. Type: Gauge.
 ### kubevirt_hco_hyperconverged_cr_exists
 Indicates whether the HyperConverged custom resource exists (1) or not (0). Type: Gauge.
 
+### kubevirt_hco_missconfigured_descheduler
+Indicates whether the optional descheduler is not properly configured (1) to work with KubeVirt or not (0). Type: Gauge.
+
 ### kubevirt_hco_out_of_band_modifications_total
 Count of out-of-band modifications overwritten by HCO. Type: Counter.
 

go.mod

@@ -1,6 +1,6 @@
 module github.com/kubevirt/hyperconverged-cluster-operator
 
-go 1.22.4
+go 1.22.5
 
 require (
 	dario.cat/mergo v1.0.1
@@ -16,6 +16,7 @@ require (
 	github.com/onsi/ginkgo/v2 v2.20.2
 	github.com/onsi/gomega v1.34.2
 	github.com/openshift/api v3.9.1-0.20190517100836-d5b34b957e91+incompatible
+	github.com/openshift/cluster-kube-descheduler-operator v0.0.0-20240916113608-1a30f3be33fa
 	github.com/openshift/custom-resource-status v1.1.2
 	github.com/openshift/library-go v0.0.0-20240830130947-d9523164b328
 	github.com/operator-framework/api v0.27.0

go.sum

@@ -227,6 +227,8 @@ github.com/onsi/gomega v1.34.2 h1:pNCwDkzrsv7MS9kpaQvVb1aVLahQXyJ/Tv5oAZMI3i8=
 github.com/onsi/gomega v1.34.2/go.mod h1:v1xfxRgk0KIsG+QOdm7p8UosrOzPYRo60fd3B/1Dukc=
 github.com/openshift/api v0.0.0-20230503133300-8bbcb7ca7183 h1:t/CahSnpqY46sQR01SoS+Jt0jtjgmhgE6lFmRnO4q70=
 github.com/openshift/api v0.0.0-20230503133300-8bbcb7ca7183/go.mod h1:4VWG+W22wrB4HfBL88P40DxLEpSOaiBVxUnfalfJo9k=
+github.com/openshift/cluster-kube-descheduler-operator v0.0.0-20240916113608-1a30f3be33fa h1:bLljfvA1G/n60SmMCkrV9ydlUX711WTYIXEO/JbMZQw=
+github.com/openshift/cluster-kube-descheduler-operator v0.0.0-20240916113608-1a30f3be33fa/go.mod h1:vq3hTEopoGcGHa64WQ1LTbyjDqKQhSGNgj9NOIu6nK4=
 github.com/openshift/custom-resource-status v1.1.2 h1:C3DL44LEbvlbItfd8mT5jWrqPfHnSOQoQf/sypqA6A4=
 github.com/openshift/custom-resource-status v1.1.2/go.mod h1:DB/Mf2oTeiAmVVX1gN+NEqweonAPY0TKUwADizj8+ZA=
 github.com/openshift/library-go v0.0.0-20240830130947-d9523164b328 h1:CEEgSC4+IVv/DbVv0Mbnx1cu5iOB5W2e7UuKGuHxcXs=

pkg/components/components.go

@@ -469,6 +469,7 @@ var (
 
 func GetClusterPermissions() []rbacv1.PolicyRule {
 	const configOpenshiftIO = "config.openshift.io"
+	const operatorOpenshiftIO = "operator.openshift.io"
 	return []rbacv1.PolicyRule{
 		{
 			APIGroups: stringListToSlice(util.APIVersionGroup),
@@ -555,6 +556,11 @@ func GetClusterPermissions() []rbacv1.PolicyRule {
 			Resources: stringListToSlice("apiservers"),
 			Verbs:     stringListToSlice("get", "list", "watch"),
 		},
+		{
+			APIGroups: stringListToSlice(operatorOpenshiftIO),
+			Resources: stringListToSlice("kubedeschedulers"),
+			Verbs:     stringListToSlice("get", "list", "watch"),
+		},
 		{
 			APIGroups: stringListToSlice(configOpenshiftIO),
 			Resources: stringListToSlice("dnses"),