From 8798044dc00a91c7e1a87d7ca1d264d2f59676e4 Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Mon, 30 Nov 2020 12:50:52 -0500 Subject: [PATCH 1/2] Handle case when Provisioning CR is absent on the Baremetal platform This could happen during the assisted installation scenario. --- controllers/provisioning_controller.go | 24 +++++++++++++++------ controllers/provisioning_controller_test.go | 2 +- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/controllers/provisioning_controller.go b/controllers/provisioning_controller.go index 530ef0a0e..6874e66a8 100644 --- a/controllers/provisioning_controller.go +++ b/controllers/provisioning_controller.go @@ -26,6 +26,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -101,17 +102,17 @@ func (r *ProvisioningReconciler) isEnabled() (bool, error) { return true, nil } -func (r *ProvisioningReconciler) readProvisioningCR(req ctrl.Request) (*metal3iov1alpha1.Provisioning, error) { +func (r *ProvisioningReconciler) readProvisioningCR(namespacedName types.NamespacedName) (*metal3iov1alpha1.Provisioning, error) { ctx := context.Background() // provisioning.metal3.io is a singleton - if req.Name != BaremetalProvisioningCR { - r.Log.V(1).Info("ignoring invalid CR", "name", req.Name) + if namespacedName.Name != BaremetalProvisioningCR { + r.Log.V(1).Info("ignoring invalid CR", "name", namespacedName.Name) return nil, nil } // Fetch the Provisioning instance instance := &metal3iov1alpha1.Provisioning{} - if err := r.Client.Get(ctx, req.NamespacedName, instance); err != nil { + if err := r.Client.Get(ctx, namespacedName, instance); err != nil { if apierrors.IsNotFound(err) { return nil, nil } @@ -140,7 +141,7 @@ func (r *ProvisioningReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error return ctrl.Result{}, nil } - baremetalConfig, err := r.readProvisioningCR(req) + baremetalConfig, err := r.readProvisioningCR(req.NamespacedName) if err != nil { // Error reading the object - requeue the request. return ctrl.Result{}, err @@ -270,7 +271,6 @@ func (r *ProvisioningReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error // SetupWithManager configures the manager to run the controller func (r *ProvisioningReconciler) SetupWithManager(mgr ctrl.Manager) error { - // Check the Platform Type to determine the state of the CO enabled, err := r.isEnabled() if err != nil { @@ -283,6 +283,18 @@ func (r *ProvisioningReconciler) SetupWithManager(mgr ctrl.Manager) error { return errors.Wrap(err, "unable to set baremetal ClusterOperator to Disabled") } } + + if enabled { + namespacedName := types.NamespacedName{Name: BaremetalProvisioningCR} + baremetalConfig, err := r.readProvisioningCR(namespacedName) + if err != nil || baremetalConfig == nil { + err = r.updateCOStatus(ReasonComplete, "Provisioning CR not found on BareMetal Platform; marking operator as available", "") + if err != nil { + return fmt.Errorf("unable to put %q ClusterOperator in Available state: %v", clusterOperatorName, err) + } + } + } + return ctrl.NewControllerManagedBy(mgr). For(&metal3iov1alpha1.Provisioning{}). Owns(&corev1.Secret{}). diff --git a/controllers/provisioning_controller_test.go b/controllers/provisioning_controller_test.go index fb1bea285..b2984ee15 100644 --- a/controllers/provisioning_controller_test.go +++ b/controllers/provisioning_controller_test.go @@ -154,7 +154,7 @@ func TestProvisioningCRName(t *testing.T) { t.Logf("Testing tc : %s", tc.name) reconciler := newFakeProvisioningReconciler(setUpSchemeForReconciler(), tc.baremetalCR) - baremetalconfig, err := reconciler.readProvisioningCR(tc.req) + baremetalconfig, err := reconciler.readProvisioningCR(tc.req.NamespacedName) if !tc.expectedError && err != nil { t.Errorf("unexpected error: %v", err) return From 2e9d11776071a58f04c5ca5eaec565510028685e Mon Sep 17 00:00:00 2001 From: Sandhya Dasu Date: Mon, 30 Nov 2020 19:55:58 -0500 Subject: [PATCH 2/2] Ensure baremetal CO is completely setup before Reconcile --- controllers/clusteroperator.go | 2 +- controllers/provisioning_controller.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/controllers/clusteroperator.go b/controllers/clusteroperator.go index b7e8ccbbc..43077fe73 100644 --- a/controllers/clusteroperator.go +++ b/controllers/clusteroperator.go @@ -130,7 +130,7 @@ func (r *ProvisioningReconciler) ensureClusterOperator(baremetalConfig *metal3io } // if the CO has been created with the manifest then we need to update the ownership - if len(co.ObjectMeta.OwnerReferences) == 0 { + if baremetalConfig != nil && len(co.ObjectMeta.OwnerReferences) == 0 { err = controllerutil.SetControllerReference(baremetalConfig, co, r.Scheme) if err != nil { return err diff --git a/controllers/provisioning_controller.go b/controllers/provisioning_controller.go index 6874e66a8..ba65137cd 100644 --- a/controllers/provisioning_controller.go +++ b/controllers/provisioning_controller.go @@ -271,6 +271,11 @@ func (r *ProvisioningReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error // SetupWithManager configures the manager to run the controller func (r *ProvisioningReconciler) SetupWithManager(mgr ctrl.Manager) error { + err := r.ensureClusterOperator(nil) + if err != nil { + return errors.Wrap(err, "unable to set get baremetal ClusterOperator") + } + // Check the Platform Type to determine the state of the CO enabled, err := r.isEnabled() if err != nil { @@ -284,6 +289,7 @@ func (r *ProvisioningReconciler) SetupWithManager(mgr ctrl.Manager) error { } } + // If Platform is BareMetal, we could still be missing the Provisioning CR if enabled { namespacedName := types.NamespacedName{Name: BaremetalProvisioningCR} baremetalConfig, err := r.readProvisioningCR(namespacedName)