diff --git a/api/v1/hypervisor_types.go b/api/v1/hypervisor_types.go index ffdfaeaa..0dd925a3 100644 --- a/api/v1/hypervisor_types.go +++ b/api/v1/hypervisor_types.go @@ -35,6 +35,13 @@ const ( ConditionReasonReadyReady = "ready" // or not ConditionReasonReadyMaintenance = "maintenance" + ConditionReasonReadyEvicted = "evicted" + + // HypervisorMaintenance "enum" + MaintenanceUnset = "" + MaintenanceManual = "manual" + MaintenanceAuto = "auto" + MaintenanceHA = "ha" ) // HypervisorSpec defines the desired state of Hypervisor diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index 336c8cd0..eac1d11d 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -24,11 +24,14 @@ import ( "context" "fmt" + "k8s.io/apimachinery/pkg/api/equality" + k8serrors "k8s.io/apimachinery/pkg/api/errors" "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" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logger "sigs.k8s.io/controller-runtime/pkg/log" "github.com/gophercloud/gophercloud/v2" @@ -39,7 +42,7 @@ import ( ) const ( - HypervisorMaintenanceControllerName = "HypervisorMaintenanceController" + HypervisorMaintenanceControllerName = "HypervisorMaintenance" ) type HypervisorMaintenanceController struct { @@ -50,6 +53,7 @@ type HypervisorMaintenanceController struct { // +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;create;update;patch;delete +// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=evictions,verbs=get;list;watch;create;update;patch;delete func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { hv := &kvmv1.Hypervisor{} @@ -68,28 +72,30 @@ func (hec *HypervisorMaintenanceController) Reconcile(ctx context.Context, req c return ctrl.Result{}, nil } - log := logger.FromContext(ctx). - WithName("HypervisorService") - ctx = logger.IntoContext(ctx, log) + old := hv.DeepCopy() - changed, err := hec.reconcileComputeService(ctx, hv) - if err != nil { + if err := hec.reconcileComputeService(ctx, hv); err != nil { return ctrl.Result{}, err } - if changed { - return ctrl.Result{}, hec.Status().Update(ctx, hv) - } else { + if err := hec.reconcileEviction(ctx, hv); err != nil { + return ctrl.Result{}, err + } + + if equality.Semantic.DeepEqual(hv, old) { return ctrl.Result{}, nil } + + return ctrl.Result{}, hec.Status().Patch(ctx, hv, k8sclient.MergeFromWithOptions(old, k8sclient.MergeFromWithOptimisticLock{})) } -func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) (bool, error) { +func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context.Context, hv *kvmv1.Hypervisor) error { log := logger.FromContext(ctx) serviceId := hv.Status.ServiceID switch hv.Spec.Maintenance { - case "": // Enable the compute service (in case we haven't done so already) + case kvmv1.MaintenanceUnset: + // Enable the compute service (in case we haven't done so already) if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeHypervisorDisabled, Status: metav1.ConditionFalse, @@ -97,7 +103,7 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. Reason: kvmv1.ConditionReasonSucceeded, }) { // Spec matches status - return false, nil + return nil } meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ @@ -112,9 +118,13 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. log.Info("Enabling hypervisor", "id", serviceId) _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract() if err != nil { - return false, fmt.Errorf("failed to enable hypervisor due to %w", err) + return fmt.Errorf("failed to enable hypervisor due to %w", err) } - case "manual", "auto", "ha": // Disable the compute service + case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceHA: + // Disable the compute service: + // Also in case of HA, as it doesn't hurt to disable it twice, and this + // allows us to enable the service again, when the maintenance field is + // cleared in the case above. if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeHypervisorDisabled, Status: metav1.ConditionTrue, @@ -122,7 +132,7 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. Reason: kvmv1.ConditionReasonSucceeded, }) { // Spec matches status - return false, nil + return nil } meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ @@ -140,11 +150,102 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. log.Info("Disabling hypervisor", "id", serviceId) _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract() if err != nil { - return false, fmt.Errorf("failed to disable hypervisor due to %w", err) + return fmt.Errorf("failed to disable hypervisor due to %w", err) + } + } + + return nil +} + +func (hec *HypervisorMaintenanceController) reconcileEviction(ctx context.Context, hv *kvmv1.Hypervisor) error { + eviction := &kvmv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{ + Name: hv.Name, + }, + } + + switch hv.Spec.Maintenance { + case kvmv1.MaintenanceUnset: + // Avoid deleting the eviction over and over. + if hv.Status.Evicted || meta.RemoveStatusCondition(&hv.Status.Conditions, kvmv1.ConditionTypeEvicting) { + err := k8sclient.IgnoreNotFound(hec.Delete(ctx, eviction)) + hv.Status.Evicted = false + return err + } + return nil + case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto: + // In case of "ha", the host gets emptied from the HA service + if cond := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeEvicting); cond != nil { + if cond.Reason == kvmv1.ConditionReasonSucceeded { + // We are done here, no need to look at the eviction any more + return nil + } + } + status, err := hec.ensureEviction(ctx, eviction, hv) + if err != nil { + return err + } + var reason, message string + + if status == metav1.ConditionFalse { + message = "Evicted" + reason = kvmv1.ConditionReasonSucceeded + hv.Status.Evicted = true + } else { + message = "Evicting" + reason = kvmv1.ConditionReasonRunning + hv.Status.Evicted = false + } + + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: status, + Reason: reason, + Message: message, + }) + + meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeReady, + Status: metav1.ConditionFalse, + Reason: kvmv1.ConditionReasonReadyEvicted, + Message: "Hypervisor is disabled and evicted", + }) + + return nil + } + + return nil +} + +func (hec *HypervisorMaintenanceController) ensureEviction(ctx context.Context, eviction *kvmv1.Eviction, hypervisor *kvmv1.Hypervisor) (metav1.ConditionStatus, error) { + log := logger.FromContext(ctx) + if err := hec.Get(ctx, k8sclient.ObjectKeyFromObject(eviction), eviction); err != nil { + if !k8serrors.IsNotFound(err) { + return metav1.ConditionUnknown, fmt.Errorf("failed to get eviction due to %w", err) + } + if err := controllerutil.SetControllerReference(hypervisor, eviction, hec.Scheme); err != nil { + return metav1.ConditionUnknown, err + } + log.Info("Creating new eviction", "name", eviction.Name) + eviction.Spec = kvmv1.EvictionSpec{ + Hypervisor: hypervisor.Name, + Reason: "openstack-hypervisor-operator maintenance", + } + + // This also transports the label-selector, if set + transportLabels(&eviction.ObjectMeta, hypervisor) + + if err = hec.Create(ctx, eviction); err != nil { + return metav1.ConditionUnknown, fmt.Errorf("failed to create eviction due to %w", err) } } - return true, nil + // check if we are still evicting (defaulting to yes) + if meta.IsStatusConditionFalse(eviction.Status.Conditions, kvmv1.ConditionTypeEvicting) { + return metav1.ConditionFalse, nil + } else { + return metav1.ConditionTrue, nil + } } // SetupWithManager sets up the controller with the Manager. @@ -161,5 +262,6 @@ func (hec *HypervisorMaintenanceController) SetupWithManager(mgr ctrl.Manager) e return ctrl.NewControllerManagedBy(mgr). Named(HypervisorMaintenanceControllerName). For(&kvmv1.Hypervisor{}). + Owns(&kvmv1.Eviction{}). // trigger Reconcile whenever an Own-ed eviction is created/updated/deleted Complete(hec) } diff --git a/internal/controller/hypervisor_maintenance_controller_test.go b/internal/controller/hypervisor_maintenance_controller_test.go index 7df04bda..18533062 100644 --- a/internal/controller/hypervisor_maintenance_controller_test.go +++ b/internal/controller/hypervisor_maintenance_controller_test.go @@ -30,6 +30,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" + k8sclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) @@ -72,7 +74,7 @@ var _ = Describe("HypervisorMaintenanceController", func() { w.WriteHeader(http.StatusOK) _, err = fmt.Fprint(w, ServiceEnabledResponse) - Expect(err).NotTo(HaveOccurred()) + Expect(err).To(Succeed()) }) } @@ -113,6 +115,16 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(err).NotTo(HaveOccurred()) }) + AfterEach(func(ctx context.Context) { + eviction := &kvmv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{ + Name: hypervisorName.Name, + Namespace: hypervisorName.Namespace, + }, + } + Expect(k8sclient.IgnoreNotFound(k8sClient.Delete(ctx, eviction))).To(Succeed()) + }) + // Tests Context("Onboarded Hypervisor", func() { BeforeEach(func() { @@ -127,7 +139,6 @@ var _ = Describe("HypervisorMaintenanceController", func() { Message: "random text", }, ) - Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) }) @@ -169,9 +180,6 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) expectedBody := fmt.Sprintf(`{"disabled_reason": "Hypervisor CRD: spec.maintenance=%v", "status": "disabled"}`, mode) mockServiceUpdate(expectedBody) - req := ctrl.Request{NamespacedName: hypervisorName} - _, err := controller.Reconcile(ctx, req) - Expect(err).NotTo(HaveOccurred()) }) It("should set the ConditionTypeHypervisorDisabled to true", func() { @@ -192,5 +200,153 @@ var _ = Describe("HypervisorMaintenanceController", func() { }) // Spec.Maintenance="" } + Describe("Eviction reconciliation", func() { + Context("Spec.Maintenance=\"\"", func() { + BeforeEach(func() { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Spec.Maintenance = "" + Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + meta.SetStatusCondition(&hypervisor.Status.Conditions, + metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Reason: "dontcare", + Status: metav1.ConditionUnknown, + Message: "dontcare", + }) + Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) + expectedBody := `{"status": "enabled"}` + mockServiceUpdate(expectedBody) + + eviction := &kvmv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name}, + Spec: kvmv1.EvictionSpec{ + Hypervisor: hypervisorName.Name, + Reason: "test", + }, + } + Expect(controllerutil.SetControllerReference(hypervisor, eviction, controller.Scheme)).To(Succeed()) + Expect(k8sClient.Create(ctx, eviction)).To(Succeed()) + }) + + It("should delete the created eviction", func() { + eviction := &kvmv1.Eviction{} + err := k8sClient.Get(ctx, hypervisorName, eviction) + By(fmt.Sprintf("%+v", *eviction)) + Expect(err).To(HaveOccurred()) + Expect(k8sclient.IgnoreNotFound(err)).To(Succeed()) + }) + }) // Spec.Maintenance="" + + Context("Spec.Maintenance=\"ha\"", func() { + BeforeEach(func() { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Spec.Maintenance = "ha" + Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) + expectedBody := `{"disabled_reason": "Hypervisor CRD: spec.maintenance=ha", "status": "disabled"}` + mockServiceUpdate(expectedBody) + }) + It("should not create an eviction resource", func() { + eviction := &kvmv1.Eviction{} + err := k8sClient.Get(ctx, hypervisorName, eviction) + Expect(err).To(HaveOccurred()) + Expect(k8sclient.IgnoreNotFound(err)).To(Succeed()) + }) + }) // Spec.Maintenance="ha" + + for _, mode := range []string{"auto", "manual"} { + Context(fmt.Sprintf("Spec.Maintenance=\"%v\"", mode), func() { + BeforeEach(func() { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Spec.Maintenance = mode + Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) + expectedBody := fmt.Sprintf(`{"disabled_reason": "Hypervisor CRD: spec.maintenance=%v", "status": "disabled"}`, mode) + mockServiceUpdate(expectedBody) + }) + + When("there is no eviction yet", func() { + It("should create an eviction resource named as the hypervisor", func() { + eviction := &kvmv1.Eviction{} + Expect(k8sClient.Get(ctx, hypervisorName, eviction)).To(Succeed()) + }) + + It("should create an evicting condition", func() { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(hypervisor.Status.Conditions).To(ContainElement( + SatisfyAll( + HaveField("Type", kvmv1.ConditionTypeEvicting), + HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", kvmv1.ConditionReasonRunning), + ), + )) + }) + + It("should reflect it in the hypervisor evicted status", func() { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(hypervisor.Status.Evicted).To(BeFalse()) + }) + }) + + When("there is a finished eviction", func() { + BeforeEach(func() { + eviction := &kvmv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{Name: hypervisorName.Name}, + Spec: kvmv1.EvictionSpec{ + Hypervisor: hypervisorName.Name, + Reason: "test", + }, + } + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(controllerutil.SetControllerReference(hypervisor, eviction, controller.Scheme)).To(Succeed()) + Expect(k8sClient.Create(ctx, eviction)).To(Succeed()) + + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + meta.SetStatusCondition(&eviction.Status.Conditions, metav1.Condition{ + Type: kvmv1.ConditionTypeEvicting, + Status: metav1.ConditionFalse, + Message: "whatever", + Reason: kvmv1.ConditionReasonSucceeded, + }) + Expect(k8sClient.Status().Update(ctx, eviction)).To(Succeed()) + }) + + It("should reflect it in the hypervisor evicting condition", func() { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(hypervisor.Status.Conditions).To(ContainElement( + SatisfyAll( + HaveField("Type", kvmv1.ConditionTypeEvicting), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", kvmv1.ConditionReasonSucceeded), + ), + )) + }) + + It("should reflect it in the hypervisor evicted status", func() { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + Expect(hypervisor.Status.Evicted).To(BeTrue()) + }) + + It("should set the ConditionTypeReady to false and reason to evicted", func() { + 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.ConditionReasonReadyEvicted), + ))) + }) + }) + }) // Spec.Maintenance="" + } + }) }) // Context Onboarded Hypervisor })