diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 8cc788c93c..b7f9601f19 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -27,6 +27,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace envFrom: - configMapRef: name: ironic @@ -37,4 +41,4 @@ spec: port: 9440 initialDelaySeconds: 3 periodSeconds: 3 - terminationGracePeriodSeconds: 10 \ No newline at end of file + terminationGracePeriodSeconds: 10 diff --git a/config/render/capm3.yaml b/config/render/capm3.yaml index 95779965f4..c36b761faf 100644 --- a/config/render/capm3.yaml +++ b/config/render/capm3.yaml @@ -1039,6 +1039,10 @@ spec: valueFrom: fieldRef: fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace envFrom: - configMapRef: name: baremetal-operator-ironic diff --git a/main.go b/main.go index 6958e5f0a0..5fea6ea657 100644 --- a/main.go +++ b/main.go @@ -103,13 +103,18 @@ func main() { printVersion() + leaderElectionNamespace := os.Getenv("POD_NAMESPACE") + if leaderElectionNamespace == "" { + leaderElectionNamespace = watchNamespace + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, Port: 0, // Add flag with default of 9443 when adding webhooks LeaderElection: enableLeaderElection, LeaderElectionID: "baremetal-operator", - LeaderElectionNamespace: watchNamespace, + LeaderElectionNamespace: leaderElectionNamespace, Namespace: watchNamespace, HealthProbeBindAddress: healthAddr, }) diff --git a/pkg/provisioner/ironic/delete_test.go b/pkg/provisioner/ironic/delete_test.go index 00a1eb9a9f..6230964f44 100644 --- a/pkg/provisioner/ironic/delete_test.go +++ b/pkg/provisioner/ironic/delete_test.go @@ -86,7 +86,7 @@ func TestDelete(t *testing.T) { }, { name: "not-ironic-node", - ironic: testserver.NewIronic(t).Ready().NoNode(nodeUUID).NoNode("myhost"), + ironic: testserver.NewIronic(t).Ready().NoNode(nodeUUID).NoNode("myns" + nameSeparator + "myhost").NoNode("myhost"), }, { name: "available-node", diff --git a/pkg/provisioner/ironic/findhost_test.go b/pkg/provisioner/ironic/findhost_test.go index 79f95d074d..e8b21af67e 100644 --- a/pkg/provisioner/ironic/findhost_test.go +++ b/pkg/provisioner/ironic/findhost_test.go @@ -23,7 +23,7 @@ func TestFindExistingHost(t *testing.T) { name: "no-node", hostName: "name", provisioningID: "uuid", - ironic: testserver.NewIronic(t).NoNode("name").NoNode("uuid"), + ironic: testserver.NewIronic(t).NoNode("myns" + nameSeparator + "name").NoNode("name").NoNode("uuid"), }, { name: "by-name", @@ -31,21 +31,21 @@ func TestFindExistingHost(t *testing.T) { provisioningID: "uuid", ironic: testserver.NewIronic(t).NoNode("uuid"). Node(nodes.Node{ - Name: "name", + Name: "myns" + nameSeparator + "name", UUID: "different-uuid", }), - nodeName: "name", + nodeName: "myns" + nameSeparator + "name", }, { name: "by-uuid", hostName: "name", provisioningID: "uuid", - ironic: testserver.NewIronic(t).NoNode("name"). + ironic: testserver.NewIronic(t).NoNode("myns" + nameSeparator + "name").NoNode("name"). Node(nodes.Node{ - Name: "different-name", + Name: "myns" + nameSeparator + "different-name", UUID: "uuid", }), - nodeName: "different-name", + nodeName: "myns" + nameSeparator + "different-name", }, } diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go index f45506e2b5..0570fb18f0 100644 --- a/pkg/provisioner/ironic/ironic.go +++ b/pkg/provisioner/ironic/ironic.go @@ -51,10 +51,11 @@ var ( const ( // See nodes.Node.PowerState for details - powerOn = "power on" - powerOff = "power off" - softPowerOff = "soft power off" - powerNone = "None" + powerOn = "power on" + powerOff = "power off" + softPowerOff = "soft power off" + powerNone = "None" + nameSeparator = "~" ) var bootModeCapabilities = map[metal3v1alpha1.BootMode]string{ @@ -183,8 +184,6 @@ func newProvisionerWithSettings(host metal3v1alpha1.BareMetalHost, bmcCreds bmc. } func newProvisionerWithIronicClients(host metal3v1alpha1.BareMetalHost, bmcCreds bmc.Credentials, publisher provisioner.EventPublisher, clientIronic *gophercloud.ServiceClient, clientInspector *gophercloud.ServiceClient) (*ironicProvisioner, error) { - provisionerLogger := log.WithValues("host", host.Name) - // Ensure we have a microversion high enough to get the features // we need. clientIronic.Microversion = "1.56" @@ -194,10 +193,10 @@ func newProvisionerWithIronicClients(host metal3v1alpha1.BareMetalHost, bmcCreds bmcCreds: bmcCreds, client: clientIronic, inspector: clientInspector, - log: provisionerLogger, - debugLog: provisionerLogger.V(1), publisher: publisher, } + p.log = log.WithValues("host", p.ironicNodeNameFromHost()) + p.debugLog = p.log.V(1) return p, nil } @@ -308,18 +307,25 @@ func (p *ironicProvisioner) findExistingHost() (ironicNode *nodes.Node, err erro } // Try to load the node by name - p.log.Info("looking for existing node by name", "name", p.host.Name) - ironicNode, err = nodes.Get(p.client, p.host.Name).Extract() - switch err.(type) { - case nil: - p.debugLog.Info("found existing node by name") - return ironicNode, nil - case gophercloud.ErrDefault404: - p.log.Info( - fmt.Sprintf("node with name %s doesn't exist", p.host.Name)) - default: - return nil, errors.Wrap(err, - fmt.Sprintf("failed to find node by name %s", p.host.Name)) + nodeSearchList := []string{p.ironicNodeNameFromHost()} + if !strings.Contains(p.host.Name, nameSeparator) { + nodeSearchList = append(nodeSearchList, p.host.Name) + } + + for _, nodeName := range nodeSearchList { + p.debugLog.Info("looking for existing node by name", "name", nodeName) + ironicNode, err = nodes.Get(p.client, nodeName).Extract() + switch err.(type) { + case nil: + p.debugLog.Info("found existing node by name") + return ironicNode, nil + case gophercloud.ErrDefault404: + p.log.Info( + fmt.Sprintf("node with name %s doesn't exist", nodeName)) + default: + return nil, errors.Wrap(err, + fmt.Sprintf("failed to find node by name %s", nodeName)) + } } // Try to load the node by port address @@ -501,12 +507,12 @@ func (p *ironicProvisioner) ValidateManagementAccess(credentialsChanged, force b // if there are differences. provID = ironicNode.UUID - if ironicNode.Name == "" { + if ironicNode.Name != p.ironicNodeNameFromHost() { updates := nodes.UpdateOpts{ nodes.UpdateOperation{ Op: nodes.ReplaceOp, Path: "/name", - Value: p.host.Name, + Value: p.ironicNodeNameFromHost(), }, } ironicNode, err = nodes.Update(p.client, ironicNode.UUID, updates).Extract() @@ -1840,6 +1846,10 @@ func (p *ironicProvisioner) softPowerOff() (result provisioner.Result, err error return result, nil } +func (p *ironicProvisioner) ironicNodeNameFromHost() string { + return p.host.Namespace + nameSeparator + p.host.Name +} + // IsReady checks if the provisioning backend is available func (p *ironicProvisioner) IsReady() (result bool, err error) { p.debugLog.Info("verifying ironic provisioner dependencies") @@ -1857,7 +1867,7 @@ func (p *ironicProvisioner) HasProvisioningCapacity() (result bool, err error) { } // If the current host is already under processing then let's skip the test - if _, ok := hosts[p.host.Name]; ok { + if _, ok := hosts[p.ironicNodeNameFromHost()]; ok { return true, nil } diff --git a/pkg/provisioner/ironic/provisioncapacity_test.go b/pkg/provisioner/ironic/provisioncapacity_test.go index f37f0c2737..1b54c124ae 100644 --- a/pkg/provisioner/ironic/provisioncapacity_test.go +++ b/pkg/provisioner/ironic/provisioncapacity_test.go @@ -62,7 +62,7 @@ func TestHasProvisioningCapacity(t *testing.T) { allNodes := []nodes.Node{} for n, state := range tc.nodeStates { allNodes = append(allNodes, nodes.Node{ - Name: fmt.Sprintf("node-%d", n), + Name: fmt.Sprintf("myns%snode-%d", nameSeparator, n), ProvisionState: string(state), }) } diff --git a/pkg/provisioner/ironic/updatehardwarestate_test.go b/pkg/provisioner/ironic/updatehardwarestate_test.go index e4e89b0b80..691a2f0a2e 100644 --- a/pkg/provisioner/ironic/updatehardwarestate_test.go +++ b/pkg/provisioner/ironic/updatehardwarestate_test.go @@ -90,15 +90,15 @@ func TestUpdateHardwareState(t *testing.T) { name: "node-not-found-by-name", hostName: "worker-0", - ironic: testserver.NewIronic(t).Ready().NoNode(nodeUUID).NodeError("myhost", http.StatusGatewayTimeout), + ironic: testserver.NewIronic(t).Ready().NoNode(nodeUUID).NodeError("myns.myhost", http.StatusGatewayTimeout), - expectedError: "failed to find existing host: failed to find node by name worker-0: EOF", + expectedError: "failed to find existing host: failed to find node by name myns.worker-0: EOF", expectUnreadablePower: true, }, { name: "not-ironic-node", - ironic: testserver.NewIronic(t).Ready().NoNode(nodeUUID).NoNode("myhost"), + ironic: testserver.NewIronic(t).Ready().NoNode(nodeUUID).NoNode("myns" + nameSeparator + "myhost").NoNode("myhost"), expectedError: "Host not registered", diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/validatemanagementaccess_test.go index 7369fba723..3f8b13aec9 100644 --- a/pkg/provisioner/ironic/validatemanagementaccess_test.go +++ b/pkg/provisioner/ironic/validatemanagementaccess_test.go @@ -22,7 +22,7 @@ func TestValidateManagementAccessNoMAC(t *testing.T) { host.Spec.BootMACAddress = "" host.Status.Provisioning.ID = "" // so we don't lookup by uuid - ironic := testserver.NewIronic(t).Ready().NoNode(host.Name) + ironic := testserver.NewIronic(t).Ready().NoNode(host.Namespace + nameSeparator + host.Name).NoNode(host.Name) ironic.Start() defer ironic.Stop() @@ -50,7 +50,7 @@ func TestValidateManagementAccessMACOptional(t *testing.T) { // Set up ironic server to return the node ironic := testserver.NewIronic(t).Ready(). Node(nodes.Node{ - Name: host.Name, + Name: host.Namespace + nameSeparator + host.Name, UUID: host.Status.Provisioning.ID, }) ironic.Start() @@ -85,7 +85,7 @@ func TestValidateManagementAccessCreateNodeNoImage(t *testing.T) { createdNode = &node } - ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Name) + ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Namespace + nameSeparator + host.Name).NoNode(host.Name) ironic.Start() defer ironic.Stop() @@ -120,7 +120,7 @@ func TestValidateManagementAccessCreateWithImage(t *testing.T) { createdNode = &node } - ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Name) + ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Namespace + nameSeparator + host.Name).NoNode(host.Name) ironic.AddDefaultResponse("/v1/nodes/node-0", "PATCH", http.StatusOK, "{}") ironic.Start() defer ironic.Stop() @@ -158,7 +158,7 @@ func TestValidateManagementAccessCreateWithLiveIso(t *testing.T) { createdNode = &node } - ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Name) + ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Namespace + nameSeparator + host.Name).NoNode(host.Name) ironic.AddDefaultResponse("/v1/nodes/node-0", "PATCH", http.StatusOK, "{}") ironic.Start() defer ironic.Stop() @@ -236,7 +236,7 @@ func TestValidateManagementAccessCreateNodeImageSpecOrStatus(t *testing.T) { createdNode = &node } - ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Name) + ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).NoNode(host.Namespace + nameSeparator + host.Name).NoNode(host.Name) ironic.NodeUpdate(nodes.Node{UUID: "node-0"}) ironic.Start() defer ironic.Stop() @@ -278,7 +278,7 @@ func TestValidateManagementAccessExistingNode(t *testing.T) { } ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).Node(nodes.Node{ - Name: host.Name, + Name: host.Namespace + nameSeparator + host.Name, UUID: "uuid", }) ironic.Start() @@ -300,6 +300,43 @@ func TestValidateManagementAccessExistingNode(t *testing.T) { assert.Equal(t, "uuid", provID) } +func TestValidateManagementAccessExistingNodeNameUpdate(t *testing.T) { + // Create a host without a bootMACAddress and with a BMC that + // does not require one. + host := makeHost() + host.Spec.BootMACAddress = "" + host.Status.Provisioning.ID = "uuid" + + ironic := testserver.NewIronic(t). + Node( + nodes.Node{ + Name: host.Name, + UUID: "uuid", + }). + NodeUpdate( + nodes.Node{ + Name: host.Namespace + nameSeparator + host.Name, + UUID: "uuid", + }) + ironic.Start() + defer ironic.Stop() + + auth := clients.AuthConfig{Type: clients.NoAuth} + prov, err := newProvisionerWithSettings(host, bmc.Credentials{}, nullEventPublisher, + ironic.Endpoint(), auth, testserver.NewInspector(t).Endpoint(), auth, + ) + if err != nil { + t.Fatalf("could not create provisioner: %s", err) + } + + result, _, err := prov.ValidateManagementAccess(false, false) + if err != nil { + t.Fatalf("error from ValidateManagementAccess: %s", err) + } + assert.Equal(t, "", result.ErrorMessage) + assert.Equal(t, false, result.Dirty) +} + func TestValidateManagementAccessExistingNodeContinue(t *testing.T) { statuses := []nodes.ProvisionState{ nodes.Manageable, @@ -340,7 +377,7 @@ func TestValidateManagementAccessExistingNodeContinue(t *testing.T) { } ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).Node(nodes.Node{ - Name: host.Name, + Name: host.Namespace + nameSeparator + host.Name, UUID: "", // to match status in host ProvisionState: string(status), }) @@ -384,7 +421,7 @@ func TestValidateManagementAccessExistingNodeWaiting(t *testing.T) { } node := nodes.Node{ - Name: host.Name, + Name: host.Namespace + nameSeparator + host.Name, UUID: "uuid", // to match status in host ProvisionState: string(status), } @@ -420,12 +457,12 @@ func TestValidateManagementAccessNewCredentials(t *testing.T) { ironic := testserver.NewIronic(t). Node( nodes.Node{ - Name: host.Name, + Name: host.Namespace + nameSeparator + host.Name, UUID: "uuid", }). NodeUpdate( nodes.Node{ - Name: host.Name, + Name: host.Namespace + nameSeparator + host.Name, UUID: "uuid", DriverInfo: map[string]interface{}{ "test_address": "test.bmc", @@ -472,6 +509,7 @@ func TestValidateManagementAccessLinkExistingIronicNodeByMAC(t *testing.T) { } ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).Node(existingNode).Port(existingNodePort) + ironic.AddDefaultResponse("/v1/nodes/myns"+nameSeparator+"myhost", "GET", http.StatusNotFound, "") ironic.AddDefaultResponse("/v1/nodes/myhost", "GET", http.StatusNotFound, "") ironic.AddDefaultResponse("/v1/nodes/"+existingNode.UUID, "PATCH", http.StatusOK, "{}") ironic.Start() @@ -515,6 +553,7 @@ func TestValidateManagementAccessExistingPortWithWrongUUID(t *testing.T) { } ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).Node(existingNode).Port(existingNodePort) + ironic.AddDefaultResponse("/v1/nodes/myns"+nameSeparator+"myhost", "GET", http.StatusNotFound, "") ironic.AddDefaultResponse("/v1/nodes/myhost", "GET", http.StatusNotFound, "") ironic.AddDefaultResponse("/v1/nodes/random-wrong-id", "GET", http.StatusNotFound, "") ironic.Start() @@ -558,6 +597,7 @@ func TestValidateManagementAccessExistingPortButHasName(t *testing.T) { } ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).Node(existingNode).Port(existingNodePort) + ironic.AddDefaultResponse("/v1/nodes/myns"+nameSeparator+"myhost", "GET", http.StatusNotFound, "") ironic.AddDefaultResponse("/v1/nodes/myhost", "GET", http.StatusNotFound, "") ironic.Start() defer ironic.Stop() @@ -583,7 +623,7 @@ func TestValidateManagementAccessAddTwoHostsWithSameMAC(t *testing.T) { existingNode := nodes.Node{ UUID: "33ce8659-7400-4c68-9535-d10766f07a58", - Name: "myhost", + Name: "myns" + nameSeparator + "myhost", } existingNodePort := ports.Port{ @@ -628,7 +668,7 @@ func TestValidateManagementAccessUnsupportedSecureBoot(t *testing.T) { host.Spec.BootMode = metal3v1alpha1.UEFISecureBoot host.Status.Provisioning.ID = "" // so we don't lookup by uuid - ironic := testserver.NewIronic(t).Ready().NoNode(host.Name) + ironic := testserver.NewIronic(t).Ready().NoNode("myns" + nameSeparator + host.Name).NoNode(host.Name) ironic.Start() defer ironic.Stop()