Harmonize reconciler logic and improve logging

.github/workflows/lint.yml

@@ -17,4 +17,4 @@ jobs:
     - name: golangci-lint
       uses: golangci/golangci-lint-action@v9
       with:
-        version: v2.5
+        version: v2.6

Makefile

@@ -201,7 +201,7 @@ CONTROLLER_TOOLS_VERSION ?= v0.17.1
 ENVTEST_VERSION ?= $(shell go list -m -f "{{ .Version }}" sigs.k8s.io/controller-runtime | awk -F'[v.]' '{printf "release-%d.%d", $$2, $$3}')
 #ENVTEST_K8S_VERSION is the version of Kubernetes to use for setting up ENVTEST binaries (i.e. 1.31)
 ENVTEST_K8S_VERSION ?= $(shell go list -m -f "{{ .Version }}" k8s.io/api | awk -F'[v.]' '{printf "1.%d.%d",$$3, $$2}')
-GOLANGCI_LINT_VERSION ?= v2.1
+GOLANGCI_LINT_VERSION ?= v2.6
 ADDLICENSE_VERSION ?= v1.1.1
 GOIMPORTS_VERSION ?= v0.31.0
 CRD_REF_DOCS_VERSION ?= v0.2.0

cmdutils/suite_test.go

@@ -59,7 +59,7 @@ var _ = BeforeSuite(func() {
 		// Note that you must have the required binaries setup under the bin directory to perform
 		// the tests directly. When we run make test it will be setup and used automatically.
 		BinaryAssetsDirectory: filepath.Join("..", "bin", "k8s",
-			fmt.Sprintf("1.31.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
+			fmt.Sprintf("1.34.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
 	}
 
 	sourceCfg, err := sourceEnv.Start()
@@ -89,7 +89,7 @@ var _ = BeforeSuite(func() {
 		// Note that you must have the required binaries setup under the bin directory to perform
 		// the tests directly. When we run make test it will be setup and used automatically.
 		BinaryAssetsDirectory: filepath.Join("..", "bin", "k8s",
-			fmt.Sprintf("1.31.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
+			fmt.Sprintf("1.34.0-%s-%s", runtime.GOOS, runtime.GOARCH)),
 	}
 
 	// cfg is defined in this file globally.

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/opencontainers/image-spec v1.1.1
 	github.com/spf13/cobra v1.10.2
 	k8s.io/api v0.34.1
+	k8s.io/apiextensions-apiserver v0.34.1
 	k8s.io/apimachinery v0.34.1
 	k8s.io/client-go v0.34.1
 	k8s.io/utils v0.0.0-20251002143259-bc988d571ff4
@@ -122,7 +123,6 @@ require (
 	gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/apiextensions-apiserver v0.34.1 // indirect
 	k8s.io/apiserver v0.34.1 // indirect
 	k8s.io/component-base v0.34.1 // indirect
 	k8s.io/klog/v2 v2.130.1 // indirect

internal/controller/httpbootconfig_controller.go

@@ -13,7 +13,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/go-logr/logr"
@@ -32,55 +31,53 @@ type HTTPBootConfigReconciler struct {
 //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
 
 func (r *HTTPBootConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
-
-	HTTPBootConfig := &bootv1alpha1.HTTPBootConfig{}
-	if err := r.Get(ctx, req.NamespacedName, HTTPBootConfig); err != nil {
+	log := ctrl.LoggerFrom(ctx)
+	config := &bootv1alpha1.HTTPBootConfig{}
+	if err := r.Get(ctx, req.NamespacedName, config); err != nil {
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
-
-	return r.reconcileExists(ctx, log, HTTPBootConfig)
+	return r.reconcileExists(ctx, log, config)
 }
 
-func (r *HTTPBootConfigReconciler) reconcileExists(ctx context.Context, log logr.Logger, HTTPBootConfig *bootv1alpha1.HTTPBootConfig) (ctrl.Result, error) {
-	if !HTTPBootConfig.DeletionTimestamp.IsZero() {
-		return r.delete(ctx, log, HTTPBootConfig)
+func (r *HTTPBootConfigReconciler) reconcileExists(ctx context.Context, log logr.Logger, config *bootv1alpha1.HTTPBootConfig) (ctrl.Result, error) {
+	if !config.DeletionTimestamp.IsZero() {
+		return r.delete(ctx, log, config)
 	}
-
-	return r.reconcile(ctx, log, HTTPBootConfig)
+	return r.reconcile(ctx, log, config)
 }
 
-func (r *HTTPBootConfigReconciler) reconcile(ctx context.Context, log logr.Logger, HTTPBootConfig *bootv1alpha1.HTTPBootConfig) (ctrl.Result, error) {
+func (r *HTTPBootConfigReconciler) reconcile(ctx context.Context, log logr.Logger, config *bootv1alpha1.HTTPBootConfig) (ctrl.Result, error) {
+	log.V(1).Info("Reconciling HTTPBootConfig")
+
 	log.V(1).Info("Ensuring Ignition")
-	state, ignitionErr := r.ensureIgnition(ctx, log, HTTPBootConfig)
-	if ignitionErr != nil {
-		patchError := r.patchStatus(ctx, HTTPBootConfig, state)
-		if patchError != nil {
-			return ctrl.Result{}, fmt.Errorf("failed to patch status %w %w", ignitionErr, patchError)
+	state, err := r.ensureIgnition(ctx, log, config)
+	if err != nil {
+		if err := r.patchStatus(ctx, config, state); err != nil {
+			return ctrl.Result{}, err
 		}
-
-		log.V(1).Info("Failed to Ensure Ignition", "Error", ignitionErr)
+		log.V(1).Info("Failed to Ensure Ignition", "Error", err)
 		return ctrl.Result{}, nil
 	}
+	log.V(1).Info("Ensured Ignition")
 
-	patchErr := r.patchStatus(ctx, HTTPBootConfig, state)
-	if patchErr != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to patch status %w", patchErr)
+	if err := r.patchStatus(ctx, config, state); err != nil {
+		return ctrl.Result{}, err
 	}
 
+	log.V(1).Info("Reconciled HTTPBootConfig")
 	return ctrl.Result{}, nil
 }
 
-func (r *HTTPBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Logger, HTTPBootConfig *bootv1alpha1.HTTPBootConfig) (bootv1alpha1.HTTPBootConfigState, error) {
+func (r *HTTPBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Logger, config *bootv1alpha1.HTTPBootConfig) (bootv1alpha1.HTTPBootConfigState, error) {
 	// Verify if the IgnitionRef is set, and it has the intended data key.
-	if HTTPBootConfig.Spec.IgnitionSecretRef != nil {
-		IgnitionSecret := &corev1.Secret{}
-		if err := r.Get(ctx, client.ObjectKey{Name: HTTPBootConfig.Spec.IgnitionSecretRef.Name, Namespace: HTTPBootConfig.Namespace}, IgnitionSecret); err != nil {
+	if config.Spec.IgnitionSecretRef != nil {
+		ignitionSecret := &corev1.Secret{}
+		if err := r.Get(ctx, client.ObjectKey{Name: config.Spec.IgnitionSecretRef.Name, Namespace: config.Namespace}, ignitionSecret); err != nil {
 			return bootv1alpha1.HTTPBootConfigStateError, err
-			// TODO: Add some validation steps to ensure that the IgntionData is compliant, if necessary.
+			// TODO: Add some validation steps to ensure that the IgnitionData is compliant, if necessary.
 			// Assume for now, that it's going to json format.
 		}
-		if IgnitionSecret.Data[bootv1alpha1.DefaultIgnitionKey] == nil {
+		if ignitionSecret.Data[bootv1alpha1.DefaultIgnitionKey] == nil {
 			return bootv1alpha1.HTTPBootConfigStateError, fmt.Errorf("ignition data is missing")
 		}
 	}
@@ -93,64 +90,60 @@ func (r *HTTPBootConfigReconciler) delete(_ context.Context, log logr.Logger, _
 
 	// TODO
 
+	log.V(1).Info("Deleted HTTPBootConfig")
 	return ctrl.Result{}, nil
 }
 
-// SetupWithManager sets up the controller with the Manager.
-func (r *HTTPBootConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewControllerManagedBy(mgr).
-		For(&bootv1alpha1.HTTPBootConfig{}).
-		Watches(
-			&corev1.Secret{},
-			handler.EnqueueRequestsFromMapFunc(r.enqueueHTTPBootConfigReferencingIgnitionSecret),
-		).
-		Complete(r)
-}
-
-func (r *HTTPBootConfigReconciler) patchStatus(
-	ctx context.Context,
-	HTTPBootConfig *bootv1alpha1.HTTPBootConfig,
-	state bootv1alpha1.HTTPBootConfigState,
-) error {
-	if HTTPBootConfig.Status.State == state {
+func (r *HTTPBootConfigReconciler) patchStatus(ctx context.Context, config *bootv1alpha1.HTTPBootConfig, state bootv1alpha1.HTTPBootConfigState) error {
+	if config.Status.State == state {
 		return nil
 	}
 
-	base := HTTPBootConfig.DeepCopy()
-	HTTPBootConfig.Status.State = state
+	base := config.DeepCopy()
+	config.Status.State = state
 
-	if err := r.Status().Patch(ctx, HTTPBootConfig, client.MergeFrom(base)); err != nil {
-		return fmt.Errorf("error patching HTTPBootConfig: %w", err)
+	if err := r.Status().Patch(ctx, config, client.MergeFrom(base)); err != nil {
+		return err
 	}
 	return nil
 }
 
 func (r *HTTPBootConfigReconciler) enqueueHTTPBootConfigReferencingIgnitionSecret(ctx context.Context, secret client.Object) []reconcile.Request {
-	log := log.Log.WithValues("secret", secret.GetName())
+	log := ctrl.LoggerFrom(ctx)
 	secretObj, ok := secret.(*corev1.Secret)
 	if !ok {
 		log.Error(nil, "cant decode object into Secret", secret)
 		return nil
 	}
 
-	list := &bootv1alpha1.HTTPBootConfigList{}
-	if err := r.List(ctx, list, client.InNamespace("")); err != nil {
-		log.Error(err, "failed to list HTTPBootConfig for secret", secret)
+	configList := &bootv1alpha1.HTTPBootConfigList{}
+	if err := r.List(ctx, configList, client.InNamespace(secretObj.Namespace)); err != nil {
+		log.Error(err, "failed to list HTTPBootConfig for Secret", "Secret", client.ObjectKeyFromObject(secretObj))
 		return nil
 	}
 
 	var requests []reconcile.Request
-	for _, HTTPBootConfig := range list.Items {
-		if HTTPBootConfig.Spec.IgnitionSecretRef != nil &&
-			HTTPBootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name &&
-			HTTPBootConfig.Namespace == secretObj.Namespace {
+	for _, config := range configList.Items {
+		if config.Spec.IgnitionSecretRef != nil &&
+			config.Spec.IgnitionSecretRef.Name == secretObj.Name {
 			requests = append(requests, reconcile.Request{
 				NamespacedName: types.NamespacedName{
-					Name:      HTTPBootConfig.Name,
-					Namespace: HTTPBootConfig.Namespace,
+					Name:      config.Name,
+					Namespace: config.Namespace,
 				},
 			})
 		}
 	}
 	return requests
 }
+
+// SetupWithManager sets up the controller with the Manager.
+func (r *HTTPBootConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewControllerManagedBy(mgr).
+		For(&bootv1alpha1.HTTPBootConfig{}).
+		Watches(
+			&corev1.Secret{},
+			handler.EnqueueRequestsFromMapFunc(r.enqueueHTTPBootConfigReferencingIgnitionSecret),
+		).
+		Complete(r)
+}

internal/controller/ipxebootconfig_controller.go

@@ -12,7 +12,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/go-logr/logr"
@@ -32,49 +31,47 @@ type IPXEBootConfigReconciler struct {
 //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch
 
 func (r *IPXEBootConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
-	IPXEBootConfig := &bootv1alpha1.IPXEBootConfig{}
-	if err := r.Get(ctx, req.NamespacedName, IPXEBootConfig); err != nil {
+	log := ctrl.LoggerFrom(ctx)
+	config := &bootv1alpha1.IPXEBootConfig{}
+	if err := r.Get(ctx, req.NamespacedName, config); err != nil {
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
-
-	return r.reconcileExists(ctx, log, IPXEBootConfig)
+	return r.reconcileExists(ctx, log, config)
 }
 
-func (r *IPXEBootConfigReconciler) reconcileExists(ctx context.Context, log logr.Logger, IPXEBootConfig *bootv1alpha1.IPXEBootConfig) (ctrl.Result, error) {
-	if !IPXEBootConfig.DeletionTimestamp.IsZero() {
-		return r.delete(ctx, log, IPXEBootConfig)
+func (r *IPXEBootConfigReconciler) reconcileExists(ctx context.Context, log logr.Logger, config *bootv1alpha1.IPXEBootConfig) (ctrl.Result, error) {
+	if !config.DeletionTimestamp.IsZero() {
+		return r.delete(ctx, log, config)
 	}
-
-	return r.reconcile(ctx, log, IPXEBootConfig)
+	return r.reconcile(ctx, log, config)
 }
 
-func (r *IPXEBootConfigReconciler) reconcile(ctx context.Context, log logr.Logger, ipxeBootConfig *bootv1alpha1.IPXEBootConfig) (ctrl.Result, error) {
+func (r *IPXEBootConfigReconciler) reconcile(ctx context.Context, log logr.Logger, config *bootv1alpha1.IPXEBootConfig) (ctrl.Result, error) {
+	log.V(1).Info("Reconciling IPXEBootConfig")
+
 	log.V(1).Info("Ensuring Ignition")
-	state, ignitionErr := r.ensureIgnition(ctx, log, ipxeBootConfig)
-	if ignitionErr != nil {
-		patchError := r.patchStatus(ctx, ipxeBootConfig, state)
-		if patchError != nil {
-			return ctrl.Result{}, fmt.Errorf("failed to patch status %w %w", ignitionErr, patchError)
+	state, err := r.ensureIgnition(ctx, log, config)
+	if err != nil {
+		if err := r.patchStatus(ctx, config, state); err != nil {
+			return ctrl.Result{}, err
 		}
-
-		log.V(1).Info("Failed to Ensure Ignition", "Error", ignitionErr)
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, fmt.Errorf("failed to ensure Ignition: %w", err)
 	}
+	log.V(1).Info("Ensured Ignition")
 
-	patchErr := r.patchStatus(ctx, ipxeBootConfig, state)
-	if patchErr != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to patch status %w", patchErr)
+	if err := r.patchStatus(ctx, config, state); err != nil {
+		return ctrl.Result{}, fmt.Errorf("failed to patch status %w", err)
 	}
 
+	log.V(1).Info("Reconciled IPXEBootConfig")
 	return ctrl.Result{}, nil
 }
 
-func (r *IPXEBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Logger, ipxeBootConfig *bootv1alpha1.IPXEBootConfig) (bootv1alpha1.IPXEBootConfigState, error) {
+func (r *IPXEBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Logger, config *bootv1alpha1.IPXEBootConfig) (bootv1alpha1.IPXEBootConfigState, error) {
 	// Verify if the IgnitionRef is set, and it has the intended data key.
-	if ipxeBootConfig.Spec.IgnitionSecretRef != nil {
+	if config.Spec.IgnitionSecretRef != nil {
 		IgnitionSecret := &corev1.Secret{}
-		if err := r.Get(ctx, client.ObjectKey{Name: ipxeBootConfig.Spec.IgnitionSecretRef.Name, Namespace: ipxeBootConfig.Namespace}, IgnitionSecret); err != nil {
+		if err := r.Get(ctx, client.ObjectKey{Name: config.Spec.IgnitionSecretRef.Name, Namespace: config.Namespace}, IgnitionSecret); err != nil {
 			return bootv1alpha1.IPXEBootConfigStateError, err
 			// TODO: Add some validation steps to ensure that the IgntionData is compliant, if necessary.
 			// Assume for now, that it's going to json format.
@@ -88,24 +85,14 @@ func (r *IPXEBootConfigReconciler) ensureIgnition(ctx context.Context, _ logr.Lo
 }
 
 func (r *IPXEBootConfigReconciler) delete(_ context.Context, log logr.Logger, _ *bootv1alpha1.IPXEBootConfig) (ctrl.Result, error) {
-	log.V(1).Info("Deleting ipxeBootConfig")
+	log.V(1).Info("Deleting IPXEBootConfig")
 
 	// TODO
 
+	log.V(1).Info("Deleted IPXEBootConfig")
 	return ctrl.Result{}, nil
 }
 
-// SetupWithManager sets up the controller with the Manager.
-func (r *IPXEBootConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewControllerManagedBy(mgr).
-		For(&bootv1alpha1.IPXEBootConfig{}).
-		Watches(
-			&corev1.Secret{},
-			handler.EnqueueRequestsFromMapFunc(r.enqueueIPXEBootConfigReferencingIgnitionSecret),
-		).
-		Complete(r)
-}
-
 func (r *IPXEBootConfigReconciler) patchStatus(
 	ctx context.Context,
 	ipxeBootConfig *bootv1alpha1.IPXEBootConfig,
@@ -121,29 +108,40 @@ func (r *IPXEBootConfigReconciler) patchStatus(
 }
 
 func (r *IPXEBootConfigReconciler) enqueueIPXEBootConfigReferencingIgnitionSecret(ctx context.Context, secret client.Object) []reconcile.Request {
-	log := log.Log.WithValues("secret", secret.GetName())
+	log := ctrl.LoggerFrom(ctx)
 	secretObj, ok := secret.(*corev1.Secret)
 	if !ok {
 		log.Error(nil, "cant decode object into Secret", secret)
 		return nil
 	}
 
-	list := &bootv1alpha1.IPXEBootConfigList{}
-	if err := r.List(ctx, list, client.InNamespace(secretObj.Namespace)); err != nil {
-		log.Error(err, "failed to list IPXEBootConfig for secret", secret)
+	configList := &bootv1alpha1.IPXEBootConfigList{}
+	if err := r.List(ctx, configList, client.InNamespace(secretObj.Namespace)); err != nil {
+		log.Error(err, "failed to list IPXEBootConfig for Secret", "Secret", client.ObjectKeyFromObject(secretObj))
 		return nil
 	}
 
 	var requests []reconcile.Request
-	for _, ipxeBootConfig := range list.Items {
-		if ipxeBootConfig.Spec.IgnitionSecretRef != nil && ipxeBootConfig.Spec.IgnitionSecretRef.Name == secretObj.Name {
+	for _, config := range configList.Items {
+		if config.Spec.IgnitionSecretRef != nil && config.Spec.IgnitionSecretRef.Name == secretObj.Name {
 			requests = append(requests, reconcile.Request{
 				NamespacedName: types.NamespacedName{
-					Name:      ipxeBootConfig.Name,
-					Namespace: ipxeBootConfig.Namespace,
+					Name:      config.Name,
+					Namespace: config.Namespace,
 				},
 			})
 		}
 	}
 	return requests
 }
+
+// SetupWithManager sets up the controller with the Manager.
+func (r *IPXEBootConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	return ctrl.NewControllerManagedBy(mgr).
+		For(&bootv1alpha1.IPXEBootConfig{}).
+		Watches(
+			&corev1.Secret{},
+			handler.EnqueueRequestsFromMapFunc(r.enqueueIPXEBootConfigReferencingIgnitionSecret),
+		).
+		Complete(r)
+}