diff --git a/cmd/main.go b/cmd/main.go index a9fe3a3..fecb731 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -49,6 +49,7 @@ import ( kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller" + "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/controller/ready" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/global" "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/logger" @@ -316,6 +317,14 @@ func main() { os.Exit(1) } + if err = (&ready.Controller{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", ready.ControllerName) + os.Exit(1) + } + // +kubebuilder:scaffold:builder if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { diff --git a/internal/controller/hypervisor_controller.go b/internal/controller/hypervisor_controller.go index e9f62b0..0456959 100644 --- a/internal/controller/hypervisor_controller.go +++ b/internal/controller/hypervisor_controller.go @@ -112,18 +112,6 @@ func (hv *HypervisorController) Reconcile(ctx context.Context, req ctrl.Request) nodeTerminationCondition := FindNodeStatusCondition(node.Status.Conditions, "Terminating") if nodeTerminationCondition != nil && nodeTerminationCondition.Status == corev1.ConditionTrue { // Node might be terminating, propagate condition to hypervisor - - if readyCondition := meta.FindStatusCondition(hypervisor.Status.Conditions, kvmv1.ConditionTypeReady); readyCondition == nil || readyCondition.Status == metav1.ConditionTrue { - // Only set Terminating condition if Ready is still True, otherwise we might overwrite other controllers that already set Ready to False - // In particular if the hypervisor is evicting - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: nodeTerminationCondition.Reason, - Message: nodeTerminationCondition.Message, - }) - } - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeTerminating, Status: metav1.ConditionStatus(nodeTerminationCondition.Status), diff --git a/internal/controller/hypervisor_controller_test.go b/internal/controller/hypervisor_controller_test.go index 44a0f49..9e1657b 100644 --- a/internal/controller/hypervisor_controller_test.go +++ b/internal/controller/hypervisor_controller_test.go @@ -21,7 +21,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -311,12 +310,7 @@ var _ = Describe("Hypervisor Controller", func() { // Get the Hypervisor resource updatedHypervisor := &kvmv1.Hypervisor{} Expect(hypervisorController.Get(ctx, hypervisorName, updatedHypervisor)).To(Succeed()) - Expect(updatedHypervisor.Status.Conditions).To(ContainElements( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Reason", terminatingReason), - HaveField("Status", metav1.ConditionFalse), - ), + Expect(updatedHypervisor.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeTerminating), HaveField("Reason", terminatingReason), @@ -342,39 +336,6 @@ var _ = Describe("Hypervisor Controller", func() { }) }) - Context("and the Hypervisor resource already has a Ready Condition set to false", func() { - BeforeEach(func(ctx SpecContext) { - hypervisor := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: "SomeOtherReason", - }) - Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) - }) - - It("should not update the existing Ready Condition with the new reason", func(ctx SpecContext) { - for range 2 { - _, err := hypervisorController.Reconcile(ctx, ctrl.Request{ - NamespacedName: types.NamespacedName{Name: resource.Name}, - }) - Expect(err).NotTo(HaveOccurred()) - } - - // Get the Hypervisor resource again - updatedHypervisor := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, updatedHypervisor)).To(Succeed()) - Expect(updatedHypervisor.Status.Conditions).To(ContainElement( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Reason", "SomeOtherReason"), - HaveField("Status", metav1.ConditionFalse), - ), - )) - }) - }) - It("should successfully reconcile the terminating node", func(ctx SpecContext) { By("Reconciling the created resource") for range 2 { @@ -391,12 +352,7 @@ var _ = Describe("Hypervisor Controller", func() { // Not sure, if that is a good idea, but that is the current behaviour // We expect another operator to set the Maintenance field to Termination Expect(updatedHypervisor.Spec.Maintenance).NotTo(Equal(kvmv1.MaintenanceTermination)) - Expect(updatedHypervisor.Status.Conditions).To(ContainElements( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Reason", terminatingReason), - HaveField("Status", metav1.ConditionFalse), - ), + Expect(updatedHypervisor.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeTerminating), HaveField("Reason", terminatingReason), diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index eeaee0a..f4662ee 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -109,13 +109,6 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. return nil } - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonReadyReady, - Message: "Hypervisor is ready", - }) - // We need to enable the host as per spec enableService := services.UpdateOpts{Status: services.ServiceEnabled} log.Info("Enabling hypervisor", "id", serviceId) @@ -138,13 +131,6 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. return nil } - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonReadyMaintenance, - Message: "Hypervisor is disabled", - }) - // We need to disable the host as per spec enableService := services.UpdateOpts{ Status: services.ServiceDisabled, @@ -194,22 +180,10 @@ func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Contex message = "Evicted" reason = kvmv1.ConditionReasonSucceeded hv.Status.Evicted = true - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonReadyEvicted, - Message: "Hypervisor is disabled and evicted", - }) } else { message = "Evicting" reason = kvmv1.ConditionReasonRunning hv.Status.Evicted = false - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonReadyEvicting, - Message: "Hypervisor is disabled and evicting", - }) } meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ diff --git a/internal/controller/hypervisor_maintenance_controller_test.go b/internal/controller/hypervisor_maintenance_controller_test.go index da8f947..8609217 100644 --- a/internal/controller/hypervisor_maintenance_controller_test.go +++ b/internal/controller/hypervisor_maintenance_controller_test.go @@ -157,16 +157,6 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(meta.IsStatusConditionFalse(updated.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)).To(BeTrue()) }) - - It("should set the ConditionTypeReady to true", func(ctx SpecContext) { - updated := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) - Expect(updated.Status.Conditions).To(ContainElement( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Status", metav1.ConditionTrue), - ))) - }) }) // Spec.Maintenance="" }) @@ -189,16 +179,6 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)).To(BeTrue()) }) - - It("should set the ConditionTypeReady to false", func(ctx SpecContext) { - updated := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) - Expect(updated.Status.Conditions).To(ContainElement( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Status", metav1.ConditionFalse), - ))) - }) }) // Spec.Maintenance="" } @@ -339,14 +319,13 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(hypervisor.Status.Evicted).To(BeFalse()) }) - It("should set the ConditionTypeReady to false and reason to evicting", func(ctx SpecContext) { + It("should set the ConditionTypeEvicting to true", func(ctx SpecContext) { updated := &kvmv1.Hypervisor{} Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Conditions).To(ContainElement( SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", kvmv1.ConditionReasonReadyEvicting), + HaveField("Type", kvmv1.ConditionTypeEvicting), + HaveField("Status", metav1.ConditionTrue), ))) }) }) @@ -393,14 +372,13 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(hypervisor.Status.Evicted).To(BeTrue()) }) - It("should set the ConditionTypeReady to false and reason to evicted", func(ctx SpecContext) { + It("should set the ConditionTypeEvicting to false when evicted", func(ctx SpecContext) { updated := &kvmv1.Hypervisor{} Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) Expect(updated.Status.Conditions).To(ContainElement( SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), + HaveField("Type", kvmv1.ConditionTypeEvicting), HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", kvmv1.ConditionReasonReadyEvicted), ))) }) }) diff --git a/internal/controller/offboarding_controller.go b/internal/controller/offboarding_controller.go index 8790dfd..14336b4 100644 --- a/internal/controller/offboarding_controller.go +++ b/internal/controller/offboarding_controller.go @@ -68,11 +68,14 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, nil } - if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeReady) { + // Check if offboarding has already started or completed + offboardedCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded) + if offboardedCondition == nil { + // Start offboarding return r.setOffboardingCondition(ctx, hv, "Hypervisor is being offboarded, removing host from nova") } - if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded) { + if offboardedCondition.Status == metav1.ConditionTrue { return ctrl.Result{}, nil } @@ -145,7 +148,7 @@ func (r *HypervisorOffboardingReconciler) Reconcile(ctx context.Context, req ctr func (r *HypervisorOffboardingReconciler) setOffboardingCondition(ctx context.Context, hv *kvmv1.Hypervisor, message string) (ctrl.Result, error) { base := hv.DeepCopy() meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, + Type: kvmv1.ConditionTypeOffboarded, Status: metav1.ConditionFalse, Reason: "Offboarding", Message: message, @@ -165,12 +168,6 @@ func (r *HypervisorOffboardingReconciler) markOffboarded(ctx context.Context, hv Reason: "Offboarded", Message: "Offboarding successful", }) - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: "Offboarded", - Message: "Offboarding successful", - }) if err := r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(OffboardingControllerName)); err != nil { return fmt.Errorf("cannot update hypervisor status due to %w", err) diff --git a/internal/controller/offboarding_controller_test.go b/internal/controller/offboarding_controller_test.go index 49107e0..06c38c4 100644 --- a/internal/controller/offboarding_controller_test.go +++ b/internal/controller/offboarding_controller_test.go @@ -175,7 +175,7 @@ var _ = Describe("Offboarding Controller", func() { }) }) - It("should set the hypervisor ready condition", func(ctx SpecContext) { + It("should set the hypervisor offboarding condition", func(ctx SpecContext) { _, err := offboardingReconciler.Reconcile(ctx, reconcileReq) Expect(err).NotTo(HaveOccurred()) @@ -183,7 +183,7 @@ var _ = Describe("Offboarding Controller", func() { Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed()) Expect(hypervisor.Status.Conditions).To(ContainElement( SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), + HaveField("Type", kvmv1.ConditionTypeOffboarded), HaveField("Status", metav1.ConditionFalse), HaveField("Reason", "Offboarding"), ), @@ -199,19 +199,13 @@ var _ = Describe("Offboarding Controller", func() { hypervisor := &kvmv1.Hypervisor{} Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed()) - Expect(hypervisor.Status.Conditions).To(ContainElements( + Expect(hypervisor.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeOffboarded), HaveField("Status", metav1.ConditionTrue), HaveField("Reason", "Offboarded"), HaveField("Message", "Offboarding successful"), ), - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", "Offboarded"), - HaveField("Message", "Offboarding successful"), - ), )) }) @@ -296,7 +290,7 @@ var _ = Describe("Offboarding Controller", func() { Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed()) Expect(hypervisor.Status.Conditions).To(ContainElement( SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), + HaveField("Type", kvmv1.ConditionTypeOffboarded), HaveField("Status", metav1.ConditionFalse), HaveField("Reason", "Offboarding"), HaveField("Message", ContainSubstring(expectedMessageSubstring)), @@ -310,6 +304,15 @@ var _ = Describe("Offboarding Controller", func() { Expect(k8sClient.Get(ctx, resourceName, hypervisor)).To(Succeed()) hypervisor.Spec.Maintenance = kvmv1.MaintenanceTermination Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) + + By("Setting initial offboarding condition") + meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOffboarded, + Status: metav1.ConditionFalse, + Reason: "Offboarding", + Message: "Hypervisor is being offboarded, removing host from nova", + }) + Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) }) Context("When getting hypervisor by name fails", func() { diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index faac00a..cc6bad6 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -110,12 +110,6 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) status := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) if status == nil { base := hv.DeepCopy() - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonOnboarding, - Message: "Onboarding in progress", - }) meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, @@ -151,18 +145,6 @@ func (r *OnboardingController) abortOnboarding(ctx context.Context, hv *kvmv1.Hy } base := hv.DeepCopy() - ready := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeReady) - if ready != nil { - // Only undo ones own readiness status reporting - if ready.Reason == kvmv1.ConditionReasonOnboarding { - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonOnboarding, - Message: "Onboarding aborted", - }) - } - } meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, @@ -354,13 +336,6 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri Message: "Onboarding completed", }) - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonReadyReady, - Message: "Hypervisor is ready", - }) - return ctrl.Result{}, r.patchStatus(ctx, hv, base) } diff --git a/internal/controller/onboarding_controller_test.go b/internal/controller/onboarding_controller_test.go index 962d66e..8c0003d 100644 --- a/internal/controller/onboarding_controller_test.go +++ b/internal/controller/onboarding_controller_test.go @@ -282,12 +282,7 @@ var _ = Describe("Onboarding Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) - Expect(hv.Status.Conditions).To(ContainElements( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", kvmv1.ConditionReasonOnboarding), - ), + Expect(hv.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeOnboarding), HaveField("Status", metav1.ConditionTrue), @@ -327,12 +322,7 @@ var _ = Describe("Onboarding Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) - Expect(hv.Status.Conditions).To(ContainElements( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Status", metav1.ConditionFalse), - HaveField("Reason", kvmv1.ConditionReasonOnboarding), - ), + Expect(hv.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeOnboarding), HaveField("Status", metav1.ConditionTrue), @@ -355,11 +345,6 @@ var _ = Describe("Onboarding Controller", func() { Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) hv.Status.HypervisorID = hypervisorId hv.Status.ServiceID = serviceId - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeReady, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonOnboarding, - }) meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue, @@ -503,11 +488,7 @@ var _ = Describe("Onboarding Controller", func() { By("Verifying final state") Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) - Expect(hv.Status.Conditions).To(ContainElements( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Status", metav1.ConditionTrue), - ), + Expect(hv.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeOnboarding), HaveField("Status", metav1.ConditionFalse), @@ -576,11 +557,7 @@ var _ = Describe("Onboarding Controller", func() { Expect(err).NotTo(HaveOccurred()) Expect(k8sClient.Get(ctx, namespacedName, hv)).To(Succeed()) - Expect(hv.Status.Conditions).To(ContainElements( - SatisfyAll( - HaveField("Type", kvmv1.ConditionTypeReady), - HaveField("Status", metav1.ConditionTrue), - ), + Expect(hv.Status.Conditions).To(ContainElement( SatisfyAll( HaveField("Type", kvmv1.ConditionTypeOnboarding), HaveField("Status", metav1.ConditionFalse), diff --git a/internal/controller/ready/controller.go b/internal/controller/ready/controller.go new file mode 100644 index 0000000..878e8ca --- /dev/null +++ b/internal/controller/ready/controller.go @@ -0,0 +1,225 @@ +/* +SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ready + +import ( + "context" + + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + k8sclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + logger "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" +) + +const ( + ControllerName = "ready" +) + +// StatusChangedPredicate triggers reconciliation only when the status subresource changes. +// This is the inverse of GenerationChangedPredicate which triggers on spec changes. +type StatusChangedPredicate struct{} + +var _ predicate.Predicate = StatusChangedPredicate{} + +func (StatusChangedPredicate) Create(_ event.CreateEvent) bool { + // Reconcile on create to compute initial Ready condition + return true +} + +func (StatusChangedPredicate) Delete(_ event.DeleteEvent) bool { + // No need to reconcile on delete + return false +} + +func (StatusChangedPredicate) Update(e event.UpdateEvent) bool { + if e.ObjectOld == nil || e.ObjectNew == nil { + return false + } + + oldHv, ok := e.ObjectOld.(*kvmv1.Hypervisor) + if !ok { + return false + } + newHv, ok := e.ObjectNew.(*kvmv1.Hypervisor) + if !ok { + return false + } + + // Trigger if status changed + if !equality.Semantic.DeepEqual(oldHv.Status, newHv.Status) { + return true + } + + // Also trigger if Maintenance field changed (affects Ready condition) + if oldHv.Spec.Maintenance != newHv.Spec.Maintenance { + return true + } + + return false +} + +func (StatusChangedPredicate) Generic(_ event.GenericEvent) bool { + return true +} + +// Controller reconciles the Ready condition based on other conditions +type Controller struct { + k8sclient.Client + Scheme *runtime.Scheme +} + +// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch +// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;list;watch;patch + +func (r *Controller) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := logger.FromContext(ctx).WithName(req.Name) + ctx = logger.IntoContext(ctx, log) + + hv := &kvmv1.Hypervisor{} + if err := r.Get(ctx, req.NamespacedName, hv); err != nil { + return ctrl.Result{}, k8sclient.IgnoreNotFound(err) + } + + base := hv.DeepCopy() + + // Compute Ready condition based on other conditions + readyCondition := ComputeReadyCondition(hv) + meta.SetStatusCondition(&hv.Status.Conditions, readyCondition) + + if equality.Semantic.DeepEqual(hv.Status, base.Status) { + return ctrl.Result{}, nil + } + + log.Info("Updating Ready condition", "status", readyCondition.Status, "reason", readyCondition.Reason) + return ctrl.Result{}, r.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(base, + k8sclient.MergeFromWithOptimisticLock{}), k8sclient.FieldOwner(ControllerName)) +} + +// ComputeReadyCondition determines the Ready condition based on other conditions +func ComputeReadyCondition(hv *kvmv1.Hypervisor) metav1.Condition { + // Priority 1: Offboarded + if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeOffboarded) { + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: "Offboarded", + Message: "Hypervisor has been offboarded", + } + } + + // Priority 2: Terminating + if meta.IsStatusConditionTrue(hv.Status.Conditions, kvmv1.ConditionTypeTerminating) { + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonTerminating, + Message: "Hypervisor is terminating", + } + } + + // Priority 3: Active onboarding (Status=True means onboarding is in progress) + onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) + if onboardingCondition != nil && onboardingCondition.Status == metav1.ConditionTrue { + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonOnboarding, + Message: "Onboarding in progress: " + onboardingCondition.Message, + } + } + + // Priority 4: Maintenance mode + if hv.Spec.Maintenance != kvmv1.MaintenanceUnset { + evictingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) + if evictingCondition != nil { + if evictingCondition.Status == metav1.ConditionTrue { + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonReadyEvicting, + Message: "Hypervisor is disabled and evicting", + } + } + if evictingCondition.Status == metav1.ConditionFalse { + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonReadyEvicted, + Message: "Hypervisor is disabled and evicted", + } + } + } + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonReadyMaintenance, + Message: "Hypervisor is in maintenance mode", + } + } + + // Priority 5: Onboarding not started, aborted, or not succeeded + if onboardingCondition == nil { + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonOnboarding, + Message: "Onboarding not started", + } + } + + if onboardingCondition.Reason == kvmv1.ConditionReasonAborted { + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonOnboarding, + Message: "Onboarding was aborted", + } + } + + if onboardingCondition.Reason != kvmv1.ConditionReasonSucceeded { + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonOnboarding, + Message: "Onboarding not yet completed", + } + } + + // Priority 6: All checks passed - Ready + return metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonReadyReady, + Message: "Hypervisor is ready", + } +} + +func (r *Controller) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + Named(ControllerName). + For(&kvmv1.Hypervisor{}, builder.WithPredicates(StatusChangedPredicate{})). + Complete(r) +} diff --git a/internal/controller/ready/controller_test.go b/internal/controller/ready/controller_test.go new file mode 100644 index 0000000..2a00f87 --- /dev/null +++ b/internal/controller/ready/controller_test.go @@ -0,0 +1,336 @@ +/* +SPDX-FileCopyrightText: Copyright 2024 SAP SE or an SAP affiliate company and cobaltcore-dev contributors +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ready + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/event" + + kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" +) + +func TestReadyController(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Ready Controller Suite") +} + +var _ = Describe("StatusChangedPredicate", func() { + var predicate StatusChangedPredicate + + BeforeEach(func() { + predicate = StatusChangedPredicate{} + }) + + Describe("Create", func() { + It("should return true for create events", func() { + e := event.CreateEvent{Object: &kvmv1.Hypervisor{}} + Expect(predicate.Create(e)).To(BeTrue()) + }) + }) + + Describe("Delete", func() { + It("should return false for delete events", func() { + e := event.DeleteEvent{Object: &kvmv1.Hypervisor{}} + Expect(predicate.Delete(e)).To(BeFalse()) + }) + }) + + Describe("Update", func() { + It("should return false when only non-maintenance spec changes", func() { + oldHv := &kvmv1.Hypervisor{ + Spec: kvmv1.HypervisorSpec{SkipTests: false}, + } + newHv := &kvmv1.Hypervisor{ + Spec: kvmv1.HypervisorSpec{SkipTests: true}, + } + e := event.UpdateEvent{ObjectOld: oldHv, ObjectNew: newHv} + Expect(predicate.Update(e)).To(BeFalse()) + }) + + It("should return true when status changes", func() { + oldHv := &kvmv1.Hypervisor{} + newHv := &kvmv1.Hypervisor{ + Status: kvmv1.HypervisorStatus{ + Conditions: []metav1.Condition{ + {Type: kvmv1.ConditionTypeOnboarding, Status: metav1.ConditionTrue}, + }, + }, + } + e := event.UpdateEvent{ObjectOld: oldHv, ObjectNew: newHv} + Expect(predicate.Update(e)).To(BeTrue()) + }) + + It("should return true when maintenance field changes", func() { + oldHv := &kvmv1.Hypervisor{ + Spec: kvmv1.HypervisorSpec{Maintenance: kvmv1.MaintenanceUnset}, + } + newHv := &kvmv1.Hypervisor{ + Spec: kvmv1.HypervisorSpec{Maintenance: kvmv1.MaintenanceManual}, + } + e := event.UpdateEvent{ObjectOld: oldHv, ObjectNew: newHv} + Expect(predicate.Update(e)).To(BeTrue()) + }) + + It("should return false when nothing changes", func() { + hv := &kvmv1.Hypervisor{ + Spec: kvmv1.HypervisorSpec{SkipTests: true}, + } + e := event.UpdateEvent{ObjectOld: hv, ObjectNew: hv} + Expect(predicate.Update(e)).To(BeFalse()) + }) + + It("should return false when ObjectOld is nil", func() { + e := event.UpdateEvent{ObjectOld: nil, ObjectNew: &kvmv1.Hypervisor{}} + Expect(predicate.Update(e)).To(BeFalse()) + }) + + It("should return false when ObjectNew is nil", func() { + e := event.UpdateEvent{ObjectOld: &kvmv1.Hypervisor{}, ObjectNew: nil} + Expect(predicate.Update(e)).To(BeFalse()) + }) + }) + + Describe("Generic", func() { + It("should return true for generic events", func() { + e := event.GenericEvent{Object: &kvmv1.Hypervisor{}} + Expect(predicate.Generic(e)).To(BeTrue()) + }) + }) +}) + +var _ = Describe("ComputeReadyCondition", func() { + var hv *kvmv1.Hypervisor + + BeforeEach(func() { + hv = &kvmv1.Hypervisor{} + }) + + Context("Priority 1: Offboarded", func() { + It("should return Ready=False with Reason=Offboarded when offboarded", func() { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOffboarded, + Status: metav1.ConditionTrue, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal("Offboarded")) + }) + }) + + Context("Priority 2: Terminating", func() { + It("should return Ready=False with Reason=Terminating when terminating", func() { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeTerminating, + Status: metav1.ConditionTrue, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonTerminating)) + }) + + It("should prioritize Offboarded over Terminating", func() { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOffboarded, + Status: metav1.ConditionTrue, + }) + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeTerminating, + Status: metav1.ConditionTrue, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Reason).To(Equal("Offboarded")) + }) + }) + + Context("Priority 3: Onboarding in progress", func() { + It("should return Ready=False when onboarding is in progress", func() { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonTesting, + Message: "Running smoke tests", + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonOnboarding)) + Expect(result.Message).To(ContainSubstring("in progress")) + }) + + It("should prioritize onboarding in progress over maintenance", func() { + hv.Spec.Maintenance = kvmv1.MaintenanceManual + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionTrue, + Reason: kvmv1.ConditionReasonTesting, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonOnboarding)) + Expect(result.Message).To(ContainSubstring("in progress")) + }) + }) + + Context("Priority 4: Maintenance mode", func() { + It("should return Ready=False with Reason=Evicting when evicting", func() { + hv.Spec.Maintenance = kvmv1.MaintenanceAuto + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionTrue, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonReadyEvicting)) + }) + + It("should return Ready=False with Reason=Evicted when evicted", func() { + hv.Spec.Maintenance = kvmv1.MaintenanceAuto + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionFalse, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonReadyEvicted)) + }) + + It("should return Ready=False with Reason=Maintenance when in maintenance without eviction", func() { + hv.Spec.Maintenance = kvmv1.MaintenanceManual + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonReadyMaintenance)) + }) + + It("should prioritize maintenance over onboarding not started", func() { + hv.Spec.Maintenance = kvmv1.MaintenanceManual + // No onboarding condition set + + result := ComputeReadyCondition(hv) + + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonReadyMaintenance)) + }) + + It("should prioritize maintenance over onboarding aborted", func() { + hv.Spec.Maintenance = kvmv1.MaintenanceManual + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonAborted, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonReadyMaintenance)) + }) + + It("should prioritize maintenance over onboarding not succeeded", func() { + hv.Spec.Maintenance = kvmv1.MaintenanceManual + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonFailed, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonReadyMaintenance)) + }) + }) + + Context("Priority 5: Onboarding not started/aborted/not succeeded", func() { + It("should return Ready=False when no onboarding condition exists", func() { + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonOnboarding)) + Expect(result.Message).To(ContainSubstring("not started")) + }) + + It("should return Ready=False when onboarding was aborted", func() { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonAborted, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonOnboarding)) + Expect(result.Message).To(ContainSubstring("aborted")) + }) + + It("should return Ready=False when onboarding has non-succeeded reason", func() { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonFailed, + }) + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionFalse)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonOnboarding)) + }) + }) + + Context("Priority 6: All checks pass - Ready", func() { + It("should return Ready=True when all conditions are satisfied", func() { + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeOnboarding, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonSucceeded, + }) + hv.Spec.Maintenance = kvmv1.MaintenanceUnset + + result := ComputeReadyCondition(hv) + + Expect(result.Type).To(Equal(kvmv1.ConditionTypeReady)) + Expect(result.Status).To(Equal(metav1.ConditionTrue)) + Expect(result.Reason).To(Equal(kvmv1.ConditionReasonReadyReady)) + }) + }) +})