Merge pull request #1757 from stuggi/conditional_crd

dataplane: Make MachineConfig CRD dependency optional

Author: openshift-merge-bot[bot]

internal/controller/dataplane/openstackdataplanenodeset_controller.go

@@ -29,17 +29,22 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	"github.com/go-logr/logr"
 	infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1"
@@ -68,8 +73,11 @@ const (
 // OpenStackDataPlaneNodeSetReconciler reconciles a OpenStackDataPlaneNodeSet object
 type OpenStackDataPlaneNodeSetReconciler struct {
 	client.Client
-	Kclient kubernetes.Interface
-	Scheme  *runtime.Scheme
+	Kclient    kubernetes.Interface
+	Scheme     *runtime.Scheme
+	Controller controller.Controller
+	Cache      cache.Cache
+	Watching   map[string]bool
 }
 
 // GetLogger returns a logger object with a prefix of "controller.name" and additional controller context fields
@@ -141,6 +149,10 @@ func (r *OpenStackDataPlaneNodeSetReconciler) Reconcile(ctx context.Context, req
 	Log := r.GetLogger(ctx)
 	Log.Info("Reconciling NodeSet")
 
+	// Try to set up MachineConfig watch if not already done
+	// This is done conditionally because MachineConfig CRD may not exist on all clusters
+	r.ensureMachineConfigWatch(ctx)
+
 	validate := validator.New()
 
 	// Fetch the OpenStackDataPlaneNodeSet instance
@@ -669,7 +681,12 @@ func (r *OpenStackDataPlaneNodeSetReconciler) SetupWithManager(
 		}); err != nil {
 		return err
 	}
-	return ctrl.NewControllerManagedBy(mgr).
+	// Initialize the Watching map for conditional CRD watches
+	r.Watching = make(map[string]bool)
+	r.Cache = mgr.GetCache()
+
+	// Build the controller without MachineConfig watch (added conditionally later)
+	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&dataplanev1.OpenStackDataPlaneNodeSet{},
 			builder.WithPredicates(predicate.Or(
 				predicate.GenerationChangedPredicate{},
@@ -692,10 +709,15 @@ func (r *OpenStackDataPlaneNodeSetReconciler) SetupWithManager(
 			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})).
 		Watches(&openstackv1.OpenStackVersion{},
 			handler.EnqueueRequestsFromMapFunc(r.genericWatcherFn)).
-		Watches(&machineconfig.MachineConfig{},
-			handler.EnqueueRequestsFromMapFunc(r.machineConfigWatcherFn),
-			builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})).
-		Complete(r)
+		// NOTE: MachineConfig watch is added conditionally during reconciliation
+		// to avoid failures when the MachineConfig CRD doesn't exist
+		Build(r)
+
+	if err != nil {
+		return err
+	}
+	r.Controller = c
+	return nil
 }
 
 // machineConfigWatcherFn - watches for changes to the registries MachineConfig resource and queues
@@ -734,6 +756,70 @@ func (r *OpenStackDataPlaneNodeSetReconciler) machineConfigWatcherFn(
 	return requests
 }
 
+// machineConfigWatcherFnTyped - typed version of machineConfigWatcherFn for use with source.Kind
+func (r *OpenStackDataPlaneNodeSetReconciler) machineConfigWatcherFnTyped(
+	ctx context.Context, obj *machineconfig.MachineConfig,
+) []reconcile.Request {
+	return r.machineConfigWatcherFn(ctx, obj)
+}
+
+const machineConfigCRDName = "machineconfigs.machineconfiguration.openshift.io"
+
+// ensureMachineConfigWatch attempts to set up a watch for MachineConfig resources.
+// This is done conditionally because the MachineConfig CRD may not exist on all clusters
+// (e.g., non-OpenShift Kubernetes clusters or clusters without the Machine Config Operator).
+// Returns true if the CRD is available (watch was set up or already exists), false otherwise.
+func (r *OpenStackDataPlaneNodeSetReconciler) ensureMachineConfigWatch(ctx context.Context) bool {
+	Log := r.GetLogger(ctx)
+
+	// Check if we're already watching
+	if r.Watching[machineConfigCRDName] {
+		return true
+	}
+
+	// Check if the MachineConfig CRD exists
+	crd := &unstructured.Unstructured{}
+	crd.SetGroupVersionKind(schema.GroupVersionKind{
+		Group:   "apiextensions.k8s.io",
+		Kind:    "CustomResourceDefinition",
+		Version: "v1",
+	})
+
+	err := r.Get(ctx, client.ObjectKey{Name: machineConfigCRDName}, crd)
+	if err != nil {
+		if k8s_errors.IsNotFound(err) {
+			Log.Info("MachineConfig CRD not found, disconnected environment features disabled")
+		} else {
+			Log.Error(err, "Error checking for MachineConfig CRD")
+		}
+		return false
+	}
+
+	// CRD exists, set up the watch
+	Log.Info("MachineConfig CRD found, enabling watch for disconnected environment support")
+	err = r.Controller.Watch(
+		source.Kind(
+			r.Cache,
+			&machineconfig.MachineConfig{},
+			handler.TypedEnqueueRequestsFromMapFunc(r.machineConfigWatcherFnTyped),
+			predicate.TypedResourceVersionChangedPredicate[*machineconfig.MachineConfig]{},
+		),
+	)
+	if err != nil {
+		Log.Error(err, "Failed to set up MachineConfig watch")
+		return false
+	}
+
+	r.Watching[machineConfigCRDName] = true
+	Log.Info("Successfully set up MachineConfig watch")
+	return true
+}
+
+// IsMachineConfigAvailable returns true if the MachineConfig CRD is available and being watched
+func (r *OpenStackDataPlaneNodeSetReconciler) IsMachineConfigAvailable() bool {
+	return r.Watching[machineConfigCRDName]
+}
+
 func (r *OpenStackDataPlaneNodeSetReconciler) secretWatcherFn(
 	ctx context.Context, obj client.Object,
 ) []reconcile.Request {

internal/dataplane/inventory.go

@@ -144,13 +144,28 @@ func GenerateNodeSetInventory(ctx context.Context, helper *helper.Helper,
 	if isDisconnected {
 		registryConfig, err := util.GetMCRegistryConf(ctx, helper)
 		if err != nil {
-			return "", err
-		}
-		helper.GetLogger().Info("disconnected registry was identified via the ImageContentSourcePolicy. Using OCP registry.")
+			// CRD not installed (non-OpenShift or no MCO) - log warning and continue.
+			// This allows graceful degradation when running on non-OpenShift clusters.
+			// Users can manually configure registries.conf via ansibleVars.
+			if util.IsNoMatchError(err) {
+				helper.GetLogger().Info("Disconnected environment detected but MachineConfig CRD not available. "+
+					"Registry configuration will not be propagated to dataplane nodes. "+
+					"You may need to configure registries.conf manually using ansibleVars "+
+					"(edpm_podman_disconnected_ocp and edpm_podman_registries_conf).",
+					"error", err.Error())
+			} else {
+				// CRD exists but resource not found, or other errors (network issues,
+				// permissions, etc.) - return the error. If MCO is installed but the
+				// registry MachineConfig doesn't exist, this indicates a misconfiguration.
+				return "", fmt.Errorf("failed to get MachineConfig registry configuration: %w", err)
+			}
+		} else {
+			helper.GetLogger().Info("disconnected registry was identified via the ImageContentSourcePolicy. Using OCP registry.")
 
-		// Use OCP registries.conf for disconnected deployments
-		nodeSetGroup.Vars["edpm_podman_registries_conf"] = registryConfig
-		nodeSetGroup.Vars["edpm_podman_disconnected_ocp"] = isDisconnected
+			// Use OCP registries.conf for disconnected deployments
+			nodeSetGroup.Vars["edpm_podman_registries_conf"] = registryConfig
+			nodeSetGroup.Vars["edpm_podman_disconnected_ocp"] = isDisconnected
+		}
 	}
 
 	// add TLS ansible variable

internal/dataplane/util/image_registry.go

@@ -38,27 +38,56 @@ type machineConfigIgnition struct {
 
 // IsDisconnectedOCP - Will retrieve a CR's related to disconnected OCP deployments. If the list is not
 // empty, we can infer that the OCP cluster is a disconnected deployment.
+// Returns false without error if the CRDs don't exist (non-OpenShift cluster).
 func IsDisconnectedOCP(ctx context.Context, helper *helper.Helper) (bool, error) {
 	icspList := ocpicsp.ImageContentSourcePolicyList{}
 	idmsList := ocpidms.ImageDigestMirrorSetList{}
 
 	listOpts := []client.ListOption{}
 
-	var err error
-	err = helper.GetClient().List(ctx, &icspList, listOpts...)
+	var icspCount, idmsCount int
+
+	err := helper.GetClient().List(ctx, &icspList, listOpts...)
 	if err != nil {
-		return false, err
+		// If the CRD doesn't exist, this is not an OpenShift cluster or ICSP is not available
+		// This is not an error condition - just means we're not in a disconnected environment
+		if IsNoMatchError(err) {
+			helper.GetLogger().Info("ImageContentSourcePolicy CRD not available, assuming not a disconnected environment")
+		} else {
+			return false, err
+		}
+	} else {
+		icspCount = len(icspList.Items)
 	}
+
 	err = helper.GetClient().List(ctx, &idmsList, listOpts...)
 	if err != nil {
-		return false, err
+		// If the CRD doesn't exist, this is not an OpenShift cluster or IDMS is not available
+		if IsNoMatchError(err) {
+			helper.GetLogger().Info("ImageDigestMirrorSet CRD not available, assuming not a disconnected environment")
+		} else {
+			return false, err
+		}
+	} else {
+		idmsCount = len(idmsList.Items)
 	}
 
-	if len(icspList.Items) != 0 || len(idmsList.Items) != 0 {
-		return true, err
+	if icspCount != 0 || idmsCount != 0 {
+		return true, nil
 	}
 
-	return false, err
+	return false, nil
+}
+
+// IsNoMatchError checks if the error indicates that a CRD/resource type doesn't exist
+func IsNoMatchError(err error) bool {
+	errStr := err.Error()
+	// Check for "no matches for kind" type errors which indicate the CRD doesn't exist.
+	// Also check for "no kind is registered" which occurs when the type isn't in the scheme.
+	// This is specifically needed for functional tests where the fake client returns a different
+	// error type than a real Kubernetes API server when CRDs are not installed.
+	return strings.Contains(errStr, "no matches for kind") ||
+		strings.Contains(errStr, "no kind is registered")
 }
 
 // GetMCRegistryConf - will unmarshal the MachineConfig ignition file the machineConfigIgnition object.