diff --git a/cmd/main.go b/cmd/main.go index 179dda9..eaf34a4 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -45,7 +45,7 @@ import ( v1alpha1 "github.com/osac-project/bare-metal-fulfillment-operator/api/v1alpha1" "github.com/osac-project/host-management-openstack/internal/controller" - "github.com/osac-project/host-management-openstack/internal/ironic" + "github.com/osac-project/host-management-openstack/internal/management" "github.com/osac-project/osac-operator/pkg/aap" "github.com/osac-project/osac-operator/pkg/provisioning" // +kubebuilder:scaffold:imports @@ -233,15 +233,17 @@ func main() { os.Exit(1) } - // Ironic client for bare metal management - var ironicClient *ironic.Client - ironicCtx, ironicCancel := context.WithTimeout(context.Background(), 30*time.Second) - defer ironicCancel() - if ironicClient, err = ironic.NewClient(ironicCtx); err != nil { - setupLog.Error(err, "failed to create Ironic client") + mgmtCtx, mgmtCancel := context.WithTimeout(context.Background(), 30*time.Second) + defer mgmtCancel() + managementClient, err := management.NewOpenStackClient(mgmtCtx, &management.Config{ + Type: "openstack", + }) + if err != nil { + setupLog.V(1).Info("management client creation failed", "error", err) + setupLog.Error(nil, "failed to create management client (check cloud credentials and endpoint configuration)") os.Exit(1) } - setupLog.Info("Connect to ironic", "endpoint", ironicClient.GetEndpoint()) + setupLog.Info("Management client created", "type", "openstack") // AAP provisioning provider for image provisioning workflows var provisioningProvider provisioning.ProvisioningProvider @@ -272,7 +274,7 @@ func main() { hostLeaseReconciler := controller.NewHostLeaseReconciler( mgr.GetClient(), mgr.GetScheme(), - ironicClient, + managementClient, provisioningProvider, 0, // Use DefaultRecheckInterval ) diff --git a/internal/controller/hostlease_controller.go b/internal/controller/hostlease_controller.go index 5be6940..4ccdb34 100644 --- a/internal/controller/hostlease_controller.go +++ b/internal/controller/hostlease_controller.go @@ -24,7 +24,6 @@ import ( "time" "github.com/go-logr/logr" - "github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -37,30 +36,24 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" v1alpha1 "github.com/osac-project/bare-metal-fulfillment-operator/api/v1alpha1" - "github.com/osac-project/host-management-openstack/internal/ironic" + "github.com/osac-project/host-management-openstack/internal/management" opv1alpha1 "github.com/osac-project/osac-operator/api/v1alpha1" "github.com/osac-project/osac-operator/pkg/provisioning" ) -// HostLeaseReconciler reconciles HostLease CRs for power management via Ironic. type HostLeaseReconciler struct { client.Client - Scheme *runtime.Scheme - IronicClient ironic.NodeClient - ProvisioningProvider provisioning.ProvisioningProvider - - // RecheckInterval is the interval for polling Ironic until power state matches desired state. - RecheckInterval time.Duration - - // ProvisionPollInterval is the interval for polling AAP job status. + Scheme *runtime.Scheme + ManagementClient management.Client + ProvisioningProvider provisioning.ProvisioningProvider + RecheckInterval time.Duration ProvisionPollInterval time.Duration } -// NewHostLeaseReconciler creates a new HostLeaseReconciler with defaults applied. func NewHostLeaseReconciler( client client.Client, scheme *runtime.Scheme, - ironicClient ironic.NodeClient, + managementClient management.Client, provider provisioning.ProvisioningProvider, recheckInterval time.Duration, ) *HostLeaseReconciler { @@ -71,7 +64,7 @@ func NewHostLeaseReconciler( return &HostLeaseReconciler{ Client: client, Scheme: scheme, - IronicClient: ironicClient, + ManagementClient: managementClient, ProvisioningProvider: provider, RecheckInterval: recheckInterval, ProvisionPollInterval: DefaultProvisionPollInterval, @@ -143,33 +136,44 @@ func (r *HostLeaseReconciler) handleUpdate(ctx context.Context, hostLease *v1alp } } - node, err := r.IronicClient.GetNode(ctx, hostLease.Spec.ExternalHostID) + powerStatus, err := r.ManagementClient.GetPowerState(ctx, hostLease.Spec.ExternalHostID) if err != nil { - log.Error(err, "failed to get Ironic node", "nodeID", hostLease.Spec.ExternalHostID) + log.Error(err, "failed to get power state", "nodeID", hostLease.Spec.ExternalHostID) + r.syncHostLeaseStatus(hostLease, nil, err, log) + return ctrl.Result{}, err + } + if powerStatus == nil { + err := fmt.Errorf("management backend returned nil power status for host %s", hostLease.Spec.ExternalHostID) + log.Error(err, "unexpected nil power status", "nodeID", hostLease.Spec.ExternalHostID) r.syncHostLeaseStatus(hostLease, nil, err, log) return ctrl.Result{}, err } - log.V(1).Info("Ironic node", "nodeID", hostLease.Spec.ExternalHostID, "power_state", node.PowerState) + log.V(1).Info("Host power state", "nodeID", hostLease.Spec.ExternalHostID, "power_state", powerStatus.State) if hostLease.Spec.PoweredOn != nil { - if err := r.reconcilePower(ctx, hostLease, node, log); err != nil { + if err := r.reconcilePower(ctx, hostLease, powerStatus, log); err != nil { r.syncHostLeaseStatus(hostLease, nil, err, log) return ctrl.Result{}, err } - node, err = r.IronicClient.GetNode(ctx, hostLease.Spec.ExternalHostID) + powerStatus, err = r.ManagementClient.GetPowerState(ctx, hostLease.Spec.ExternalHostID) if err != nil { - log.Error(err, "failed to refresh node after power reconciliation", "nodeID", hostLease.Spec.ExternalHostID) + log.Error(err, "failed to refresh power state after reconciliation", "nodeID", hostLease.Spec.ExternalHostID) + r.syncHostLeaseStatus(hostLease, nil, err, log) + return ctrl.Result{}, err + } + if powerStatus == nil { + err := fmt.Errorf("management backend returned nil power status for host %s", hostLease.Spec.ExternalHostID) + log.Error(err, "unexpected nil power status after reconciliation", "nodeID", hostLease.Spec.ExternalHostID) r.syncHostLeaseStatus(hostLease, nil, err, log) return ctrl.Result{}, err } } - r.syncHostLeaseStatus(hostLease, node, nil, log) + r.syncHostLeaseStatus(hostLease, powerStatus, nil, log) if hostLease.Spec.PoweredOn != nil { - currentlyOn := node.PowerState == ironic.PowerOn.String() - if *hostLease.Spec.PoweredOn != currentlyOn { + if powerStatus.IsTransitioning || *hostLease.Spec.PoweredOn != (powerStatus.State == management.PowerOn) { hostLease.Status.Phase = v1alpha1.HostLeasePhaseProgressing return ctrl.Result{RequeueAfter: r.RecheckInterval}, nil } @@ -303,38 +307,41 @@ func (r *HostLeaseReconciler) validateOpenStackHost(hostLease *v1alpha1.HostLeas return true } -func (r *HostLeaseReconciler) reconcilePower(ctx context.Context, hostLease *v1alpha1.HostLease, node *nodes.Node, log logr.Logger) error { - currentlyOn := node.PowerState == ironic.PowerOn.String() +func (r *HostLeaseReconciler) reconcilePower(ctx context.Context, hostLease *v1alpha1.HostLease, powerStatus *management.PowerStatus, log logr.Logger) error { + currentlyOn := powerStatus.State == management.PowerOn desiredOn := *hostLease.Spec.PoweredOn - // If Ironic is already processing a power state change, skip to avoid 409 Conflict. - if r.IronicClient.IsNodePowerTransitioning(node) { + if powerStatus.IsTransitioning { log.V(1).Info("Node is transitioning, skipping power action", - "nodeID", hostLease.Spec.ExternalHostID, - "targetPowerState", node.TargetPowerState) + "nodeID", hostLease.Spec.ExternalHostID) return nil } - var err error needsPowerUpdate := desiredOn != currentlyOn if !needsPowerUpdate { - log.Info("Power state already matches desired", "poweredOn", desiredOn, "power_state", node.PowerState) + log.Info("Power state already matches desired", "poweredOn", desiredOn, "power_state", powerStatus.State) return nil } - targetState := ironic.PowerOff + targetState := management.PowerOff action := "off" if desiredOn { - targetState = ironic.PowerOn + targetState = management.PowerOn action = "on" } log.Info("Powering "+action+" node", "nodeID", hostLease.Spec.ExternalHostID) - if err = r.IronicClient.SetPowerState(ctx, hostLease.Spec.ExternalHostID, targetState); err != nil { + if err := r.ManagementClient.SetPowerState(ctx, hostLease.Spec.ExternalHostID, targetState); err != nil { + if errors.Is(err, management.ErrTransitioning) { + log.Info("Node is transitioning (conflict), will retry", + "nodeID", hostLease.Spec.ExternalHostID) + return nil + } log.Error(err, "failed to power "+action+" node", "nodeID", hostLease.Spec.ExternalHostID) + return err } - return err + return nil } func (r *HostLeaseReconciler) reconcileProvisioning(ctx context.Context, hostLease *v1alpha1.HostLease) (ctrl.Result, error) { @@ -393,8 +400,7 @@ func (r *HostLeaseReconciler) reconcileProvisioning(ctx context.Context, hostLea return result, nil } -// syncHostLeaseStatus syncs power-related conditions and observed power state in memory. -func (r *HostLeaseReconciler) syncHostLeaseStatus(hostLease *v1alpha1.HostLease, node *nodes.Node, reconcileErr error, log logr.Logger) { +func (r *HostLeaseReconciler) syncHostLeaseStatus(hostLease *v1alpha1.HostLease, powerStatus *management.PowerStatus, reconcileErr error, log logr.Logger) { if reconcileErr != nil { hostLease.Status.Phase = v1alpha1.HostLeasePhaseFailed hostLease.SetStatusCondition( @@ -407,13 +413,23 @@ func (r *HostLeaseReconciler) syncHostLeaseStatus(hostLease *v1alpha1.HostLease, return } - if node == nil { + if powerStatus == nil { return } - poweredOn := node.PowerState == ironic.PowerOn.String() + poweredOn := powerStatus.State == management.PowerOn hostLease.Status.PoweredOn = &poweredOn + if powerStatus.IsTransitioning { + hostLease.SetStatusCondition( + v1alpha1.HostConditionPowerSynced, + metav1.ConditionFalse, + v1alpha1.HostConditionReasonProgressing, + "node power state is transitioning", + ) + return + } + if hostLease.Spec.PoweredOn != nil && *hostLease.Spec.PoweredOn != poweredOn { hostLease.SetStatusCondition( v1alpha1.HostConditionPowerSynced, diff --git a/internal/controller/hostlease_controller_test.go b/internal/controller/hostlease_controller_test.go index 32f33ef..d203a96 100644 --- a/internal/controller/hostlease_controller_test.go +++ b/internal/controller/hostlease_controller_test.go @@ -19,12 +19,12 @@ package controller import ( "context" "errors" + "fmt" "time" . "github.com/onsi/ginkgo/v2" //nolint:revive,staticcheck . "github.com/onsi/gomega" //nolint:revive,staticcheck - "github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -37,7 +37,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" v1alpha1 "github.com/osac-project/bare-metal-fulfillment-operator/api/v1alpha1" - "github.com/osac-project/host-management-openstack/internal/ironic" + "github.com/osac-project/host-management-openstack/internal/management" opv1alpha1 "github.com/osac-project/osac-operator/api/v1alpha1" "github.com/osac-project/osac-operator/pkg/provisioning" @@ -45,41 +45,30 @@ import ( ) const ( - testPowerOff = "power off" - testPowerOn = "power on" testNodeID = "node-1" testNamespace = "default" testNoopTmpl = "noop" ) -// mockIronicClient implements ironic.NodeClient for testing. -type mockIronicClient struct { - getNodeFunc func(ctx context.Context, nodeID string) (*nodes.Node, error) - setPowerStateFunc func(ctx context.Context, nodeID string, target ironic.TargetPowerState) error - isNodePowerTransitioningFunc func(node *nodes.Node) bool +type mockManagementClient struct { + getPowerStateFunc func(ctx context.Context, hostID string) (*management.PowerStatus, error) + setPowerStateFunc func(ctx context.Context, hostID string, target management.PowerState) error } -func (m *mockIronicClient) GetNode(ctx context.Context, nodeID string) (*nodes.Node, error) { - if m.getNodeFunc != nil { - return m.getNodeFunc(ctx, nodeID) +func (m *mockManagementClient) GetPowerState(ctx context.Context, hostID string) (*management.PowerStatus, error) { + if m.getPowerStateFunc != nil { + return m.getPowerStateFunc(ctx, hostID) } - return &nodes.Node{PowerState: testPowerOff}, nil + return &management.PowerStatus{State: management.PowerOff}, nil } -func (m *mockIronicClient) SetPowerState(ctx context.Context, nodeID string, target ironic.TargetPowerState) error { +func (m *mockManagementClient) SetPowerState(ctx context.Context, hostID string, target management.PowerState) error { if m.setPowerStateFunc != nil { - return m.setPowerStateFunc(ctx, nodeID, target) + return m.setPowerStateFunc(ctx, hostID, target) } return nil } -func (m *mockIronicClient) IsNodePowerTransitioning(node *nodes.Node) bool { - if m.isNodePowerTransitioningFunc != nil { - return m.isNodePowerTransitioningFunc(node) - } - return node.TargetPowerState != "" -} - func boolPtr(b bool) *bool { return &b } @@ -87,7 +76,7 @@ func boolPtr(b bool) *bool { var _ = Describe("HostLeaseReconciler", func() { var ( reconciler *HostLeaseReconciler - mockIronic *mockIronicClient + mockMgmt *mockManagementClient testScheme *runtime.Scheme log = zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)) ) @@ -97,28 +86,28 @@ var _ = Describe("HostLeaseReconciler", func() { testScheme = runtime.NewScheme() Expect(v1alpha1.AddToScheme(testScheme)).To(Succeed()) - mockIronic = &mockIronicClient{} + mockMgmt = &mockManagementClient{} reconciler = &HostLeaseReconciler{ - Scheme: testScheme, - IronicClient: mockIronic, - RecheckInterval: 10 * time.Second, + Scheme: testScheme, + ManagementClient: mockMgmt, + RecheckInterval: 10 * time.Second, } }) Describe("NewHostLeaseReconciler", func() { It("should use the provided recheck interval when positive", func() { - r := NewHostLeaseReconciler(nil, testScheme, mockIronic, nil, 30*time.Second) + r := NewHostLeaseReconciler(nil, testScheme, mockMgmt, nil, 30*time.Second) Expect(r.RecheckInterval).To(Equal(30 * time.Second)) }) It("should use the default recheck interval when zero", func() { - r := NewHostLeaseReconciler(nil, testScheme, mockIronic, nil, 0) + r := NewHostLeaseReconciler(nil, testScheme, mockMgmt, nil, 0) Expect(r.RecheckInterval).To(Equal(DefaultRecheckInterval)) }) It("should store the provisioning provider", func() { mockProvider := &provisioning.AAPProvider{} - r := NewHostLeaseReconciler(nil, testScheme, mockIronic, mockProvider, 0) + r := NewHostLeaseReconciler(nil, testScheme, mockMgmt, mockProvider, 0) Expect(r.ProvisioningProvider).To(Equal(mockProvider)) }) }) @@ -173,109 +162,121 @@ var _ = Describe("HostLeaseReconciler", func() { It("should power on when desired on and currently off", func() { hostLease.Spec.PoweredOn = boolPtr(true) - node := &nodes.Node{PowerState: testPowerOff} + powerStatus := &management.PowerStatus{State: management.PowerOff} - var calledTarget ironic.TargetPowerState - mockIronic.setPowerStateFunc = func(ctx context.Context, nodeID string, target ironic.TargetPowerState) error { + var calledTarget management.PowerState + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, target management.PowerState) error { calledTarget = target return nil } - err := reconciler.reconcilePower(ctx, hostLease, node, log) + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) Expect(err).NotTo(HaveOccurred()) - Expect(calledTarget.String()).To(Equal(ironic.PowerOn.String())) + Expect(calledTarget).To(Equal(management.PowerOn)) }) It("should power off when desired off and currently on", func() { hostLease.Spec.PoweredOn = boolPtr(false) - node := &nodes.Node{PowerState: testPowerOn} + powerStatus := &management.PowerStatus{State: management.PowerOn} - var calledTarget ironic.TargetPowerState - mockIronic.setPowerStateFunc = func(ctx context.Context, nodeID string, target ironic.TargetPowerState) error { + var calledTarget management.PowerState + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, target management.PowerState) error { calledTarget = target return nil } - err := reconciler.reconcilePower(ctx, hostLease, node, log) + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) Expect(err).NotTo(HaveOccurred()) - Expect(calledTarget.String()).To(Equal(ironic.PowerOff.String())) + Expect(calledTarget).To(Equal(management.PowerOff)) }) It("should not call SetPowerState when power state already matches (on)", func() { hostLease.Spec.PoweredOn = boolPtr(true) - node := &nodes.Node{PowerState: testPowerOn} + powerStatus := &management.PowerStatus{State: management.PowerOn} called := false - mockIronic.setPowerStateFunc = func(ctx context.Context, nodeID string, target ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { called = true return nil } - err := reconciler.reconcilePower(ctx, hostLease, node, log) + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) Expect(err).NotTo(HaveOccurred()) Expect(called).To(BeFalse()) }) It("should not call SetPowerState when power state already matches (off)", func() { hostLease.Spec.PoweredOn = boolPtr(false) - node := &nodes.Node{PowerState: testPowerOff} + powerStatus := &management.PowerStatus{State: management.PowerOff} called := false - mockIronic.setPowerStateFunc = func(ctx context.Context, nodeID string, target ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { called = true return nil } - err := reconciler.reconcilePower(ctx, hostLease, node, log) + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) Expect(err).NotTo(HaveOccurred()) Expect(called).To(BeFalse()) }) It("should not be called when PoweredOn is nil (guarded by Reconcile)", func() { hostLease.Spec.PoweredOn = boolPtr(true) - node := &nodes.Node{PowerState: testPowerOn} + powerStatus := &management.PowerStatus{State: management.PowerOn} - err := reconciler.reconcilePower(ctx, hostLease, node, log) + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) Expect(err).NotTo(HaveOccurred()) }) It("should skip SetPowerState when node is transitioning", func() { hostLease.Spec.PoweredOn = boolPtr(true) - node := &nodes.Node{PowerState: testPowerOff, TargetPowerState: testPowerOn} + powerStatus := &management.PowerStatus{State: management.PowerOff, IsTransitioning: true} called := false - mockIronic.setPowerStateFunc = func(ctx context.Context, nodeID string, target ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { called = true return nil } - err := reconciler.reconcilePower(ctx, hostLease, node, log) + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) Expect(err).NotTo(HaveOccurred()) Expect(called).To(BeFalse()) }) + It("should not return error when SetPowerState returns ErrTransitioning", func() { + hostLease.Spec.PoweredOn = boolPtr(true) + powerStatus := &management.PowerStatus{State: management.PowerOff} + + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { + return fmt.Errorf("node %s: %w", testNodeID, management.ErrTransitioning) + } + + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) + Expect(err).NotTo(HaveOccurred()) + }) + It("should return error when SetPowerState fails on power on", func() { hostLease.Spec.PoweredOn = boolPtr(true) - node := &nodes.Node{PowerState: testPowerOff} + powerStatus := &management.PowerStatus{State: management.PowerOff} - mockIronic.setPowerStateFunc = func(ctx context.Context, nodeID string, target ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { return errors.New("ironic API error") } - err := reconciler.reconcilePower(ctx, hostLease, node, log) + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("ironic API error")) }) It("should return error when SetPowerState fails on power off", func() { hostLease.Spec.PoweredOn = boolPtr(false) - node := &nodes.Node{PowerState: testPowerOn} + powerStatus := &management.PowerStatus{State: management.PowerOn} - mockIronic.setPowerStateFunc = func(ctx context.Context, nodeID string, target ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { return errors.New("ironic API error") } - err := reconciler.reconcilePower(ctx, hostLease, node, log) + err := reconciler.reconcilePower(ctx, hostLease, powerStatus, log) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("ironic API error")) }) @@ -302,13 +303,13 @@ var _ = Describe("HostLeaseReconciler", func() { WithObjects(hostLease). Build() - getNodeCalls := 0 + getPowerStateCalls := 0 setPowerStateCalls := 0 - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - getNodeCalls++ - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + getPowerStateCalls++ + return &management.PowerStatus{State: management.PowerOff}, nil } - mockIronic.setPowerStateFunc = func(_ context.Context, _ string, _ ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { setPowerStateCalls++ return nil } @@ -321,11 +322,9 @@ var _ = Describe("HostLeaseReconciler", func() { }) Expect(err).NotTo(HaveOccurred()) Expect(result).To(Equal(ctrl.Result{})) - Expect(getNodeCalls).To(Equal(0)) + Expect(getPowerStateCalls).To(Equal(0)) Expect(setPowerStateCalls).To(Equal(0)) - // Fake client deletes the object once all finalizers are removed - // and DeletionTimestamp is set, so verify it no longer exists. updatedHostLease := &v1alpha1.HostLease{} err = reconciler.Get(context.Background(), types.NamespacedName{ Name: hostLease.Name, @@ -425,10 +424,10 @@ var _ = Describe("HostLeaseReconciler", func() { WithObjects(hostLease). Build() - getNodeCalls := 0 - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - getNodeCalls++ - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + getPowerStateCalls := 0 + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + getPowerStateCalls++ + return &management.PowerStatus{State: management.PowerOff}, nil } result, err := reconciler.Reconcile(context.Background(), ctrl.Request{ @@ -439,7 +438,7 @@ var _ = Describe("HostLeaseReconciler", func() { }) Expect(err).NotTo(HaveOccurred()) Expect(result).To(Equal(ctrl.Result{Requeue: true})) - Expect(getNodeCalls).To(Equal(0)) + Expect(getPowerStateCalls).To(Equal(0)) updatedHostLease := &v1alpha1.HostLease{} Expect(reconciler.Get(context.Background(), types.NamespacedName{ @@ -470,13 +469,13 @@ var _ = Describe("HostLeaseReconciler", func() { WithObjects(hostLease). Build() - getNodeCalls := 0 + getPowerStateCalls := 0 setPowerStateCalls := 0 - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - getNodeCalls++ - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + getPowerStateCalls++ + return &management.PowerStatus{State: management.PowerOff}, nil } - mockIronic.setPowerStateFunc = func(_ context.Context, _ string, _ ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { setPowerStateCalls++ return nil } @@ -489,7 +488,7 @@ var _ = Describe("HostLeaseReconciler", func() { }) Expect(err).NotTo(HaveOccurred()) Expect(result).To(Equal(ctrl.Result{})) - Expect(getNodeCalls).To(Equal(1)) + Expect(getPowerStateCalls).To(Equal(1)) Expect(setPowerStateCalls).To(Equal(0)) updatedHostLease := &v1alpha1.HostLease{} @@ -527,10 +526,10 @@ var _ = Describe("HostLeaseReconciler", func() { WithObjects(hostLease). Build() - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + return &management.PowerStatus{State: management.PowerOff}, nil } - mockIronic.setPowerStateFunc = func(_ context.Context, _ string, _ ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { Fail("SetPowerState should not be called when power already matches desired") return nil } @@ -583,15 +582,10 @@ var _ = Describe("HostLeaseReconciler", func() { WithObjects(hostLease). Build() - getNodeCalls := 0 - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - getNodeCalls++ - if getNodeCalls == 1 { - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil - } - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + return &management.PowerStatus{State: management.PowerOff}, nil } - mockIronic.setPowerStateFunc = func(_ context.Context, _ string, _ ironic.TargetPowerState) error { + mockMgmt.setPowerStateFunc = func(_ context.Context, _ string, _ management.PowerState) error { return nil } @@ -709,8 +703,8 @@ var _ = Describe("HostLeaseReconciler", func() { Build() reconciler.ProvisioningProvider = nil - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + return &management.PowerStatus{State: management.PowerOff}, nil } result, err := reconciler.Reconcile(context.Background(), ctrl.Request{ @@ -743,8 +737,8 @@ var _ = Describe("HostLeaseReconciler", func() { WithObjects(hostLease). Build() - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + return &management.PowerStatus{State: management.PowerOff}, nil } result, err := reconciler.Reconcile(context.Background(), ctrl.Request{ @@ -777,8 +771,8 @@ var _ = Describe("HostLeaseReconciler", func() { WithObjects(hostLease). Build() - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + return &management.PowerStatus{State: management.PowerOff}, nil } result, err := reconciler.Reconcile(context.Background(), ctrl.Request{ @@ -822,8 +816,8 @@ var _ = Describe("HostLeaseReconciler", func() { WithObjects(hostLease). Build() - mockIronic.getNodeFunc = func(_ context.Context, _ string) (*nodes.Node, error) { - return &nodes.Node{PowerState: ironic.PowerOff.String()}, nil + mockMgmt.getPowerStateFunc = func(_ context.Context, _ string) (*management.PowerStatus, error) { + return &management.PowerStatus{State: management.PowerOff}, nil } result, err := reconciler.Reconcile(context.Background(), ctrl.Request{ @@ -869,8 +863,8 @@ var _ = Describe("HostLeaseReconciler", func() { }) It("should set PowerSynced to True when node is on", func() { - node := &nodes.Node{PowerState: testPowerOn} - reconciler.syncHostLeaseStatus(hostLease, node, nil, log) + powerStatus := &management.PowerStatus{State: management.PowerOn} + reconciler.syncHostLeaseStatus(hostLease, powerStatus, nil, log) Expect(hostLease.Status.PoweredOn).NotTo(BeNil()) Expect(*hostLease.Status.PoweredOn).To(BeTrue()) @@ -882,8 +876,8 @@ var _ = Describe("HostLeaseReconciler", func() { }) It("should set PowerSynced to True when node is off", func() { - node := &nodes.Node{PowerState: testPowerOff} - reconciler.syncHostLeaseStatus(hostLease, node, nil, log) + powerStatus := &management.PowerStatus{State: management.PowerOff} + reconciler.syncHostLeaseStatus(hostLease, powerStatus, nil, log) Expect(hostLease.Status.PoweredOn).NotTo(BeNil()) Expect(*hostLease.Status.PoweredOn).To(BeFalse()) @@ -896,8 +890,20 @@ var _ = Describe("HostLeaseReconciler", func() { It("should set PowerSynced to False when power state does not match desired", func() { hostLease.Spec.PoweredOn = boolPtr(true) - node := &nodes.Node{PowerState: testPowerOff} - reconciler.syncHostLeaseStatus(hostLease, node, nil, log) + powerStatus := &management.PowerStatus{State: management.PowerOff} + reconciler.syncHostLeaseStatus(hostLease, powerStatus, nil, log) + + Expect(hostLease.Status.PoweredOn).NotTo(BeNil()) + Expect(*hostLease.Status.PoweredOn).To(BeFalse()) + condition := hostLease.GetStatusCondition(v1alpha1.HostConditionPowerSynced) + Expect(condition).NotTo(BeNil()) + Expect(condition.Status).To(Equal(metav1.ConditionFalse)) + Expect(condition.Reason).To(Equal(v1alpha1.HostConditionReasonProgressing)) + }) + + It("should set PowerSynced to False when node is transitioning", func() { + powerStatus := &management.PowerStatus{State: management.PowerOff, IsTransitioning: true} + reconciler.syncHostLeaseStatus(hostLease, powerStatus, nil, log) Expect(hostLease.Status.PoweredOn).NotTo(BeNil()) Expect(*hostLease.Status.PoweredOn).To(BeFalse()) @@ -905,9 +911,10 @@ var _ = Describe("HostLeaseReconciler", func() { Expect(condition).NotTo(BeNil()) Expect(condition.Status).To(Equal(metav1.ConditionFalse)) Expect(condition.Reason).To(Equal(v1alpha1.HostConditionReasonProgressing)) + Expect(condition.Message).To(Equal("node power state is transitioning")) }) - It("should not modify status when node is nil and no error", func() { + It("should not modify status when powerStatus is nil and no error", func() { reconciler.syncHostLeaseStatus(hostLease, nil, nil, log) Expect(hostLease.Status.PoweredOn).To(BeNil()) diff --git a/internal/ironic/client.go b/internal/ironic/client.go deleted file mode 100644 index d41af3a..0000000 --- a/internal/ironic/client.go +++ /dev/null @@ -1,168 +0,0 @@ -/* -Copyright 2026. - -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 ironic provides a client for interacting with OpenStack Ironic bare metal API. -package ironic - -import ( - "context" - "errors" - "fmt" - "net/http" - - "github.com/gophercloud/gophercloud/v2" - "github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes" - "github.com/gophercloud/utils/v2/openstack/clientconfig" - ctrllog "sigs.k8s.io/controller-runtime/pkg/log" -) - -// NodeClient is the interface for interacting with Ironic nodes. -type NodeClient interface { - GetNode(ctx context.Context, nodeID string) (*nodes.Node, error) - SetPowerState(ctx context.Context, nodeID string, target TargetPowerState) error - IsNodePowerTransitioning(node *nodes.Node) bool -} - -// Client talks to Ironic over REST via gophercloud. -type ( - Client struct { - serviceClient *gophercloud.ServiceClient - newServiceClient func(ctx context.Context) (*gophercloud.ServiceClient, error) - } - - TargetPowerState struct { - powerstate nodes.TargetPowerState - } -) - -func (t TargetPowerState) String() string { - return string(t.powerstate) -} - -var ( - PowerOn = TargetPowerState{nodes.PowerOn} - PowerOff = TargetPowerState{nodes.PowerOff} - Rebooting = TargetPowerState{nodes.Rebooting} - SoftPowerOff = TargetPowerState{nodes.SoftPowerOff} - SoftRebooting = TargetPowerState{nodes.SoftRebooting} -) - -// NewClient creates an Ironic client configured via clouds.yaml or OS_* environment variables. -func NewClient(ctx context.Context) (*Client, error) { - factory := func(ctx context.Context) (*gophercloud.ServiceClient, error) { - return clientconfig.NewServiceClient(ctx, "baremetal", nil) - } - sc, err := factory(ctx) - if err != nil { - return nil, fmt.Errorf("failed to create baremetal client: %w", err) - } - if sc == nil { - return nil, fmt.Errorf("failed to create baremetal client: nil service client") - } - if sc.Endpoint == "" { - return nil, fmt.Errorf("baremetal client has no endpoint configured") - } - return &Client{ - serviceClient: sc, - newServiceClient: factory, - }, nil -} - -func isAuthError(err error) bool { - if err == nil { - return false - } - if gophercloud.ResponseCodeIs(err, http.StatusUnauthorized) { - return true - } - var errReauth *gophercloud.ErrUnableToReauthenticate - if errors.As(err, &errReauth) { - return true - } - var errAfterReauth *gophercloud.ErrErrorAfterReauthentication - return errors.As(err, &errAfterReauth) -} - -func (c *Client) reconnect(ctx context.Context) error { - log := ctrllog.FromContext(ctx) - log.Info("recreating ironic service client after authentication failure") - sc, err := c.newServiceClient(ctx) - if err != nil { - log.Error(err, "failed to recreate ironic service client") - return fmt.Errorf("failed to recreate baremetal client: %w", err) - } - if sc == nil { - return fmt.Errorf("failed to recreate baremetal client: nil service client") - } - if sc.Endpoint == "" { - return fmt.Errorf("recreated baremetal client has no endpoint configured") - } - c.serviceClient = sc - log.Info("ironic service client reconnected successfully", "endpoint", sc.Endpoint) - return nil -} - -// GetNode fetches a node by UUID or name from Ironic. -func (c *Client) GetNode(ctx context.Context, nodeID string) (*nodes.Node, error) { - log := ctrllog.FromContext(ctx) - node, err := nodes.Get(ctx, c.serviceClient, nodeID).Extract() - if err != nil { - if isAuthError(err) { - log.Info("auth error on GetNode, attempting reconnect", "nodeID", nodeID, "error", err) - if reconnErr := c.reconnect(ctx); reconnErr != nil { - return nil, fmt.Errorf("get node %s: reconnect failed: %w", nodeID, reconnErr) - } - node, err = nodes.Get(ctx, c.serviceClient, nodeID).Extract() - if err != nil { - return nil, fmt.Errorf("get node %s after reconnect: %w", nodeID, err) - } - return node, nil - } - return nil, fmt.Errorf("get node %s: %w", nodeID, err) - } - return node, nil -} - -func (c *Client) GetEndpoint() string { - return c.serviceClient.Endpoint -} - -// IsNodePowerTransitioning returns true if the node is currently transitioning -// to a new power state (i.e., TargetPowerState is non-empty). -func (c *Client) IsNodePowerTransitioning(node *nodes.Node) bool { - return node.TargetPowerState != "" -} - -// SetPowerState requests power on or off for the node via Ironic. -func (c *Client) SetPowerState(ctx context.Context, nodeID string, target TargetPowerState) error { - log := ctrllog.FromContext(ctx) - res := nodes.ChangePowerState(ctx, c.serviceClient, nodeID, nodes.PowerStateOpts{Target: target.powerstate}) - if err := res.ExtractErr(); err != nil { - if isAuthError(err) { - log.Info("auth error on SetPowerState, attempting reconnect", "nodeID", nodeID, "target", target.String(), "error", err) - if reconnErr := c.reconnect(ctx); reconnErr != nil { - return fmt.Errorf("failed to set power state on node %s: reconnect failed: %w", nodeID, reconnErr) - } - res = nodes.ChangePowerState(ctx, c.serviceClient, nodeID, nodes.PowerStateOpts{Target: target.powerstate}) - if err := res.ExtractErr(); err != nil { - return fmt.Errorf("failed to set power state on node %s after reconnect: %w", nodeID, err) - } - return nil - } - return fmt.Errorf("failed to set power state on node %s: %w", nodeID, err) - } - return nil -} diff --git a/internal/ironic/client_internal_test.go b/internal/ironic/client_internal_test.go deleted file mode 100644 index cd2b024..0000000 --- a/internal/ironic/client_internal_test.go +++ /dev/null @@ -1,182 +0,0 @@ -/* -Copyright 2026. - -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 ironic - -import ( - "context" - "fmt" - "net/http" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/gophercloud/gophercloud/v2" -) - -const ( - oldEndpoint = "http://old:6385/v1/" - newEndpoint = "http://new:6385/v1/" -) - -var _ = Describe("isAuthError", func() { - It("should return false for nil error", func() { - Expect(isAuthError(nil)).To(BeFalse()) - }) - - It("should return false for generic errors", func() { - Expect(isAuthError(fmt.Errorf("some random error"))).To(BeFalse()) - }) - - It("should return true for 401 ErrUnexpectedResponseCode", func() { - err := gophercloud.ErrUnexpectedResponseCode{ - Actual: http.StatusUnauthorized, - Expected: []int{http.StatusOK}, - } - Expect(isAuthError(err)).To(BeTrue()) - }) - - It("should return false for 404 ErrUnexpectedResponseCode", func() { - err := gophercloud.ErrUnexpectedResponseCode{ - Actual: http.StatusNotFound, - Expected: []int{http.StatusOK}, - } - Expect(isAuthError(err)).To(BeFalse()) - }) - - It("should return false for 500 ErrUnexpectedResponseCode", func() { - err := gophercloud.ErrUnexpectedResponseCode{ - Actual: http.StatusInternalServerError, - Expected: []int{http.StatusOK}, - } - Expect(isAuthError(err)).To(BeFalse()) - }) - - It("should return true for *ErrUnableToReauthenticate (pointer, as gophercloud returns)", func() { - err := &gophercloud.ErrUnableToReauthenticate{ - ErrOriginal: fmt.Errorf("original"), - ErrReauth: fmt.Errorf("reauth failed"), - } - Expect(isAuthError(err)).To(BeTrue()) - }) - - It("should return true for *ErrErrorAfterReauthentication (pointer, as gophercloud returns)", func() { - err := &gophercloud.ErrErrorAfterReauthentication{ - ErrOriginal: fmt.Errorf("still failing"), - } - Expect(isAuthError(err)).To(BeTrue()) - }) - - It("should return true for wrapped *ErrUnableToReauthenticate", func() { - inner := &gophercloud.ErrUnableToReauthenticate{ - ErrOriginal: fmt.Errorf("original"), - ErrReauth: fmt.Errorf("reauth failed"), - } - wrapped := fmt.Errorf("operation failed: %w", inner) - Expect(isAuthError(wrapped)).To(BeTrue()) - }) - - It("should return true for wrapped *ErrErrorAfterReauthentication", func() { - inner := &gophercloud.ErrErrorAfterReauthentication{ - ErrOriginal: fmt.Errorf("still failing"), - } - wrapped := fmt.Errorf("operation failed: %w", inner) - Expect(isAuthError(wrapped)).To(BeTrue()) - }) - - It("should return true for wrapped 401 error", func() { - inner := gophercloud.ErrUnexpectedResponseCode{ - Actual: http.StatusUnauthorized, - Expected: []int{http.StatusOK}, - } - wrapped := fmt.Errorf("operation failed: %w", inner) - Expect(isAuthError(wrapped)).To(BeTrue()) - }) - - It("should return false for wrapped non-auth error", func() { - inner := gophercloud.ErrUnexpectedResponseCode{ - Actual: http.StatusNotFound, - Expected: []int{http.StatusOK}, - } - wrapped := fmt.Errorf("operation failed: %w", inner) - Expect(isAuthError(wrapped)).To(BeFalse()) - }) -}) - -var _ = Describe("reconnect", func() { - It("should swap the service client on success", func() { - oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint} - newSC := &gophercloud.ServiceClient{Endpoint: newEndpoint} - - c := &Client{ - serviceClient: oldSC, - newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) { - return newSC, nil - }, - } - - Expect(c.reconnect(context.Background())).To(Succeed()) - Expect(c.serviceClient).To(Equal(newSC)) - Expect(c.GetEndpoint()).To(Equal(newEndpoint)) - }) - - It("should return error when factory fails", func() { - oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint} - - c := &Client{ - serviceClient: oldSC, - newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) { - return nil, fmt.Errorf("keystone is down") - }, - } - - err := c.reconnect(context.Background()) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("keystone is down")) - Expect(c.serviceClient).To(Equal(oldSC), "should keep old client on failure") - }) - - It("should return error when factory returns nil client without error", func() { - oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint} - - c := &Client{ - serviceClient: oldSC, - newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) { - return nil, nil - }, - } - - err := c.reconnect(context.Background()) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("nil service client")) - Expect(c.serviceClient).To(Equal(oldSC), "should keep old client on nil return") - }) - - It("should return error when new client has empty endpoint", func() { - oldSC := &gophercloud.ServiceClient{Endpoint: oldEndpoint} - - c := &Client{ - serviceClient: oldSC, - newServiceClient: func(context.Context) (*gophercloud.ServiceClient, error) { - return &gophercloud.ServiceClient{Endpoint: ""}, nil - }, - } - - err := c.reconnect(context.Background()) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("no endpoint configured")) - }) -}) diff --git a/internal/ironic/client_test.go b/internal/ironic/client_test.go deleted file mode 100644 index 3a63e0b..0000000 --- a/internal/ironic/client_test.go +++ /dev/null @@ -1,174 +0,0 @@ -/* -Copyright 2026. - -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 ironic_test - -import ( - "context" - "os" - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes" - - "github.com/osac-project/host-management-openstack/internal/ironic" -) - -func TestIronic(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Ironic Suite") -} - -var _ = Describe("TargetPowerState", func() { - Describe("String", func() { - It("should convert PowerOn to string correctly", func() { - Expect(ironic.PowerOn.String()).To(Equal(string(nodes.PowerOn))) - Expect(ironic.PowerOn.String()).To(Equal("power on")) - }) - - It("should convert PowerOff to string correctly", func() { - Expect(ironic.PowerOff.String()).To(Equal(string(nodes.PowerOff))) - Expect(ironic.PowerOff.String()).To(Equal("power off")) - }) - - It("should convert Rebooting to string correctly", func() { - Expect(ironic.Rebooting.String()).To(Equal(string(nodes.Rebooting))) - Expect(ironic.Rebooting.String()).To(Equal("rebooting")) - }) - - It("should convert SoftPowerOff to string correctly", func() { - Expect(ironic.SoftPowerOff.String()).To(Equal(string(nodes.SoftPowerOff))) - Expect(ironic.SoftPowerOff.String()).To(Equal("soft power off")) - }) - - It("should convert SoftRebooting to string correctly", func() { - Expect(ironic.SoftRebooting.String()).To(Equal(string(nodes.SoftRebooting))) - Expect(ironic.SoftRebooting.String()).To(Equal("soft rebooting")) - }) - }) - - Describe("Power state constants", func() { - It("should have distinct values", func() { - states := []string{ - ironic.PowerOn.String(), - ironic.PowerOff.String(), - ironic.Rebooting.String(), - ironic.SoftPowerOff.String(), - ironic.SoftRebooting.String(), - } - - // Verify all states are unique - uniqueStates := make(map[string]bool) - for _, state := range states { - Expect(uniqueStates[state]).To(BeFalse(), "duplicate power state: %s", state) - uniqueStates[state] = true - } - Expect(uniqueStates).To(HaveLen(5)) - }) - }) -}) - -var _ = Describe("Client", func() { - Describe("NewClient", func() { - Context("when OpenStack credentials are not configured", func() { - var originalOSCloud, originalAuthURL string - - BeforeEach(func() { - // Save and clear OpenStack environment variables - originalOSCloud = os.Getenv("OS_CLOUD") - originalAuthURL = os.Getenv("OS_AUTH_URL") - Expect(os.Unsetenv("OS_CLOUD")).To(Succeed()) - Expect(os.Unsetenv("OS_AUTH_URL")).To(Succeed()) - }) - - AfterEach(func() { - // Restore original values - if originalOSCloud != "" { - Expect(os.Setenv("OS_CLOUD", originalOSCloud)).To(Succeed()) - } - if originalAuthURL != "" { - Expect(os.Setenv("OS_AUTH_URL", originalAuthURL)).To(Succeed()) - } - }) - - It("should return an error when no credentials are available", func() { - client, err := ironic.NewClient(context.Background()) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("failed to create baremetal client")) - Expect(client).To(BeNil()) - }) - }) - - Context("when OpenStack credentials are configured", func() { - BeforeEach(func() { - if !hasIronicCredentials() { - Skip("Skipping: OpenStack/Ironic credentials not configured in environment") - } - }) - - It("should create a client successfully", func() { - client, err := ironic.NewClient(context.Background()) - Expect(err).NotTo(HaveOccurred()) - Expect(client).NotTo(BeNil()) - Expect(client.GetEndpoint()).NotTo(BeEmpty()) - }) - - It("should have a non-empty endpoint", func() { - client, err := ironic.NewClient(context.Background()) - Expect(err).NotTo(HaveOccurred()) - - endpoint := client.GetEndpoint() - Expect(endpoint).NotTo(BeEmpty()) - }) - }) - }) - - Describe("GetEndpoint", func() { - BeforeEach(func() { - if !hasIronicCredentials() { - Skip("Skipping: OpenStack/Ironic credentials not configured in environment") - } - }) - - It("should return a valid HTTP(S) endpoint", func() { - client, err := ironic.NewClient(context.Background()) - Expect(err).NotTo(HaveOccurred()) - - endpoint := client.GetEndpoint() - Expect(endpoint).To(Or( - HavePrefix("http://"), - HavePrefix("https://"), - )) - }) - }) -}) - -// hasIronicCredentials checks if OpenStack/Ironic credentials are available -func hasIronicCredentials() bool { - // Check for OS_CLOUD (most common for clouds.yaml) - if os.Getenv("OS_CLOUD") != "" { - return true - } - - // Check for explicit auth URL (required for direct credentials) - if os.Getenv("OS_AUTH_URL") != "" { - return true - } - - return false -} diff --git a/internal/management/client.go b/internal/management/client.go new file mode 100644 index 0000000..7728fd0 --- /dev/null +++ b/internal/management/client.go @@ -0,0 +1,48 @@ +// Package management provides a backend-agnostic interface for bare metal power control. +package management + +import ( + "context" + "errors" + "fmt" +) + +var ErrTransitioning = errors.New("power state transition already in progress") + +type PowerState string + +const ( + PowerOn PowerState = "power on" + PowerOff PowerState = "power off" +) + +type PowerStatus struct { + State PowerState + IsTransitioning bool +} + +type Config struct { + Name string `json:"name"` + Type string `json:"type"` + Options map[string]any `json:"options"` +} + +type Client interface { + GetPowerState(ctx context.Context, hostID string) (*PowerStatus, error) + SetPowerState(ctx context.Context, hostID string, target PowerState) error +} + +type NewClientFunc func(ctx context.Context, cfg *Config) (Client, error) + +var newClientFuncs = make(map[string]NewClientFunc) + +func NewClient(ctx context.Context, cfg *Config) (Client, error) { + if cfg == nil { + return nil, fmt.Errorf("management backend configuration is required") + } + fn, ok := newClientFuncs[cfg.Type] + if !ok { + return nil, fmt.Errorf("unsupported management backend type: %q", cfg.Type) + } + return fn(ctx, cfg) +} diff --git a/internal/management/management_test.go b/internal/management/management_test.go new file mode 100644 index 0000000..edd0f6a --- /dev/null +++ b/internal/management/management_test.go @@ -0,0 +1,133 @@ +package management_test + +import ( + "context" + "os" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/osac-project/host-management-openstack/internal/management" +) + +const testBackendType = "openstack" + +func TestManagement(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Management Suite") +} + +var _ = Describe("PowerState", func() { + It("should have correct PowerOn value", func() { + Expect(string(management.PowerOn)).To(Equal("power on")) + }) + + It("should have correct PowerOff value", func() { + Expect(string(management.PowerOff)).To(Equal("power off")) + }) + + It("should have distinct values", func() { + Expect(management.PowerOn).NotTo(Equal(management.PowerOff)) + }) +}) + +var _ = Describe("PowerStatus", func() { + It("should represent a powered on stable state", func() { + status := &management.PowerStatus{ + State: management.PowerOn, + IsTransitioning: false, + } + Expect(status.State).To(Equal(management.PowerOn)) + Expect(status.IsTransitioning).To(BeFalse()) + }) + + It("should represent a transitioning state", func() { + status := &management.PowerStatus{ + State: management.PowerOff, + IsTransitioning: true, + } + Expect(status.State).To(Equal(management.PowerOff)) + Expect(status.IsTransitioning).To(BeTrue()) + }) +}) + +var _ = Describe("NewClient factory", func() { + It("should return error for unknown type", func() { + client, err := management.NewClient(context.Background(), &management.Config{ + Type: "unknown", + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("unsupported management backend type")) + Expect(client).To(BeNil()) + }) + + Context("when OpenStack credentials are configured", func() { + BeforeEach(func() { + if !hasOpenStackCredentials() { + Skip("Skipping: set MANAGEMENT_TEST_OPENSTACK=1 with valid OpenStack credentials to run") + } + }) + + It("should return a client for openstack type", func() { + client, err := management.NewClient(context.Background(), &management.Config{ + Type: testBackendType, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(client).NotTo(BeNil()) + }) + }) +}) + +var _ = Describe("OpenStackClient", func() { + Context("when OpenStack credentials are not configured", func() { + var originalOSCloud, originalAuthURL string + + BeforeEach(func() { + originalOSCloud = os.Getenv("OS_CLOUD") + originalAuthURL = os.Getenv("OS_AUTH_URL") + Expect(os.Unsetenv("OS_CLOUD")).To(Succeed()) + Expect(os.Unsetenv("OS_AUTH_URL")).To(Succeed()) + }) + + AfterEach(func() { + if originalOSCloud != "" { + Expect(os.Setenv("OS_CLOUD", originalOSCloud)).To(Succeed()) + } + if originalAuthURL != "" { + Expect(os.Setenv("OS_AUTH_URL", originalAuthURL)).To(Succeed()) + } + }) + + It("should return an error when no credentials are available", func() { + client, err := management.NewOpenStackClient(context.Background(), &management.Config{ + Type: testBackendType, + }) + Expect(err).To(HaveOccurred()) + Expect(client).To(BeNil()) + }) + }) + + Context("when OpenStack credentials are configured", func() { + BeforeEach(func() { + if !hasOpenStackCredentials() { + Skip("Skipping: set MANAGEMENT_TEST_OPENSTACK=1 with valid OpenStack credentials to run") + } + }) + + It("should create a client successfully", func() { + client, err := management.NewOpenStackClient(context.Background(), &management.Config{ + Type: testBackendType, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(client).NotTo(BeNil()) + }) + }) +}) + +func hasOpenStackCredentials() bool { + if os.Getenv("MANAGEMENT_TEST_OPENSTACK") == "" { + return false + } + return os.Getenv("OS_CLOUD") != "" || os.Getenv("OS_AUTH_URL") != "" +} diff --git a/internal/management/openstack.go b/internal/management/openstack.go new file mode 100644 index 0000000..7c27e9f --- /dev/null +++ b/internal/management/openstack.go @@ -0,0 +1,102 @@ +package management + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + + "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack" + "github.com/gophercloud/gophercloud/v2/openstack/baremetal/v1/nodes" + "github.com/gophercloud/utils/v2/openstack/clientconfig" +) + +var ( + _ Client = (*OpenStackClient)(nil) + _ NewClientFunc = NewClientFunc(NewOpenStackClient) +) + +func init() { + newClientFuncs["openstack"] = NewOpenStackClient +} + +type OpenStackClient struct { + client *gophercloud.ServiceClient +} + +func NewOpenStackClient(ctx context.Context, cfg *Config) (Client, error) { + var cloud clientconfig.Cloud + if cfg != nil && cfg.Options != nil { + if openstackOpts, ok := cfg.Options["openstack"]; ok { + openstackOptsJSON, err := json.Marshal(openstackOpts) + if err != nil { + return nil, fmt.Errorf("failed to marshal openstack options (check cloud configuration)") + } + if err := json.Unmarshal(openstackOptsJSON, &cloud); err != nil { + return nil, fmt.Errorf("failed to unmarshal openstack options (check cloud configuration)") + } + } + } + + clientOpts := clientconfig.ClientOpts{ + Cloud: cloud.Cloud, + AuthType: cloud.AuthType, + AuthInfo: cloud.AuthInfo, + RegionName: cloud.RegionName, + EndpointType: cloud.EndpointType, + } + + providerClient, err := clientconfig.AuthenticatedClient(ctx, &clientOpts) + if err != nil { + return nil, fmt.Errorf("failed to create authenticated client (check cloud credentials and endpoint configuration)") + } + + ironicClient, err := openstack.NewBareMetalV1(providerClient, gophercloud.EndpointOpts{ + Region: cloud.RegionName, + Availability: gophercloud.Availability(cloud.EndpointType), + }) + if err != nil { + return nil, fmt.Errorf("failed to create baremetal client (check endpoint configuration and region)") + } + + return &OpenStackClient{client: ironicClient}, nil +} + +func (c *OpenStackClient) GetPowerState(ctx context.Context, hostID string) (*PowerStatus, error) { + node, err := nodes.Get(ctx, c.client, hostID).Extract() + if err != nil { + return nil, fmt.Errorf("get node %s: %w", hostID, err) + } + + state := PowerState(node.PowerState) + switch state { + case PowerOn, PowerOff: + default: + return nil, fmt.Errorf("node %s: unexpected power state %q", hostID, node.PowerState) + } + + return &PowerStatus{ + State: state, + IsTransitioning: node.TargetPowerState != "", + }, nil +} + +func (c *OpenStackClient) SetPowerState(ctx context.Context, hostID string, target PowerState) error { + switch target { + case PowerOn, PowerOff: + default: + return fmt.Errorf("node %s: invalid target power state %q", hostID, target) + } + + res := nodes.ChangePowerState(ctx, c.client, hostID, nodes.PowerStateOpts{ + Target: nodes.TargetPowerState(target), + }) + if err := res.ExtractErr(); err != nil { + if gophercloud.ResponseCodeIs(err, http.StatusConflict) { + return fmt.Errorf("node %s: %w", hostID, ErrTransitioning) + } + return fmt.Errorf("failed to set power state on node %s: %w", hostID, err) + } + return nil +}