diff --git a/pkg/controller/hyperconverged/hyperconverged_controller.go b/pkg/controller/hyperconverged/hyperconverged_controller.go index a9797ba07..abde3b2a7 100644 --- a/pkg/controller/hyperconverged/hyperconverged_controller.go +++ b/pkg/controller/hyperconverged/hyperconverged_controller.go @@ -292,11 +292,7 @@ func (r *ReconcileHyperConverged) doReconcile(req *common.HcoRequest) (reconcile init := req.Instance.Status.Conditions == nil if init { r.eventEmitter.EmitEvent(req.Instance, corev1.EventTypeNormal, "InitHCO", "Initiating the HyperConverged") - err = r.setInitialConditions(req) - if err != nil { - req.Logger.Error(err, "Failed to add conditions to status") - return reconcile.Result{}, err - } + r.setInitialConditions(req) } r.setLabels(req) @@ -305,19 +301,19 @@ func (r *ReconcileHyperConverged) doReconcile(req *common.HcoRequest) (reconcile // negative conditions (!Available, Degraded, Progressing) req.Conditions = common.NewHcoConditions() - fin_dropped := false + finDropped := false // Handle finalizers if contains(req.Instance.ObjectMeta.Finalizers, badFinalizerName) { req.Logger.Info("removing a finalizer set in the past (without a fully qualified name)") - req.Instance.ObjectMeta.Finalizers, fin_dropped = drop(req.Instance.ObjectMeta.Finalizers, badFinalizerName) - req.Dirty = req.Dirty || fin_dropped + req.Instance.ObjectMeta.Finalizers, finDropped = drop(req.Instance.ObjectMeta.Finalizers, badFinalizerName) + req.Dirty = req.Dirty || finDropped } if req.Instance.ObjectMeta.DeletionTimestamp.IsZero() { // Add the finalizer if it's not there if !contains(req.Instance.ObjectMeta.Finalizers, FinalizerName) { req.Logger.Info("setting a finalizer (with fully qualified name)") req.Instance.ObjectMeta.Finalizers = append(req.Instance.ObjectMeta.Finalizers, FinalizerName) - req.Dirty = req.Dirty || fin_dropped + req.Dirty = req.Dirty || finDropped } } else { if !req.HCOTriggered { @@ -344,19 +340,22 @@ func (r *ReconcileHyperConverged) doReconcile(req *common.HcoRequest) (reconcile err = r.operandHandler.Ensure(req) if err != nil { - return reconcile.Result{}, r.updateConditions(req) + r.updateConditions(req) + hcoutil.SetReady(false) + return reconcile.Result{Requeue: init}, nil } req.Logger.Info("Reconcile complete") // Requeue if we just created everything if init { + hcoutil.SetReady(false) return reconcile.Result{Requeue: true}, err } - err = r.completeReconciliation(req) + r.completeReconciliation(req) - return reconcile.Result{}, err + return reconcile.Result{}, nil } func (r *ReconcileHyperConverged) getHcoInstanceFromK8s(req *common.HcoRequest) (*hcov1beta1.HyperConverged, error) { @@ -392,13 +391,13 @@ func (r *ReconcileHyperConverged) validateNamespace(req *common.HcoRequest) (boo Reason: invalidRequestReason, Message: fmt.Sprintf(invalidRequestMessageFormat, hco.Name, hco.Namespace), }) - err := r.updateConditions(req) - return false, err + r.updateConditions(req) + return false, nil } return true, nil } -func (r *ReconcileHyperConverged) setInitialConditions(req *common.HcoRequest) error { +func (r *ReconcileHyperConverged) setInitialConditions(req *common.HcoRequest) { req.Instance.Status.UpdateVersion(hcoVersionName, r.ownVersion) req.Instance.Spec.Version = r.ownVersion req.Dirty = true @@ -434,7 +433,7 @@ func (r *ReconcileHyperConverged) setInitialConditions(req *common.HcoRequest) e Message: reconcileInitMessage, }) - return r.updateConditions(req) + r.updateConditions(req) } func (r *ReconcileHyperConverged) ensureHcoDeleted(req *common.HcoRequest) (reconcile.Result, error) { @@ -662,7 +661,7 @@ func (r *ReconcileHyperConverged) aggregateComponentConditions(req *common.HcoRe return allComponentsAreUp } -func (r *ReconcileHyperConverged) completeReconciliation(req *common.HcoRequest) error { +func (r *ReconcileHyperConverged) completeReconciliation(req *common.HcoRequest) { allComponentsAreUp := r.aggregateComponentConditions(req) hcoReady := false @@ -713,11 +712,12 @@ func (r *ReconcileHyperConverged) completeReconciliation(req *common.HcoRequest) r.eventEmitter.EmitEvent(req.Instance, corev1.EventTypeWarning, "ReconcileHCO", "Not all the operators are ready") } } - return r.updateConditions(req) + + r.updateConditions(req) } // This function is used to exit from the reconcile function, updating the conditions and returns the reconcile result -func (r *ReconcileHyperConverged) updateConditions(req *common.HcoRequest) error { +func (r *ReconcileHyperConverged) updateConditions(req *common.HcoRequest) { for _, condType := range common.HcoConditionTypes { cond, found := req.Conditions[condType] if !found { @@ -734,7 +734,6 @@ func (r *ReconcileHyperConverged) updateConditions(req *common.HcoRequest) error r.detectTaintedConfiguration(req) req.StatusDirty = true - return nil } func (r *ReconcileHyperConverged) setLabels(req *common.HcoRequest) { diff --git a/pkg/controller/hyperconverged/hyperconverged_controller_test.go b/pkg/controller/hyperconverged/hyperconverged_controller_test.go index c7746fb42..c0daa9957 100644 --- a/pkg/controller/hyperconverged/hyperconverged_controller_test.go +++ b/pkg/controller/hyperconverged/hyperconverged_controller_test.go @@ -615,6 +615,83 @@ var _ = Describe("HyperconvergedController", func() { Expect(foundResource.ObjectMeta.Finalizers).Should(Equal([]string{FinalizerName})) }) + It("Should not be ready if one of the operands is returns error, on create", func() { + hcoutil.SetReady(true) + Expect(checkHcoReady()).To(BeTrue()) + hco := commonTestUtils.NewHco() + cl := commonTestUtils.InitClient([]runtime.Object{hco}) + cl.InitiateWriteErrors(nil, errors.New("fake write error")) + r := initReconciler(cl) + + // Do the reconcile + res, err := r.Reconcile(request) + Expect(err).To(BeNil()) + Expect(res).Should(Equal(reconcile.Result{Requeue: true})) + + // Get the HCO + foundResource := &hcov1beta1.HyperConverged{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: hco.Name, Namespace: hco.Namespace}, + foundResource), + ).To(BeNil()) + + // Check condition + foundCond := false + for _, cond := range foundResource.Status.Conditions { + if cond.Type == hcov1beta1.ConditionReconcileComplete { + foundCond = true + Expect(cond.Status).Should(Equal(corev1.ConditionFalse)) + Expect(cond.Message).Should(ContainSubstring("fake write error")) + break + } + } + Expect(foundCond).To(BeTrue()) + + Expect(checkHcoReady()).To(BeFalse()) + }) + + It("Should not be ready if one of the operands is returns error, on update", func() { + expected := getBasicDeployment() + expected.kv.Spec.Configuration.DeveloperConfiguration = &kubevirtv1.DeveloperConfiguration{ + FeatureGates: []string{"fakeFg"}, // force update + } + cl := expected.initClient() + cl.InitiateWriteErrors(nil, errors.New("fake write error")) + + hcoutil.SetReady(true) + Expect(checkHcoReady()).To(BeTrue()) + + hco := commonTestUtils.NewHco() + r := initReconciler(cl) + + // Do the reconcile + res, err := r.Reconcile(request) + Expect(err).To(BeNil()) + Expect(res).Should(Equal(reconcile.Result{Requeue: false})) + + // Get the HCO + foundResource := &hcov1beta1.HyperConverged{} + Expect( + cl.Get(context.TODO(), + types.NamespacedName{Name: hco.Name, Namespace: hco.Namespace}, + foundResource), + ).To(BeNil()) + + // Check condition + foundCond := false + for _, cond := range foundResource.Status.Conditions { + if cond.Type == hcov1beta1.ConditionReconcileComplete { + foundCond = true + Expect(cond.Status).Should(Equal(corev1.ConditionFalse)) + Expect(cond.Message).Should(ContainSubstring("fake write error")) + break + } + } + Expect(foundCond).To(BeTrue()) + + Expect(checkHcoReady()).To(BeFalse()) + }) }) Context("Validate OLM required fields", func() {