cleanup + restructure of config and setup logic (#102)

Rework APIs and basic units to simplify codeflare operator integration.

Author: dgrove-oss

cmd/main.go

@@ -135,20 +135,27 @@ func main() {
 
 	certsReady := make(chan struct{})
 
-	if *cfg.WebhooksEnabled {
+	if ptr.Deref(cfg.WebhooksEnabled, false) {
 		exitOnError(controller.SetupCertManagement(mgr, cfg.CertManagement, certsReady), "Unable to set up cert rotation")
 	} else {
 		close(certsReady)
 	}
 
-	// Ascynchronous because controllers need to wait for certificate to be ready for webhooks to work
-	go controller.SetupControllers(ctx, mgr, cfg.AppWrapper, *cfg.WebhooksEnabled, certsReady, setupLog)
+	go func() {
+		setupLog.Info("Waiting for certificates to be generated")
+		<-certsReady
+		setupLog.Info("Certs ready")
+		if ptr.Deref(cfg.WebhooksEnabled, false) {
+			exitOnError(controller.SetupWebhooks(ctx, mgr, cfg.AppWrapper), "unable to configure webhook")
+		}
+		exitOnError(controller.SetupControllers(ctx, mgr, cfg.AppWrapper), "unable to start controllers")
+	}()
 
 	exitOnError(controller.SetupIndexers(ctx, mgr, cfg.AppWrapper), "unable to setup indexers")
 	exitOnError(controller.SetupProbeEndpoints(mgr, certsReady), "unable to setup probe endpoints")
 
 	setupLog.Info("starting manager")
-	exitOnError(mgr.Start(ctx), "problem running manager")
+	exitOnError(mgr.Start(ctx), "problem starting manager")
 }
 
 func getNamespace() (string, error) {

config/default/config.yaml

@@ -4,6 +4,9 @@ metadata:
   name: operator-config
 data:
   config.yaml: |
+    appwrapper:
+      enableKueueIntegrations: true
+      manageJobsWithoutQueueName: true
     controllerManager:
       health:
         bindAddress: ":8081"

config/dev/config.yaml

@@ -4,6 +4,9 @@ metadata:
   name: operator-config
 data:
   config.yaml: |
+    appwrapper:
+      enableKueueIntegrations: true
+      manageJobsWithoutQueueName: true
     controllerManager:
       health:
         bindAddress: "localhost:0"

config/standalone/config.yaml

@@ -5,7 +5,7 @@ metadata:
 data:
   config.yaml: |
     appwrapper:
-      standaloneMode: true
+      enableKueueIntegrations: false
       manageJobsWithoutQueueName: false
     controllerManager:
       health:

go.mod

@@ -3,7 +3,6 @@ module github.com/project-codeflare/appwrapper
 go 1.21
 
 require (
-	github.com/go-logr/logr v1.4.1
 	github.com/onsi/ginkgo/v2 v2.16.0
 	github.com/onsi/gomega v1.31.1
 	github.com/open-policy-agent/cert-controller v0.10.1
@@ -24,6 +23,7 @@ require (
 	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
 	github.com/evanphx/json-patch/v5 v5.8.0 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
+	github.com/go-logr/logr v1.4.1 // indirect
 	github.com/go-logr/zapr v1.3.0 // indirect
 	github.com/go-openapi/jsonpointer v0.20.0 // indirect
 	github.com/go-openapi/jsonreference v0.20.2 // indirect

internal/controller/appwrapper/resource_management.go

@@ -80,16 +80,16 @@ func (r *AppWrapperReconciler) createComponent(ctx context.Context, aw *workload
 	if err != nil {
 		return nil, err, true
 	}
-	if r.Config.StandaloneMode {
-		obj.SetLabels(utilmaps.MergeKeepFirst(obj.GetLabels(), map[string]string{AppWrapperLabel: aw.Name}))
-	} else {
+	if r.Config.EnableKueueIntegrations {
 		obj.SetLabels(utilmaps.MergeKeepFirst(obj.GetLabels(), map[string]string{AppWrapperLabel: aw.Name, constants.QueueLabel: childJobQueueName}))
+	} else {
+		obj.SetLabels(utilmaps.MergeKeepFirst(obj.GetLabels(), map[string]string{AppWrapperLabel: aw.Name}))
 	}
 
 	awLabels := map[string]string{AppWrapperLabel: aw.Name}
 	for podSetsIdx, podSet := range component.PodSets {
 		toInject := &workloadv1beta2.AppWrapperPodSetInfo{}
-		if !r.Config.StandaloneMode {
+		if r.Config.EnableKueueIntegrations {
 			if podSetsIdx < len(component.PodSetInfos) {
 				toInject = &component.PodSetInfos[podSetsIdx]
 			} else {

internal/webhook/appwrapper_webhook.go

@@ -59,7 +59,7 @@ var _ webhook.CustomDefaulter = &AppWrapperWebhook{}
 func (w *AppWrapperWebhook) Default(ctx context.Context, obj runtime.Object) error {
 	aw := obj.(*workloadv1beta2.AppWrapper)
 	log.FromContext(ctx).Info("Applying defaults", "job", aw)
-	if !w.Config.StandaloneMode {
+	if w.Config.EnableKueueIntegrations {
 		jobframework.ApplyDefaultForSuspend((*wlc.AppWrapper)(aw), w.Config.ManageJobsWithoutQueueName)
 	}
 	if err := expandPodSets(ctx, aw); err != nil {
@@ -78,7 +78,7 @@ func (w *AppWrapperWebhook) ValidateCreate(ctx context.Context, obj runtime.Obje
 	aw := obj.(*workloadv1beta2.AppWrapper)
 	log.FromContext(ctx).Info("Validating create", "job", aw)
 	allErrors := w.validateAppWrapperCreate(ctx, aw)
-	if !w.Config.StandaloneMode {
+	if w.Config.EnableKueueIntegrations {
 		allErrors = append(allErrors, jobframework.ValidateCreateForQueueName((*wlc.AppWrapper)(aw))...)
 	}
 	return nil, allErrors.ToAggregate()
@@ -90,7 +90,7 @@ func (w *AppWrapperWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj r
 	newAW := newObj.(*workloadv1beta2.AppWrapper)
 	log.FromContext(ctx).Info("Validating update", "job", newAW)
 	allErrors := w.validateAppWrapperUpdate(oldAW, newAW)
-	if !w.Config.StandaloneMode {
+	if w.Config.EnableKueueIntegrations {
 		allErrors = append(allErrors, jobframework.ValidateUpdateForQueueName((*wlc.AppWrapper)(oldAW), (*wlc.AppWrapper)(newAW))...)
 		allErrors = append(allErrors, jobframework.ValidateUpdateForWorkloadPriorityClassName((*wlc.AppWrapper)(oldAW), (*wlc.AppWrapper)(newAW))...)
 	}

pkg/config/config.go

@@ -30,7 +30,7 @@ type OperatorConfig struct {
 
 type AppWrapperConfig struct {
 	ManageJobsWithoutQueueName bool                  `json:"manageJobsWithoutQueueName,omitempty"`
-	StandaloneMode             bool                  `json:"standaloneMode,omitempty"`
+	EnableKueueIntegrations    bool                  `json:"enableKueueIntegrations,omitempty"`
 	FaultTolerance             *FaultToleranceConfig `json:"faultTolerance,omitempty"`
 }
 
@@ -74,7 +74,7 @@ type HealthConfiguration struct {
 func NewAppWrapperConfig() *AppWrapperConfig {
 	return &AppWrapperConfig{
 		ManageJobsWithoutQueueName: true,
-		StandaloneMode:             false,
+		EnableKueueIntegrations:    true,
 		FaultTolerance: &FaultToleranceConfig{
 			WarmupGracePeriod:   5 * time.Minute,
 			FailureGracePeriod:  1 * time.Minute,

pkg/controller/setup.go

@@ -21,13 +21,11 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
-	"os"
 
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 
-	"github.com/go-logr/logr"
 	cert "github.com/open-policy-agent/cert-controller/pkg/rotator"
 
 	"github.com/project-codeflare/appwrapper/internal/controller/appwrapper"
@@ -39,29 +37,21 @@ import (
 )
 
 // SetupControllers creates and configures all components of the AppWrapper controller
-func SetupControllers(ctx context.Context, mgr ctrl.Manager, awConfig *config.AppWrapperConfig,
-	webhooksEnabled bool, certsReady chan struct{}, log logr.Logger) {
-
-	log.Info("Waiting for certificates to be generated")
-	<-certsReady
-	log.Info("Certs ready")
-
-	if !awConfig.StandaloneMode {
+func SetupControllers(ctx context.Context, mgr ctrl.Manager, awConfig *config.AppWrapperConfig) error {
+	if awConfig.EnableKueueIntegrations {
 		if err := workload.WorkloadReconciler(
 			mgr.GetClient(),
 			mgr.GetEventRecorderFor("kueue"),
 			jobframework.WithManageJobsWithoutQueueName(awConfig.ManageJobsWithoutQueueName),
 		).SetupWithManager(mgr); err != nil {
-			log.Error(err, "Failed to create workload controller")
-			os.Exit(1)
+			return fmt.Errorf("workload controller: %w", err)
 		}
 
 		if err := (&workload.ChildWorkloadReconciler{
 			Client: mgr.GetClient(),
 			Scheme: mgr.GetScheme(),
 		}).SetupWithManager(mgr); err != nil {
-			log.Error(err, "Failed to create child admission controller")
-			os.Exit(1)
+			return fmt.Errorf("child admission controller: %w", err)
 		}
 	}
 
@@ -70,22 +60,24 @@ func SetupControllers(ctx context.Context, mgr ctrl.Manager, awConfig *config.Ap
 		Scheme: mgr.GetScheme(),
 		Config: awConfig,
 	}).SetupWithManager(mgr); err != nil {
-		log.Error(err, "Failed to create appwrapper controller")
-		os.Exit(1)
+		return fmt.Errorf("appwrapper controller: %w", err)
 	}
 
-	if webhooksEnabled {
-		if err := (&webhook.AppWrapperWebhook{
-			Config: awConfig,
-		}).SetupWebhookWithManager(mgr); err != nil {
-			log.Error(err, "Failed to create webhook")
-			os.Exit(1)
-		}
+	return nil
+}
+
+// SetupWebhooks creates and configures the AppWrapper controller's Webhooks
+func SetupWebhooks(ctx context.Context, mgr ctrl.Manager, awConfig *config.AppWrapperConfig) error {
+	if err := (&webhook.AppWrapperWebhook{
+		Config: awConfig,
+	}).SetupWebhookWithManager(mgr); err != nil {
+		return fmt.Errorf("webhook: %w", err)
 	}
+	return nil
 }
 
 func SetupIndexers(ctx context.Context, mgr ctrl.Manager, awConfig *config.AppWrapperConfig) error {
-	if !awConfig.StandaloneMode {
+	if awConfig.EnableKueueIntegrations {
 		if err := jobframework.SetupWorkloadOwnerIndex(ctx, mgr.GetFieldIndexer(), workload.GVK); err != nil {
 			return fmt.Errorf("workload indexer: %w", err)
 		}