Specify watch namespace (#48)

* Add watch namespace CLI arg Signed-off-by: Atanas Dinov <[email protected]> * Propagate plan namespace and name to Helm chart resources Signed-off-by: Atanas Dinov <[email protected]> * Add kube-system and cattle-system to cache when watch namespace is specified Signed-off-by: Atanas Dinov <[email protected]> * Set watch namespace for the deployment Signed-off-by: Atanas Dinov <[email protected]> * Adjust namespaces of samples Signed-off-by: Atanas Dinov <[email protected]> * Drop controller references Signed-off-by: Atanas Dinov <[email protected]> * Remove event recording wrapper Signed-off-by: Atanas Dinov <[email protected]> * Unify object creation Signed-off-by: Atanas Dinov <[email protected]> * Add comment explaining the order of execution for GetObjectKind Signed-off-by: Atanas Dinov <[email protected]> * Linter fix Signed-off-by: Atanas Dinov <[email protected]> * Drop LeaderElectionNamespace setting Signed-off-by: Atanas Dinov <[email protected]> --------- Signed-off-by: Atanas Dinov <[email protected]>
suse-edge · Aug 16, 2024 · cdcf828 · cdcf828
1 parent 6b60c47
commit cdcf828
Show file tree

Hide file tree

Showing 13 changed files with 117 additions and 94 deletions.
diff --git a/cmd/main.go b/cmd/main.go
@@ -29,6 +29,7 @@ import (
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
@@ -38,6 +39,7 @@ import (
 	upgradecattlev1 "github.com/rancher/system-upgrade-controller/pkg/apis/upgrade.cattle.io/v1"
 	lifecyclev1alpha1 "github.com/suse-edge/upgrade-controller/api/v1alpha1"
 	"github.com/suse-edge/upgrade-controller/internal/controller"
+	"github.com/suse-edge/upgrade-controller/internal/upgrade"
 	// +kubebuilder:scaffold:imports
 )
 
@@ -61,6 +63,8 @@ func main() {
 	var probeAddr string
 	var secureMetrics bool
 	var enableHTTP2 bool
+	var watchNamespace string
+
 	flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metric endpoint binds to. "+
 		"Use the port :8080. If not set, it will be 0 in order to disable the metrics server")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
@@ -71,6 +75,8 @@ func main() {
 		"If set the metrics endpoint is served securely")
 	flag.BoolVar(&enableHTTP2, "enable-http2", false,
 		"If set, HTTP/2 will be enabled for the metrics and webhook servers")
+	flag.StringVar(&watchNamespace, "namespace", os.Getenv("WATCH_NAMESPACE"),
+		"Namespace that the controller watches to reconcile resources.")
 	opts := zap.Options{
 		Development: true,
 	}
@@ -99,6 +105,15 @@ func main() {
 		TLSOpts: tlsOpts,
 	})
 
+	var watchNamespaces map[string]cache.Config
+	if watchNamespace != "" {
+		watchNamespaces = map[string]cache.Config{
+			watchNamespace:             {},
+			upgrade.HelmChartNamespace: {},
+			upgrade.SUCNamespace:       {},
+		}
+	}
+
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Scheme: scheme,
 		Metrics: metricsserver.Options{
@@ -121,6 +136,9 @@ func main() {
 		// if you are doing or is intended to do any operation such as perform cleanups
 		// after the manager stops then its usage might be unsafe.
 		// LeaderElectionReleaseOnCancel: true,
+		Cache: cache.Options{
+			DefaultNamespaces: watchNamespaces,
+		},
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")

diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml
@@ -65,6 +65,11 @@ spec:
           - --health-probe-bind-address=:8081
         image: controller:latest
         name: manager
+        env:
+          - name: WATCH_NAMESPACE
+            valueFrom:
+              fieldRef:
+                fieldPath: metadata.namespace
         securityContext:
           allowPrivilegeEscalation: false
           capabilities:

diff --git a/config/samples/lifecycle_v1alpha1_releasemanifest.yaml b/config/samples/lifecycle_v1alpha1_releasemanifest.yaml
@@ -5,7 +5,7 @@ metadata:
     app.kubernetes.io/name: upgrade-controller
     app.kubernetes.io/managed-by: kustomize
   name: releasemanifest-sample
-  namespace: cattle-system
+  namespace: upgrade-controller-system
 spec:
   releaseVersion: 3.0.1
   components:

diff --git a/config/samples/lifecycle_v1alpha1_upgradeplan.yaml b/config/samples/lifecycle_v1alpha1_upgradeplan.yaml
@@ -5,6 +5,6 @@ metadata:
     app.kubernetes.io/name: upgrade-controller
     app.kubernetes.io/managed-by: kustomize
   name: upgradeplan-sample
-  namespace: cattle-system
+  namespace: upgrade-controller-system
 spec:
   releaseVersion: 3.0.1
diff --git a/internal/controller/helm.go b/internal/controller/helm.go
@@ -72,7 +72,8 @@ func (r *UpgradePlanReconciler) updateHelmChart(ctx context.Context, upgradePlan
 	if chart.Annotations == nil {
 		chart.Annotations = map[string]string{}
 	}
-	chart.Annotations[upgrade.PlanAnnotation] = upgradePlan.Name
+	chart.Annotations[upgrade.PlanNameAnnotation] = upgradePlan.Name
+	chart.Annotations[upgrade.PlanNamespaceAnnotation] = upgradePlan.Namespace
 	chart.Annotations[upgrade.ReleaseAnnotation] = upgradePlan.Spec.ReleaseVersion
 	chart.Spec.ChartContent = ""
 	chart.Spec.Chart = releaseChart.Name
@@ -98,18 +99,18 @@ func (r *UpgradePlanReconciler) createHelmChart(ctx context.Context, upgradePlan
 		}
 	}
 
+	annotations := upgrade.PlanIdentifierAnnotations(upgradePlan.Name, upgradePlan.Namespace)
+	annotations[upgrade.ReleaseAnnotation] = upgradePlan.Spec.ReleaseVersion
+
 	chart := &helmcattlev1.HelmChart{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "HelmChart",
 			APIVersion: "helm.cattle.io/v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      installedChart.Name,
-			Namespace: upgrade.ChartNamespace,
-			Annotations: map[string]string{
-				upgrade.PlanAnnotation:    upgradePlan.Name,
-				upgrade.ReleaseAnnotation: upgradePlan.Spec.ReleaseVersion,
-			},
+			Name:        installedChart.Name,
+			Namespace:   upgrade.HelmChartNamespace,
+			Annotations: annotations,
 		},
 		Spec: helmcattlev1.HelmChartSpec{
 			Chart:           releaseChart.Name,
@@ -121,7 +122,7 @@ func (r *UpgradePlanReconciler) createHelmChart(ctx context.Context, upgradePlan
 		},
 	}
 
-	return r.Create(ctx, chart)
+	return r.createObject(ctx, upgradePlan, chart)
 }
 
 func (r *UpgradePlanReconciler) upgradeHelmChart(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, releaseChart *lifecyclev1alpha1.HelmChart) (upgrade.HelmChartState, error) {
@@ -157,7 +158,7 @@ func (r *UpgradePlanReconciler) upgradeHelmChart(ctx context.Context, upgradePla
 	}
 
 	job := &batchv1.Job{}
-	if err = r.Get(ctx, types.NamespacedName{Name: chart.Status.JobName, Namespace: upgrade.ChartNamespace}, job); err != nil {
+	if err = r.Get(ctx, types.NamespacedName{Name: chart.Status.JobName, Namespace: upgrade.HelmChartNamespace}, job); err != nil {
 		return upgrade.ChartStateUnknown, client.IgnoreNotFound(err)
 	}
 

diff --git a/internal/controller/reconcile_helm.go b/internal/controller/reconcile_helm.go
@@ -55,14 +55,14 @@ func (r *UpgradePlanReconciler) reconcileHelmChart(ctx context.Context, upgradeP
 
 			switch addonState {
 			case upgrade.ChartStateFailed:
-				msg := fmt.Sprintf("'%s' upgraded successfully, but add-on component '%s' failed to upgrade", chart.ReleaseName, addonChart.ReleaseName)
-				r.recordPlanEvent(upgradePlan, corev1.EventTypeWarning, conditionType, msg)
+				r.Recorder.Eventf(upgradePlan, corev1.EventTypeWarning, conditionType,
+					"'%s' upgraded successfully, but add-on component '%s' failed to upgrade", chart.ReleaseName, addonChart.ReleaseName)
 			case upgrade.ChartStateNotInstalled:
-				msg := fmt.Sprintf("'%s' add-on component upgrade skipped as it is missing in the cluster", addonChart.ReleaseName)
-				r.recordPlanEvent(upgradePlan, corev1.EventTypeNormal, conditionType, msg)
+				r.Recorder.Eventf(upgradePlan, corev1.EventTypeNormal, conditionType,
+					"'%s' add-on component upgrade skipped as it is missing in the cluster", addonChart.ReleaseName)
 			case upgrade.ChartStateSucceeded:
-				msg := fmt.Sprintf("'%s' add-on component successfully upgraded", addonChart.ReleaseName)
-				r.recordPlanEvent(upgradePlan, corev1.EventTypeNormal, conditionType, msg)
+				r.Recorder.Eventf(upgradePlan, corev1.EventTypeNormal, conditionType,
+					"'%s' add-on component successfully upgraded", addonChart.ReleaseName)
 			case upgrade.ChartStateInProgress:
 				// mark that current add-on chart upgrade is in progress
 				setInProgressCondition(upgradePlan, conditionType, addonState.FormattedMessage(addonChart.ReleaseName))

diff --git a/internal/controller/reconcile_kubernetes.go b/internal/controller/reconcile_kubernetes.go
@@ -26,15 +26,16 @@ func (r *UpgradePlanReconciler) reconcileKubernetes(ctx context.Context, upgrade
 		return ctrl.Result{}, fmt.Errorf("identifying target kubernetes version: %w", err)
 	}
 
+	identifierAnnotations := upgrade.PlanIdentifierAnnotations(upgradePlan.Name, upgradePlan.Namespace)
 	drainControlPlane, drainWorker := parseDrainOptions(upgradePlan)
-	controlPlanePlan := upgrade.KubernetesControlPlanePlan(kubernetesVersion, drainControlPlane)
+	controlPlanePlan := upgrade.KubernetesControlPlanePlan(kubernetesVersion, drainControlPlane, identifierAnnotations)
 	if err = r.Get(ctx, client.ObjectKeyFromObject(controlPlanePlan), controlPlanePlan); err != nil {
 		if !errors.IsNotFound(err) {
 			return ctrl.Result{}, err
 		}
 
 		setInProgressCondition(upgradePlan, lifecyclev1alpha1.KubernetesUpgradedCondition, "Control plane nodes are being upgraded")
-		return ctrl.Result{}, r.createPlan(ctx, upgradePlan, controlPlanePlan)
+		return ctrl.Result{}, r.createObject(ctx, upgradePlan, controlPlanePlan)
 	}
 
 	selector, err := metav1.LabelSelectorAsSelector(controlPlanePlan.Spec.NodeSelector)
@@ -50,14 +51,14 @@ func (r *UpgradePlanReconciler) reconcileKubernetes(ctx context.Context, upgrade
 		return ctrl.Result{Requeue: true}, nil
 	}
 
-	workerPlan := upgrade.KubernetesWorkerPlan(kubernetesVersion, drainWorker)
+	workerPlan := upgrade.KubernetesWorkerPlan(kubernetesVersion, drainWorker, identifierAnnotations)
 	if err = r.Get(ctx, client.ObjectKeyFromObject(workerPlan), workerPlan); err != nil {
 		if !errors.IsNotFound(err) {
 			return ctrl.Result{}, err
 		}
 
 		setInProgressCondition(upgradePlan, lifecyclev1alpha1.KubernetesUpgradedCondition, "Worker nodes are being upgraded")
-		return ctrl.Result{}, r.createPlan(ctx, upgradePlan, workerPlan)
+		return ctrl.Result{}, r.createObject(ctx, upgradePlan, workerPlan)
 	}
 
 	selector, err = metav1.LabelSelectorAsSelector(workerPlan.Spec.NodeSelector)

diff --git a/internal/controller/reconcile_os.go b/internal/controller/reconcile_os.go
@@ -15,7 +15,9 @@ import (
 )
 
 func (r *UpgradePlanReconciler) reconcileOS(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, releaseVersion string, releaseOS *lifecyclev1alpha1.OperatingSystem) (ctrl.Result, error) {
-	secret, err := upgrade.OSUpgradeSecret(releaseOS)
+	identifierAnnotations := upgrade.PlanIdentifierAnnotations(upgradePlan.Name, upgradePlan.Namespace)
+
+	secret, err := upgrade.OSUpgradeSecret(releaseOS, identifierAnnotations)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("generating OS upgrade secret: %w", err)
 	}
@@ -25,18 +27,18 @@ func (r *UpgradePlanReconciler) reconcileOS(ctx context.Context, upgradePlan *li
 			return ctrl.Result{}, err
 		}
 
-		return ctrl.Result{}, r.createSecret(ctx, upgradePlan, secret)
+		return ctrl.Result{}, r.createObject(ctx, upgradePlan, secret)
 	}
 
 	drainControlPlane, drainWorker := parseDrainOptions(upgradePlan)
-	controlPlanePlan := upgrade.OSControlPlanePlan(releaseVersion, secret.Name, releaseOS, drainControlPlane)
+	controlPlanePlan := upgrade.OSControlPlanePlan(releaseVersion, secret.Name, releaseOS, drainControlPlane, identifierAnnotations)
 	if err = r.Get(ctx, client.ObjectKeyFromObject(controlPlanePlan), controlPlanePlan); err != nil {
 		if !errors.IsNotFound(err) {
 			return ctrl.Result{}, err
 		}
 
 		setInProgressCondition(upgradePlan, lifecyclev1alpha1.OperatingSystemUpgradedCondition, "Control plane nodes are being upgraded")
-		return ctrl.Result{}, r.createPlan(ctx, upgradePlan, controlPlanePlan)
+		return ctrl.Result{}, r.createObject(ctx, upgradePlan, controlPlanePlan)
 	}
 
 	selector, err := metav1.LabelSelectorAsSelector(controlPlanePlan.Spec.NodeSelector)
@@ -57,14 +59,14 @@ func (r *UpgradePlanReconciler) reconcileOS(ctx context.Context, upgradePlan *li
 		return ctrl.Result{Requeue: true}, nil
 	}
 
-	workerPlan := upgrade.OSWorkerPlan(releaseVersion, secret.Name, releaseOS, drainWorker)
+	workerPlan := upgrade.OSWorkerPlan(releaseVersion, secret.Name, releaseOS, drainWorker, identifierAnnotations)
 	if err = r.Get(ctx, client.ObjectKeyFromObject(workerPlan), workerPlan); err != nil {
 		if !errors.IsNotFound(err) {
 			return ctrl.Result{}, err
 		}
 
 		setInProgressCondition(upgradePlan, lifecyclev1alpha1.OperatingSystemUpgradedCondition, "Worker nodes are being upgraded")
-		return ctrl.Result{}, r.createPlan(ctx, upgradePlan, workerPlan)
+		return ctrl.Result{}, r.createObject(ctx, upgradePlan, workerPlan)
 	}
 
 	selector, err = metav1.LabelSelectorAsSelector(workerPlan.Spec.NodeSelector)

diff --git a/internal/controller/upgradeplan_controller.go b/internal/controller/upgradeplan_controller.go
@@ -33,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -137,40 +138,20 @@ func (r *UpgradePlanReconciler) executePlan(ctx context.Context, upgradePlan *li
 	return ctrl.Result{}, nil
 }
 
-func (r *UpgradePlanReconciler) createSecret(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, secret *corev1.Secret) error {
-	if err := r.createObject(ctx, upgradePlan, secret); err != nil {
-		return fmt.Errorf("creating secret: %w", err)
-	}
+func (r *UpgradePlanReconciler) createObject(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, object client.Object) error {
+	// Extract the kind first since the data of the object pointer is modified during creation.
+	kind := object.GetObjectKind().GroupVersionKind().Kind
 
-	r.recordPlanEvent(upgradePlan, corev1.EventTypeNormal, "SecretCreated", fmt.Sprintf("Secret created: %s/%s", secret.Namespace, secret.Name))
-	return nil
-}
-
-func (r *UpgradePlanReconciler) createPlan(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, plan *upgradecattlev1.Plan) error {
-	if err := r.createObject(ctx, upgradePlan, plan); err != nil {
-		return fmt.Errorf("creating upgrade plan: %w", err)
+	if err := r.Create(ctx, object); err != nil {
+		return err
 	}
 
-	r.recordPlanEvent(upgradePlan, corev1.EventTypeNormal, "PlanCreated", fmt.Sprintf("Upgrade plan created: %s/%s", plan.Namespace, plan.Name))
+	reason := fmt.Sprintf("%sCreated", kind)
+	r.Recorder.Eventf(upgradePlan, corev1.EventTypeNormal, reason,
+		"%s created: %s/%s", kind, object.GetNamespace(), object.GetName())
 	return nil
 }
 
-func (r *UpgradePlanReconciler) createObject(ctx context.Context, upgradePlan *lifecyclev1alpha1.UpgradePlan, obj client.Object) error {
-	if err := ctrl.SetControllerReference(upgradePlan, obj, r.Scheme); err != nil {
-		return fmt.Errorf("setting controller reference: %w", err)
-	}
-
-	if err := r.Create(ctx, obj); err != nil {
-		return fmt.Errorf("creating object: %w", err)
-	}
-
-	return nil
-}
-
-func (r *UpgradePlanReconciler) recordPlanEvent(upgradePlan *lifecyclev1alpha1.UpgradePlan, eventType, reason, msg string) {
-	r.Recorder.Eventf(upgradePlan, eventType, reason, msg)
-}
-
 func isHelmUpgradeFinished(plan *lifecyclev1alpha1.UpgradePlan, conditionType string) bool {
 	condition := meta.FindStatusCondition(plan.Status.Conditions, conditionType)
 
@@ -257,14 +238,22 @@ func (r *UpgradePlanReconciler) findUpgradePlanFromJob(ctx context.Context, job
 		return []reconcile.Request{}
 	}
 
-	planName, ok := helmChart.Annotations[upgrade.PlanAnnotation]
+	return r.findUpgradePlanFromAnnotations(ctx, helmChart)
+}
+
+func (r *UpgradePlanReconciler) findUpgradePlanFromAnnotations(_ context.Context, object client.Object) []reconcile.Request {
+	annotations := object.GetAnnotations()
+
+	planName, ok := annotations[upgrade.PlanNameAnnotation]
 	if !ok || planName == "" {
-		// Helm chart is not managed by the Upgrade controller.
+		// Object is not managed by the Upgrade controller.
 		return []reconcile.Request{}
 	}
 
+	planNamespace := annotations[upgrade.PlanNamespaceAnnotation]
+
 	return []reconcile.Request{
-		{NamespacedName: upgrade.PlanNamespacedName(planName)},
+		{NamespacedName: types.NamespacedName{Namespace: planNamespace, Name: planName}},
 	}
 }
 
@@ -284,7 +273,7 @@ func (r *UpgradePlanReconciler) SetupWithManager(mgr ctrl.Manager) error {
 
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&lifecyclev1alpha1.UpgradePlan{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
-		Owns(&upgradecattlev1.Plan{}, builder.WithPredicates(predicate.Funcs{
+		Watches(&upgradecattlev1.Plan{}, handler.EnqueueRequestsFromMapFunc(r.findUpgradePlanFromAnnotations), builder.WithPredicates(predicate.Funcs{
 			CreateFunc: func(e event.CreateEvent) bool {
 				return false
 			},
@@ -319,6 +308,6 @@ func (r *UpgradePlanReconciler) SetupWithManager(mgr ctrl.Manager) error {
 				return false
 			},
 		})).
-		Owns(&corev1.Secret{}).
+		Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.findUpgradePlanFromAnnotations)).
 		Complete(r)
 }