diff --git a/cmd/main.go b/cmd/main.go index e0482b4f..6efef160 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -256,13 +256,6 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "Eviction") os.Exit(1) } - if err = (&controller.HypervisorInstanceHaController{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "unable to create controller", "controller", "HypervisorInstanceHa") - os.Exit(1) - } if err = (&controller.HypervisorOffboardingReconciler{ Client: mgr.GetClient(), diff --git a/go.mod b/go.mod index 3029f8ba..f9e4d88c 100644 --- a/go.mod +++ b/go.mod @@ -83,10 +83,10 @@ require ( go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.28.0 golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 // indirect - golang.org/x/net v0.54.0 // indirect + golang.org/x/net v0.55.0 // indirect golang.org/x/oauth2 v0.35.0 // indirect golang.org/x/sync v0.20.0 // indirect - golang.org/x/sys v0.44.0 // indirect + golang.org/x/sys v0.45.0 // indirect golang.org/x/term v0.43.0 // indirect golang.org/x/text v0.37.0 // indirect golang.org/x/time v0.15.0 // indirect diff --git a/go.sum b/go.sum index d53c5a84..2de6254c 100644 --- a/go.sum +++ b/go.sum @@ -195,14 +195,14 @@ golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93 h1:fQsdNF2N+/YewlRZiricy4P1i golang.org/x/exp v0.0.0-20251219203646-944ab1f22d93/go.mod h1:EPRbTFwzwjXj9NpYyyrvenVh9Y+GFeEvMNh7Xuz7xgU= golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= -golang.org/x/net v0.54.0 h1:2zJIZAxAHV/OHCDTCOHAYehQzLfSXuf/5SoL/Dv6w/w= -golang.org/x/net v0.54.0/go.mod h1:Sj4oj8jK6XmHpBZU/zWHw3BV3abl4Kvi+Ut7cQcY+cQ= +golang.org/x/net v0.55.0 h1:bcvxaJn3e1U6InsFWt1JUq1aSjnRxLzT2rtD2KfkDF8= +golang.org/x/net v0.55.0/go.mod h1:L5U2KuzuOe1lY7Z+aWVIKK6qEeJXnXV9yzGA+WCHJww= golang.org/x/oauth2 v0.35.0 h1:Mv2mzuHuZuY2+bkyWXIHMfhNdJAdwW3FuWeCPYN5GVQ= golang.org/x/oauth2 v0.35.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= -golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= -golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.45.0 h1:dO4czNzziLiiXplLQgBCEpCvXQ3dnkn0SdaZSYdQ+FY= +golang.org/x/sys v0.45.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= diff --git a/internal/controller/hypervisor_instance_ha_controller.go b/internal/controller/hypervisor_instance_ha_controller.go deleted file mode 100644 index 9b622de5..00000000 --- a/internal/controller/hypervisor_instance_ha_controller.go +++ /dev/null @@ -1,160 +0,0 @@ -/* -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 controller - -import ( - "context" - "errors" - - "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" - k8sclient "sigs.k8s.io/controller-runtime/pkg/client" - logger "sigs.k8s.io/controller-runtime/pkg/log" - - kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" - "github.com/cobaltcore-dev/openstack-hypervisor-operator/internal/utils" -) - -const ( - HypervisorInstanceHaControllerName = "HypervisorInstanceHa" -) - -type HypervisorInstanceHaController struct { - k8sclient.Client - Scheme *runtime.Scheme -} - -// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch;update;patch -// +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors/status,verbs=get;list;watch;update;patch -func (r *HypervisorInstanceHaController) 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 { - // ignore not found errors, could be deleted - return ctrl.Result{}, k8sclient.IgnoreNotFound(err) - } - - onboardingCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeOnboarding) - - if onboardingCondition == nil { - return ctrl.Result{}, nil // Onboarding not started, so no hypervisor and nothing to do - } - - old := hv.DeepCopy() - - // Determine if HA should be disabled and the reason - evicted := meta.IsStatusConditionFalse(hv.Status.Conditions, kvmv1.ConditionTypeEvicting) - testing := onboardingCondition.Status == metav1.ConditionTrue && - onboardingCondition.Reason != kvmv1.ConditionReasonHandover // Onboarding still testing (not yet in Handover phase) - aborted := onboardingCondition.Status == metav1.ConditionFalse && - onboardingCondition.Reason == kvmv1.ConditionReasonAborted // Onboarding was aborted - shouldDisable := !hv.Spec.HighAvailability || // HA not requested - evicted || // HA not needed as it is empty - testing || // HA not needed for test VMs - aborted // HA not needed when onboarding aborted - - if shouldDisable { - // Determine the reason based on why HA is being disabled - var reason string - var message string - switch { - case evicted: - reason = kvmv1.ConditionReasonHaEvicted - message = "HA disabled due to eviction" - case testing, aborted: - reason = kvmv1.ConditionReasonHaOnboarding - message = "HA disabled before onboarding" - default: - reason = kvmv1.ConditionReasonSucceeded - message = "HA disabled per spec" - } - - if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeHaEnabled, - Status: metav1.ConditionFalse, - Message: message, - Reason: reason, - }) { - // Desired state already achieved - return ctrl.Result{}, nil - } - - if err := disableInstanceHA(hv); err != nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeHaEnabled, - Status: metav1.ConditionUnknown, - Message: err.Error(), - Reason: kvmv1.ConditionReasonFailed, - } - - patchErr := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, HypervisorInstanceHaControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) - return ctrl.Result{}, errors.Join(err, patchErr) - } - } else { - if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeHaEnabled, - Status: metav1.ConditionTrue, - Message: "HA is enabled", - Reason: kvmv1.ConditionReasonSucceeded, - }) { - // Desired state already achieved - return ctrl.Result{}, nil - } - - if err := enableInstanceHA(hv); err != nil { - condition := metav1.Condition{ - Type: kvmv1.ConditionTypeHaEnabled, - Status: metav1.ConditionUnknown, - Message: err.Error(), - Reason: kvmv1.ConditionReasonFailed, - } - - patchErr := utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, HypervisorInstanceHaControllerName, func(h *kvmv1.Hypervisor) { - meta.SetStatusCondition(&h.Status.Conditions, condition) - }) - return ctrl.Result{}, errors.Join(err, patchErr) - } - } - - if equality.Semantic.DeepEqual(hv, old) { - return ctrl.Result{}, nil - } - - // Only set the HaEnabled condition this controller owns - haCondition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - return ctrl.Result{}, utils.PatchHypervisorStatusWithRetry(ctx, r.Client, hv.Name, HypervisorInstanceHaControllerName, func(h *kvmv1.Hypervisor) { - if haCondition != nil { - meta.SetStatusCondition(&h.Status.Conditions, *haCondition) - } - }) -} - -// SetupWithManager sets up the controller with the Manager. -func (r *HypervisorInstanceHaController) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - Named(HypervisorInstanceHaControllerName). - For(&kvmv1.Hypervisor{}). // trigger the r.Reconcile whenever a hypervisor is created/updated/deleted. - Complete(r) -} diff --git a/internal/controller/hypervisor_instance_ha_controller_test.go b/internal/controller/hypervisor_instance_ha_controller_test.go deleted file mode 100644 index 0487b057..00000000 --- a/internal/controller/hypervisor_instance_ha_controller_test.go +++ /dev/null @@ -1,364 +0,0 @@ -/* -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 controller - -import ( - "context" - "fmt" - "net/http" - "os" - - "github.com/gophercloud/gophercloud/v2/testhelper" - . "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" - - kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" -) - -var _ = Describe("HypervisorInstanceHaController", Label("test"), func() { - const ( - EOF = "EOF" - hostName = "test-hostname" - region = "region" - zone = "zone" - ) - var ( - controller *HypervisorInstanceHaController - fakeServer testhelper.FakeServer - hypervisorName = types.NamespacedName{Name: "test-node"} - numApiRequest = 0 - ) - - mockApi := func(expectedBody string) { - fakeServer.Mux.HandleFunc(fmt.Sprintf("POST /api/hypervisors/%v", hostName), func(w http.ResponseWriter, r *http.Request) { - numApiRequest++ - Expect(r.Header.Get("Content-Type")).To(Equal("application/json")) - body := make([]byte, r.ContentLength) - _, err := r.Body.Read(body) - Expect(err == nil || err.Error() == EOF).To(BeTrue()) - Expect(string(body)).To(MatchJSON(expectedBody)) - - w.Header().Add("Content-Type", "application/json") - _, err = fmt.Fprint(w, `{}`) - Expect(err).NotTo(HaveOccurred()) - }) - } - - mockApiDisable := func() { - mockApi(`{"enabled": false}`) - } - - mockApiEnable := func() { - mockApi(`{"enabled": true}`) - } - - BeforeEach(func(ctx SpecContext) { - fakeServer = testhelper.SetupHTTP() - numApiRequest = 0 - DeferCleanup(fakeServer.Teardown) - - Expect(os.Setenv("KVM_HA_SERVICE_URL", fakeServer.Endpoint())).To(Succeed()) - DeferCleanup(os.Unsetenv, "KVM_HA_SERVICE_URL") - - controller = &HypervisorInstanceHaController{ - Client: k8sClient, - Scheme: k8sClient.Scheme(), - } - - By("creating the hypervisor resource with ha-enabled") - hypervisor := &kvmv1.Hypervisor{ - ObjectMeta: metav1.ObjectMeta{ - Name: hypervisorName.Name, - Labels: map[string]string{ - corev1.LabelHostname: hostName, - corev1.LabelTopologyRegion: region, - corev1.LabelTopologyZone: zone, - }, - }, - Spec: kvmv1.HypervisorSpec{ - HighAvailability: true, - }, - } - Expect(k8sClient.Create(ctx, hypervisor)).To(Succeed()) - DeferCleanup(func(ctx SpecContext) { - Expect(k8sClient.Delete(ctx, hypervisor)).To(Succeed()) - }) - - By("setting onboarding to completed successfully") - meta.SetStatusCondition(&hypervisor.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonSucceeded, - Message: "Onboarding succeeded", - }) - Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) - }) - - Context("Happy path", func() { - JustBeforeEach(func(ctx context.Context) { - By("reconciling twice") - for range 2 { - _, err := controller.Reconcile(ctx, ctrl.Request{NamespacedName: hypervisorName}) - Expect(err).To(Succeed()) - } - }) - - Context("with ha disabled", func() { - BeforeEach(func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - By("setting hv.Spec.HighAvailability=false") - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - hv.Spec.HighAvailability = false - Expect(k8sClient.Update(ctx, hv)).To(Succeed()) - - mockApiDisable() - }) - - It("should have called the api server once", func() { - Expect(numApiRequest).To(Equal(1)) - }) - - It("should reflect the disabled state in the status with Succeeded reason", func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - - condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - Expect(condition).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionFalse)) - Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonSucceeded)) - Expect(condition.Message).To(Equal("HA disabled per spec")) - }) - }) - - Context("with hypervisor evicted", func() { - BeforeEach(func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - By("setting ConditionTypeEvicting to false (evicted)") - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeEvicting, - Status: metav1.ConditionFalse, - Reason: "dontcare", - Message: "dontcare", - }) - Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) - - mockApiDisable() - }) - - It("should have called the api server once", func() { - Expect(numApiRequest).To(Equal(1)) - }) - - It("should reflect the disabled state in the status with Evicted reason", func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - - condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - Expect(condition).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionFalse)) - Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonHaEvicted)) - Expect(condition.Message).To(Equal("HA disabled due to eviction")) - }) - }) - - Context("with hypervisor in Initial/Testing phase during onboarding", func() { - BeforeEach(func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - By("setting onboarding to Testing (still in progress)") - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonTesting, - Message: "Testing in progress", - }) - Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) - - mockApiDisable() - }) - - It("should have called the api server once", func() { - Expect(numApiRequest).To(Equal(1)) - }) - - It("should reflect the disabled state in the status with Onboarding reason", func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - - condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - Expect(condition).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionFalse)) - Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonHaOnboarding)) - Expect(condition.Message).To(Equal("HA disabled before onboarding")) - }) - }) - - Context("with hypervisor in Handover phase during onboarding", func() { - BeforeEach(func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - By("setting onboarding to Handover (ready for HA)") - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionTrue, - Reason: kvmv1.ConditionReasonHandover, - Message: "Handover in progress", - }) - Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) - - mockApiEnable() - }) - - It("should have called the api server once", func() { - Expect(numApiRequest).To(Equal(1)) - }) - - It("should reflect the enabled state in the status with Succeeded reason", func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - - condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - Expect(condition).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionTrue)) - Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonSucceeded)) - Expect(condition.Message).To(Equal("HA is enabled")) - }) - }) - - Context("with onboarding aborted", func() { - BeforeEach(func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - By("setting onboarding to aborted") - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ - Type: kvmv1.ConditionTypeOnboarding, - Status: metav1.ConditionFalse, - Reason: kvmv1.ConditionReasonAborted, - Message: "Onboarding aborted", - }) - Expect(k8sClient.Status().Update(ctx, hv)).To(Succeed()) - - mockApiDisable() - }) - - It("should have called the api server once", func() { - Expect(numApiRequest).To(Equal(1)) - }) - - It("should reflect the disabled state in the status with Onboarding reason", func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - - condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - Expect(condition).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionFalse)) - Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonHaOnboarding)) - Expect(condition.Message).To(Equal("HA disabled before onboarding")) - }) - }) - - Context("with ha enabled", func() { - BeforeEach(func() { - mockApiEnable() - }) - - It("should have called the api server once", func() { - Expect(numApiRequest).To(Equal(1)) - }) - - It("should reflect the enabled state in the status with Succeeded reason", func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - - condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - Expect(condition).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionTrue)) - Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonSucceeded)) - Expect(condition.Message).To(Equal("HA is enabled")) - }) - }) - }) - - Context("Failure modes", func() { - JustBeforeEach(func(ctx context.Context) { - By("reconciling once (expecting error)") - //nolint:errcheck // Ignoring error - failure tests expect errors - controller.Reconcile(ctx, ctrl.Request{NamespacedName: hypervisorName}) - }) - - Context("when enabling HA fails", func() { - BeforeEach(func(ctx SpecContext) { - // Mock API to return error BEFORE JustBeforeEach runs - fakeServer.Mux.HandleFunc(fmt.Sprintf("POST /api/hypervisors/%v", hostName), func(w http.ResponseWriter, r *http.Request) { - numApiRequest++ - w.WriteHeader(http.StatusInternalServerError) - _, err := fmt.Fprint(w, `{"error": "internal server error"}`) - Expect(err).NotTo(HaveOccurred()) - }) - }) - - It("should reflect the error in status with Unknown condition", func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - - condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - Expect(condition).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionUnknown)) - Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonFailed)) - Expect(condition.Message).To(ContainSubstring("unexpected response")) - Expect(numApiRequest).To(BeNumerically(">", 0)) - }) - }) - - Context("when disabling HA fails", func() { - BeforeEach(func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - By("setting hv.Spec.HighAvailability=false") - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - hv.Spec.HighAvailability = false - Expect(k8sClient.Update(ctx, hv)).To(Succeed()) - - // Mock API to return error BEFORE JustBeforeEach runs - fakeServer.Mux.HandleFunc(fmt.Sprintf("POST /api/hypervisors/%v", hostName), func(w http.ResponseWriter, r *http.Request) { - numApiRequest++ - w.WriteHeader(http.StatusInternalServerError) - _, err := fmt.Fprint(w, `{"error": "internal server error"}`) - Expect(err).NotTo(HaveOccurred()) - }) - }) - - It("should reflect the error in status with Unknown condition", func(ctx SpecContext) { - hv := &kvmv1.Hypervisor{} - Expect(k8sClient.Get(ctx, hypervisorName, hv)).To(Succeed()) - - condition := meta.FindStatusCondition(hv.Status.Conditions, kvmv1.ConditionTypeHaEnabled) - Expect(condition).NotTo(BeNil()) - Expect(condition.Status).To(Equal(metav1.ConditionUnknown)) - Expect(condition.Reason).To(Equal(kvmv1.ConditionReasonFailed)) - Expect(condition.Message).To(ContainSubstring("unexpected response")) - Expect(numApiRequest).To(BeNumerically(">", 0)) - }) - }) - }) -}) diff --git a/internal/controller/hypervisor_maintenance_controller.go b/internal/controller/hypervisor_maintenance_controller.go index c35b54e9..2d77cb0b 100644 --- a/internal/controller/hypervisor_maintenance_controller.go +++ b/internal/controller/hypervisor_maintenance_controller.go @@ -124,18 +124,20 @@ func (hec *HypervisorMaintenanceController) reconcileComputeService(ctx context. return nil } - // We need to enable the host as per spec - enableService := services.UpdateOpts{Status: services.ServiceEnabled} + // We need to enable the host as per spec and clear forced_down + // in case it was set by the HA service during maintenance. + falseVal := false + enableService := openstack.UpdateServiceOpts{ + Status: services.ServiceEnabled, + ForcedDown: &falseVal, + } log.Info("Enabling hypervisor", "id", serviceId) _, err := services.Update(ctx, hec.computeClient, serviceId, enableService).Extract() if err != nil { return fmt.Errorf("failed to enable hypervisor due to %w", err) } - case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceHA, kvmv1.MaintenanceTermination: - // 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. + case kvmv1.MaintenanceManual, kvmv1.MaintenanceAuto, kvmv1.MaintenanceTermination: + // Disable the compute service. if !meta.SetStatusCondition(&hv.Status.Conditions, metav1.Condition{ Type: kvmv1.ConditionTypeHypervisorDisabled, Status: metav1.ConditionTrue, diff --git a/internal/controller/hypervisor_maintenance_controller_test.go b/internal/controller/hypervisor_maintenance_controller_test.go index 86092176..bdbe6159 100644 --- a/internal/controller/hypervisor_maintenance_controller_test.go +++ b/internal/controller/hypervisor_maintenance_controller_test.go @@ -148,7 +148,7 @@ var _ = Describe("HypervisorMaintenanceController", func() { Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) hypervisor.Spec.Maintenance = "" Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) - expectedBody := `{"status": "enabled"}` + expectedBody := `{"status": "enabled", "forced_down": false}` mockServiceUpdate(expectedBody) }) @@ -160,7 +160,7 @@ var _ = Describe("HypervisorMaintenanceController", func() { }) // Spec.Maintenance="" }) - for _, mode := range []string{"auto", "manual", "ha"} { + for _, mode := range []string{"auto", "manual"} { Context(fmt.Sprintf("Spec.Maintenance=\"%v\"", mode), func() { BeforeEach(func(ctx SpecContext) { hypervisor := &kvmv1.Hypervisor{} @@ -182,6 +182,23 @@ var _ = Describe("HypervisorMaintenanceController", func() { }) // Spec.Maintenance="" } + Context("Spec.Maintenance=\"ha\"", func() { + BeforeEach(func(ctx SpecContext) { + hypervisor := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, hypervisor)).To(Succeed()) + hypervisor.Spec.Maintenance = kvmv1.MaintenanceHA + Expect(k8sClient.Update(ctx, hypervisor)).To(Succeed()) + }) + + It("should not call the Nova API (kvm-ha-service owns enable/disable for ha mode)", func(ctx SpecContext) { + // The controller takes no action for ha mode; kvm-ha-service handles it. + // Verify the hypervisor is still accessible and no condition is set by this controller. + updated := &kvmv1.Hypervisor{} + Expect(k8sClient.Get(ctx, hypervisorName, updated)).To(Succeed()) + Expect(meta.IsStatusConditionTrue(updated.Status.Conditions, kvmv1.ConditionTypeHypervisorDisabled)).To(BeFalse()) + }) + }) // Spec.Maintenance="ha" + Describe("Eviction reconciliation", func() { Context("Spec.Maintenance=\"\"", func() { BeforeEach(func(ctx SpecContext) { @@ -198,7 +215,7 @@ var _ = Describe("HypervisorMaintenanceController", func() { Message: "dontcare", }) Expect(k8sClient.Status().Update(ctx, hypervisor)).To(Succeed()) - expectedBody := `{"status": "enabled"}` + expectedBody := `{"status": "enabled", "forced_down": false}` mockServiceUpdate(expectedBody) eviction := &kvmv1.Eviction{ @@ -227,8 +244,6 @@ var _ = Describe("HypervisorMaintenanceController", func() { 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(ctx SpecContext) { eviction := &kvmv1.Eviction{} diff --git a/internal/controller/utils.go b/internal/controller/utils.go index c1ca4b47..ee861f63 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -18,72 +18,14 @@ limitations under the License. package controller import ( - "bytes" - "fmt" - "io" - "net/http" - "os" - "slices" "strings" - "time" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" v1ac "k8s.io/client-go/applyconfigurations/meta/v1" - - kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) -func InstanceHaUrl(region, zone, hostname string) string { - if haURL, found := os.LookupEnv("KVM_HA_SERVICE_URL"); found { - if !strings.HasSuffix(haURL, "/") { - haURL += "/" - } - return haURL + "api/hypervisors/" + hostname - } - return fmt.Sprintf("https://kvm-ha-service-%v.%v.cloud.sap/api/hypervisors/%v", zone, region, hostname) -} - -func updateInstanceHA(hypervisor *kvmv1.Hypervisor, data string, acceptedCodes []int) error { - zone, found := hypervisor.Labels[corev1.LabelTopologyZone] - if !found { - return fmt.Errorf("could not find label %v for node", corev1.LabelTopologyZone) - } - region, found := hypervisor.Labels[corev1.LabelTopologyRegion] - if !found { - return fmt.Errorf("could not find label %v for node", corev1.LabelTopologyRegion) - } - - hostname, found := hypervisor.Labels[corev1.LabelHostname] - if !found { - return fmt.Errorf("could not find label %v for node", corev1.LabelHostname) - } - - url := InstanceHaUrl(region, zone, hostname) - client := &http.Client{Timeout: 30 * time.Second} - // G107: Potential HTTP request made with variable url - resp, err := client.Post(url, "application/json", bytes.NewBuffer([]byte(data))) //nolint:bodyclose - if err != nil { - return fmt.Errorf("failed to send request to ha service due to %w", err) - } - defer func(Body io.ReadCloser) { - _ = Body.Close() - }(resp.Body) - if !slices.Contains(acceptedCodes, resp.StatusCode) { - return fmt.Errorf("ha service answered with unexpected response %v for %v from %v", resp.StatusCode, data, url) - } - return nil -} - -func enableInstanceHA(hypervisor *kvmv1.Hypervisor) error { - return updateInstanceHA(hypervisor, `{"enabled": true}`, []int{http.StatusOK}) -} - -func disableInstanceHA(hypervisor *kvmv1.Hypervisor) error { - return updateInstanceHA(hypervisor, `{"enabled": false}`, []int{http.StatusOK, http.StatusNotFound}) -} - // IsNodeConditionPresentAndEqual returns true when conditionType is present and equal to status. func IsNodeConditionPresentAndEqual(conditions []corev1.NodeCondition, conditionType corev1.NodeConditionType, status corev1.ConditionStatus) bool { for _, condition := range conditions {