Add Configuration Support for the SiteConfig Operator

This commit introduces functionality to manage the SiteConfig Operator's configuration via a ConfigMap.

A new controller, `ConfigurationMonitor`, has been implemented to monitor and maintain the configuration. Key features include:
- Automatic creation of the ConfigMap with default values during manager pod initialization if it doesn't exist.
- Re-creation of the ConfigMap with default values if it is deleted.

The following configuration fields are supported:
- `AllowReinstalls`: Enables or disables the reinstall functionality.
- `MaxConcurrentReconciles`: Sets the maximum number of `ClusterInstances` that can be processed concurrently.

Unit tests have been added to validate the behavior of the new components.

Signed-off-by: Sharat Akhoury <[email protected]>

Makefile

@@ -3,7 +3,7 @@
 # To re-generate a bundle for another specific version without changing the standard setup, you can:
 # - use the VERSION as arg of the bundle target (e.g make bundle VERSION=0.0.2)
 # - use environment variables to overwrite this value (e.g export VERSION=0.0.2)
-VERSION ?= 2.12.0
+VERSION ?= 2.13.0
 
 # CHANNELS define the bundle channels used in the bundle.
 # Add a new line here if you would like to change its default config. (E.g CHANNELS = "candidate,fast,stable")

cmd/main.go

@@ -33,12 +33,14 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
 	ci "github.com/stolostron/siteconfig/internal/controller/clusterinstance"
+	"github.com/stolostron/siteconfig/internal/controller/configuration"
 	"github.com/stolostron/siteconfig/internal/controller/retry"
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -120,8 +122,9 @@ func main() {
 	}
 
 	// Check that the SiteConfig namespace value is defined
-	if getSiteConfigNamespace(setupLog) == "" {
-		setupLog.Error("Unable to retrieve the SiteConfig namespace")
+	siteConfigNamespace := configuration.GetPodNamespace(setupLog)
+	if siteConfigNamespace == "" {
+		setupLog.Error("Unable to retrieve the SiteConfig Operator namespace")
 		os.Exit(1)
 	}
 
@@ -131,18 +134,46 @@ func main() {
 		mgr.GetClient(),
 		setupLog,
 	); err != nil {
-		setupLog.Error("Unable to initialize the default reference install template ConfigMaps",
+		setupLog.Error("Unable to initialize the default reference installation template ConfigMaps",
 			zap.Error(err))
 		os.Exit(1)
 	}
 
+	// Create initial SiteConfig Operator configuration
+	operatorConfig, err := getSiteConfigConfiguration(context.TODO(), mgr.GetClient(), siteConfigNamespace, setupLog)
+	if err != nil {
+		setupLog.Error("Failed to process SiteConfig Operator configuration",
+			zap.Error(err))
+		os.Exit(1)
+	}
+	sharedConfigStore := configuration.NewConfigurationStore(operatorConfig)
+	if sharedConfigStore == nil {
+		setupLog.Error("Failed to initialize the ConfigurationStore for the SiteConfig Operator")
+		os.Exit(1)
+	}
+
+	// Create configuration monitor controller to track SiteConfig Operator configuration change(s)
+	if err = (&controller.ConfigurationMonitor{
+		Client:      mgr.GetClient(),
+		Log:         siteconfigLogger.Named("ConfigurationMonitor"),
+		Scheme:      mgr.GetScheme(),
+		ConfigStore: sharedConfigStore,
+	}).SetupWithManager(mgr); err != nil {
+		setupLog.Error("Unable to create controller",
+			zap.String("controller", "ConfigurationMonitor"),
+			zap.Error(err))
+		os.Exit(1)
+	}
+
+	// Create ClusterInstance controller for reconciling ClusterInstance CRs
 	clusterInstanceLogger := siteconfigLogger.Named("ClusterInstanceController")
 	if err = (&controller.ClusterInstanceReconciler{
-		Client:     mgr.GetClient(),
-		Scheme:     mgr.GetScheme(),
-		Recorder:   mgr.GetEventRecorderFor("ClusterInstanceController"),
-		Log:        clusterInstanceLogger,
-		TmplEngine: ci.NewTemplateEngine(),
+		Client:      mgr.GetClient(),
+		Scheme:      mgr.GetScheme(),
+		Recorder:    mgr.GetEventRecorderFor("ClusterInstanceController"),
+		Log:         clusterInstanceLogger,
+		TmplEngine:  ci.NewTemplateEngine(),
+		ConfigStore: sharedConfigStore,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error("Unable to create controller",
 			zap.String("controller", "ClusterInstance"),
@@ -151,6 +182,7 @@ func main() {
 		os.Exit(1)
 	}
 
+	// Create ClusterDeployment controller for monitoring cluster provisioning progress
 	if err = (&controller.ClusterDeploymentReconciler{
 		Client: mgr.GetClient(),
 		Log:    siteconfigLogger.Named("ClusterDeploymentReconciler"),
@@ -179,22 +211,14 @@ func main() {
 	}
 }
 
-func getSiteConfigNamespace(log *zap.Logger) string {
-	namespace := os.Getenv("POD_NAMESPACE")
-	if namespace == "" {
-		log.Info("POD_NAMESPACE environment variable is not defined")
-	}
-	return namespace
-}
-
 func initConfigMapTemplates(ctx context.Context, c client.Client, log *zap.Logger) error {
 	templates := make(map[string]map[string]string, 4)
 	templates[ai_templates.ClusterLevelInstallTemplates] = ai_templates.GetClusterTemplates()
 	templates[ai_templates.NodeLevelInstallTemplates] = ai_templates.GetNodeTemplates()
 	templates[ibi_templates.ClusterLevelInstallTemplates] = ibi_templates.GetClusterTemplates()
 	templates[ibi_templates.NodeLevelInstallTemplates] = ibi_templates.GetNodeTemplates()
 
-	siteConfigNamespace := getSiteConfigNamespace(log)
+	siteConfigNamespace := configuration.GetPodNamespace(log)
 
 	for k, v := range templates {
 		immutable := true
@@ -220,3 +244,22 @@ func initConfigMapTemplates(ctx context.Context, c client.Client, log *zap.Logge
 
 	return nil
 }
+
+func getSiteConfigConfiguration(
+	ctx context.Context,
+	c client.Client,
+	siteConfigNamespace string,
+	log *zap.Logger,
+) (*configuration.Configuration, error) {
+	config, err := configuration.LoadFromConfigMap(ctx, c, siteConfigNamespace)
+	if err != nil && k8serrors.IsNotFound(err) {
+		// Configuration CM not found, creating default
+		config, err = configuration.CreateDefaultConfigurationConfigMap(ctx, c, siteConfigNamespace)
+		if err != nil {
+			log.Error("Unable to create the default SiteConfig Operator configuration ConfigMap",
+				zap.Error(err))
+			return nil, err
+		}
+	}
+	return config, err
+}

cmd/main_test.go

@@ -18,6 +18,7 @@ package main
 
 import (
 	"context"
+	"reflect"
 	"testing"
 
 	"go.uber.org/zap"
@@ -29,6 +30,7 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"github.com/stolostron/siteconfig/internal/controller/configuration"
 	ai_templates "github.com/stolostron/siteconfig/internal/templates/assisted-installer"
 	ibi_templates "github.com/stolostron/siteconfig/internal/templates/image-based-installer"
 )
@@ -44,7 +46,7 @@ var _ = Describe("initConfigMapTemplates", func() {
 		c                   client.Client
 		ctx                 = context.Background()
 		testLogger          = zap.NewNop().Named("Test")
-		SiteConfigNamespace = getSiteConfigNamespace(testLogger)
+		SiteConfigNamespace = configuration.GetPodNamespace(testLogger)
 	)
 
 	BeforeEach(func() {
@@ -122,4 +124,77 @@ var _ = Describe("initConfigMapTemplates", func() {
 		Expect(c.Get(ctx, key, aiNodeCM)).To(Succeed())
 		Expect(aiNodeCM.Data).To(Equal(data))
 	})
+
+	It("creates a default SiteConfig Operator configuration ConfigMap on initialization if not created previously", func() {
+		initConfig, err := getSiteConfigConfiguration(ctx, c, SiteConfigNamespace, testLogger)
+		// Given that a configuration CM has not been created, the initConfig is expected to be the default configuration
+		Expect(err).ToNot(HaveOccurred())
+		Expect(reflect.DeepEqual(initConfig, configuration.NewDefaultConfiguration())).To(BeTrue())
+
+		// retrieve the configuration CM and verify that the data is correctly set to the default configuration
+		cm := &corev1.ConfigMap{}
+		key := types.NamespacedName{
+			Name:      configuration.SiteConfigOperatorConfigMap,
+			Namespace: SiteConfigNamespace,
+		}
+		Expect(c.Get(ctx, key, cm)).To(Succeed())
+		config, err := configuration.FromMap(cm.Data)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(reflect.DeepEqual(config, configuration.NewDefaultConfiguration())).To(BeTrue())
+	})
+
+	It("uses an existing SiteConfig Operator configuration ConfigMap on initialization", func() {
+		expectedConfig := &configuration.Configuration{
+			AllowReinstalls:         true,
+			MaxConcurrentReconciles: 10,
+		}
+		// create the configuration CM
+		key := types.NamespacedName{
+			Name:      configuration.SiteConfigOperatorConfigMap,
+			Namespace: SiteConfigNamespace,
+		}
+		cm := &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      key.Name,
+				Namespace: key.Namespace,
+			},
+			Data: expectedConfig.ToMap(),
+		}
+		Expect(c.Create(ctx, cm)).To(Succeed())
+
+		// retrieve the configuration CM and verify that the data is correctly set
+		actualConfig, err := getSiteConfigConfiguration(ctx, c, SiteConfigNamespace, testLogger)
+		// Given that a configuration CM has not been created, the initConfig is expected to be the default configuration
+		Expect(err).ToNot(HaveOccurred())
+		Expect(reflect.DeepEqual(actualConfig, expectedConfig)).To(BeTrue())
+
+		Expect(c.Get(ctx, key, cm)).To(Succeed())
+		gotConfig, err := configuration.FromMap(cm.Data)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(reflect.DeepEqual(gotConfig, expectedConfig)).To(BeTrue())
+	})
+
+	It("exits when encounters an error with the configuration object", func() {
+		// create the configuration CM
+		key := types.NamespacedName{
+			Name:      configuration.SiteConfigOperatorConfigMap,
+			Namespace: SiteConfigNamespace,
+		}
+		cm := &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      key.Name,
+				Namespace: key.Namespace,
+			},
+			Data: map[string]string{
+				"allowReinstalls":         "foobar",
+				"maxConcurrentReconciles": "1",
+			},
+		}
+		Expect(c.Create(ctx, cm)).To(Succeed())
+
+		// retrieve the configuration CM
+		_, err := getSiteConfigConfiguration(ctx, c, SiteConfigNamespace, testLogger)
+		// Given that a configuration CM has not been created, the initConfig is expected to be the default configuration
+		Expect(err).To(HaveOccurred())
+	})
 })

config/manager/kustomization.yaml

@@ -5,4 +5,4 @@ kind: Kustomization
 images:
 - name: controller
   newName: quay.io/stolostron/siteconfig-operator
-  newTag: 2.12.0
+  newTag: 2.13.0

internal/controller/clusterinstance/suite_test.go

@@ -33,7 +33,7 @@ import (
 	//+kubebuilder:scaffold:imports
 )
 
-func TestControllers(t *testing.T) {
+func TestClusterInstance(t *testing.T) {
 	RegisterFailHandler(Fail)
 
 	RunSpecs(t, "ClusterInstanceSuite")

internal/controller/clusterinstance_controller.go

@@ -26,6 +26,7 @@ import (
 
 	ci "github.com/stolostron/siteconfig/internal/controller/clusterinstance"
 	"github.com/stolostron/siteconfig/internal/controller/conditions"
+	"github.com/stolostron/siteconfig/internal/controller/configuration"
 	"go.uber.org/zap"
 	"golang.org/x/exp/maps"
 	corev1 "k8s.io/api/core/v1"
@@ -57,10 +58,11 @@ const (
 // ClusterInstanceReconciler reconciles a ClusterInstance object
 type ClusterInstanceReconciler struct {
 	client.Client
-	Scheme     *runtime.Scheme
-	Recorder   record.EventRecorder
-	Log        *zap.Logger
-	TmplEngine *ci.TemplateEngine
+	Scheme      *runtime.Scheme
+	Recorder    record.EventRecorder
+	Log         *zap.Logger
+	TmplEngine  *ci.TemplateEngine
+	ConfigStore *configuration.ConfigurationStore
 }
 
 func doNotRequeue() ctrl.Result {
@@ -134,6 +136,12 @@ func (r *ClusterInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		return doNotRequeue(), nil
 	}
 
+	if r.ConfigStore.GetConfiguration().AllowReinstalls {
+		log.Info("SiteConfig Operator is configured to allow reinstalls")
+	} else {
+		log.Info("SiteConfig Operator is not configured for reinstalls")
+	}
+
 	// Validate ClusterInstance
 	if err := r.handleValidate(ctx, log, clusterInstance); err != nil {
 		return requeueWithError(err)
@@ -825,6 +833,6 @@ func (r *ClusterInstanceReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&v1alpha1.ClusterInstance{}).
 		WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicate.LabelChangedPredicate{})).
-		WithOptions(controller.Options{MaxConcurrentReconciles: 1}).
+		WithOptions(controller.Options{MaxConcurrentReconciles: r.ConfigStore.GetConfiguration().MaxConcurrentReconciles}).
 		Complete(r)
 }

internal/controller/clusterinstance_controller_test.go

@@ -27,6 +27,7 @@ import (
 	hivev1 "github.com/openshift/hive/apis/hive/v1"
 	"github.com/stolostron/siteconfig/api/v1alpha1"
 	ci "github.com/stolostron/siteconfig/internal/controller/clusterinstance"
+	"github.com/stolostron/siteconfig/internal/controller/configuration"
 	ai_templates "github.com/stolostron/siteconfig/internal/templates/assisted-installer"
 	ibi_templates "github.com/stolostron/siteconfig/internal/templates/image-based-installer"
 	"go.uber.org/zap"
@@ -104,10 +105,11 @@ var _ = Describe("Reconcile", func() {
 			Build()
 
 		r = &ClusterInstanceReconciler{
-			Client:     c,
-			Scheme:     scheme.Scheme,
-			Log:        testLogger,
-			TmplEngine: ci.NewTemplateEngine(),
+			Client:      c,
+			Scheme:      scheme.Scheme,
+			Log:         testLogger,
+			TmplEngine:  ci.NewTemplateEngine(),
+			ConfigStore: configuration.NewConfigurationStore(configuration.NewDefaultConfiguration()),
 		}
 
 		Expect(c.Create(ctx, testParams.GeneratePullSecret())).To(Succeed())