diff --git a/internal/controller/node_eviction_label_controller.go b/internal/controller/node_eviction_label_controller.go index dd841920..cb3030ff 100644 --- a/internal/controller/node_eviction_label_controller.go +++ b/internal/controller/node_eviction_label_controller.go @@ -107,7 +107,7 @@ func (r *NodeEvictionLabelReconciler) Reconcile(ctx context.Context, req ctrl.Re } if value != "" { - err = disableInstanceHA(node) + err = disableInstanceHA(hv) if err != nil { return ctrl.Result{}, err } diff --git a/internal/controller/onboarding_controller.go b/internal/controller/onboarding_controller.go index df4283a8..43bd7f03 100644 --- a/internal/controller/onboarding_controller.go +++ b/internal/controller/onboarding_controller.go @@ -77,20 +77,6 @@ type OnboardingController struct { testNetworkClient *gophercloud.ServiceClient } -func getHypervisorAddress(node *corev1.Node) string { - for _, addr := range node.Status.Addresses { - if addr.Address == "" { - continue - } - - if addr.Type == corev1.NodeHostName { - return addr.Address - } - } - - return "" -} - // +kubebuilder:rbac:groups=kvm.cloud.sap,resources=hypervisors,verbs=get;list;watch;patch func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := logger.FromContext(ctx).WithName(req.Name) @@ -144,24 +130,14 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, r.Status().Update(ctx, hv) } - // TODO: cleanup node retrieval - node := &corev1.Node{} - if err := r.Get(ctx, types.NamespacedName{Name: hv.Name}, node); err != nil { - if k8sclient.IgnoreNotFound(err) == nil { - // Node not found, could be deleted - return ctrl.Result{}, nil - } - return ctrl.Result{}, err - } - switch status.Reason { case ConditionReasonInitial: return ctrl.Result{}, r.initialOnboarding(ctx, hv, computeHost) case ConditionReasonTesting: if hv.Spec.SkipTests { - return r.completeOnboarding(ctx, computeHost, node, hv) + return r.completeOnboarding(ctx, computeHost, hv) } else { - return r.smokeTest(ctx, node, hv, computeHost) + return r.smokeTest(ctx, hv, computeHost) } default: // Nothing to be done @@ -170,16 +146,7 @@ func (r *OnboardingController) Reconcile(ctx context.Context, req ctrl.Request) } func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1.Hypervisor, host string) error { - node := &corev1.Node{} - if err := r.Get(ctx, types.NamespacedName{Name: hv.Name}, node); err != nil { - if k8sclient.IgnoreNotFound(err) == nil { - // Node not found, could be deleted - return nil - } - return err - } - - zone, found := node.Labels[corev1.LabelTopologyZone] + zone, found := hv.Labels[corev1.LabelTopologyZone] if !found || zone == "" { return fmt.Errorf("cannot find availability-zone label %v on node", corev1.LabelTopologyZone) } @@ -193,11 +160,9 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1. return fmt.Errorf("failed to agg to availability-zone aggregate %w", err) } - if _, found := node.Labels[corev1.LabelTopologyZone]; found { - err = addToAggregate(ctx, r.computeClient, aggs, host, testAggregateName, "") - if err != nil { - return fmt.Errorf("failed to agg to test aggregate %w", err) - } + err = addToAggregate(ctx, r.computeClient, aggs, host, testAggregateName, "") + if err != nil { + return fmt.Errorf("failed to agg to test aggregate %w", err) } // The service may be forced down previously due to an HA event, @@ -220,10 +185,10 @@ func (r *OnboardingController) initialOnboarding(ctx context.Context, hv *kvmv1. return r.Status().Update(ctx, hv) } -func (r *OnboardingController) smokeTest(ctx context.Context, node *corev1.Node, hv *kvmv1.Hypervisor, host string) (ctrl.Result, error) { +func (r *OnboardingController) smokeTest(ctx context.Context, hv *kvmv1.Hypervisor, host string) (ctrl.Result, error) { log := logger.FromContext(ctx) - zone := node.Labels[corev1.LabelTopologyZone] - server, err := r.createOrGetTestServer(ctx, zone, host, node.UID) + zone := hv.Labels[corev1.LabelTopologyZone] + server, err := r.createOrGetTestServer(ctx, zone, host, hv.UID) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to create or get test instance %w", err) } @@ -288,13 +253,13 @@ func (r *OnboardingController) smokeTest(ctx context.Context, node *corev1.Node, return ctrl.Result{RequeueAfter: defaultWaitTime}, nil } - return r.completeOnboarding(ctx, host, node, hv) + return r.completeOnboarding(ctx, host, hv) default: return ctrl.Result{RequeueAfter: defaultWaitTime}, nil } } -func (r *OnboardingController) completeOnboarding(ctx context.Context, host string, node *corev1.Node, hv *kvmv1.Hypervisor) (ctrl.Result, error) { +func (r *OnboardingController) completeOnboarding(ctx context.Context, host string, hv *kvmv1.Hypervisor) (ctrl.Result, error) { log := logger.FromContext(ctx) serverPrefix := fmt.Sprintf("%v-%v", testPrefixName, host) @@ -331,7 +296,7 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri } log.Info("removed from test-aggregate", "name", testAggregateName) - err = enableInstanceHA(node) + err = enableInstanceHA(hv) log.Info("enabled instance-ha") if err != nil { return ctrl.Result{}, err @@ -356,12 +321,7 @@ func (r *OnboardingController) completeOnboarding(ctx context.Context, host stri } func (r *OnboardingController) ensureNovaProperties(ctx context.Context, hv *kvmv1.Hypervisor) error { - node := &corev1.Node{} - if err := r.Get(ctx, types.NamespacedName{Name: hv.Name}, node); err != nil { - return k8sclient.IgnoreNotFound(err) - } - - hypervisorAddress := getHypervisorAddress(node) + hypervisorAddress := hv.Labels[corev1.LabelHostname] if hypervisorAddress == "" { return errRequeue } diff --git a/internal/controller/utils.go b/internal/controller/utils.go index b351e9de..c897442a 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -31,6 +31,8 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + + kvmv1 "github.com/cobaltcore-dev/openstack-hypervisor-operator/api/v1" ) // setNodeLabels sets the labels on the node. @@ -51,17 +53,17 @@ func InstanceHaUrl(region, zone, hostname string) string { return fmt.Sprintf("https://kvm-ha-service-%v.%v.cloud.sap/api/hypervisors/%v", zone, region, hostname) } -func updateInstanceHA(node *corev1.Node, data string, acceptedCodes []int) error { - zone, found := node.Labels[corev1.LabelTopologyZone] +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 := node.Labels[corev1.LabelTopologyRegion] + region, found := hypervisor.Labels[corev1.LabelTopologyRegion] if !found { return fmt.Errorf("could not find label %v for node", corev1.LabelTopologyRegion) } - hostname, found := node.Labels[corev1.LabelHostname] + hostname, found := hypervisor.Labels[corev1.LabelHostname] if !found { return fmt.Errorf("could not find label %v for node", corev1.LabelHostname) } @@ -81,12 +83,12 @@ func updateInstanceHA(node *corev1.Node, data string, acceptedCodes []int) error return nil } -func enableInstanceHA(node *corev1.Node) error { - return updateInstanceHA(node, `{"enabled": true}`, []int{http.StatusOK}) +func enableInstanceHA(hypervisor *kvmv1.Hypervisor) error { + return updateInstanceHA(hypervisor, `{"enabled": true}`, []int{http.StatusOK}) } -func disableInstanceHA(node *corev1.Node) error { - return updateInstanceHA(node, `{"enabled": false}`, []int{http.StatusOK, http.StatusNotFound}) +func disableInstanceHA(hypervisor *kvmv1.Hypervisor) error { + return updateInstanceHA(hypervisor, `{"enabled": false}`, []int{http.StatusOK, http.StatusNotFound}) } // IsNodeConditionTrue returns true when the conditionType is present and set to `corev1.ConditionTrue`