Repository: metal3-io/baremetal-operator
Pull request #1696 (2024-04-23 17:52:29)

🌱 Update to the Gophercloud beta.4+commits
It keeps changing, now they reworked the HTTP error handling, so
1) switch to the new helper function,
2) stop relying on the exact error message in tests,
3) modernize the touched code a bit.

Signed-off-by: Dmitry Tantsur <dtantsur@protonmail.com>


Patch:
diff --git a/go.mod b/go.mod
index 5ecc3f8a..381fd2f0 100644
--- a/go.mod
+++ b/go.mod
@@ -5,7 +5,7 @@ go 1.21
 require (
 	github.com/go-logr/logr v1.4.1
 	github.com/google/safetext v0.0.0-20230106111101-7156a760e523
-	github.com/gophercloud/gophercloud/v2 v2.0.0-beta.3.0.20240416104816-9aef3836d310
+	github.com/gophercloud/gophercloud/v2 v2.0.0-beta.4.0.20240422125343-8b1eebe1fa46
 	github.com/metal3-io/baremetal-operator/apis v0.5.1
 	github.com/metal3-io/baremetal-operator/pkg/hardwareutils v0.5.1
 	github.com/onsi/gomega v1.32.0
diff --git a/go.sum b/go.sum
index 7ada0926..1f2067ec 100644
--- a/go.sum
+++ b/go.sum
@@ -49,8 +49,8 @@ github.com/google/safetext v0.0.0-20230106111101-7156a760e523 h1:i4NsbmB9pD5+Ggp
 github.com/google/safetext v0.0.0-20230106111101-7156a760e523/go.mod h1:mJNEy0r5YPHC7ChQffpOszlGB4L1iqjXWpIEKcFpr9s=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
-github.com/gophercloud/gophercloud/v2 v2.0.0-beta.3.0.20240416104816-9aef3836d310 h1:7pom2WwZIzl8hujnJriF9f2MLP9EmL8c36JbTBpOUDw=
-github.com/gophercloud/gophercloud/v2 v2.0.0-beta.3.0.20240416104816-9aef3836d310/go.mod h1:HbB8pJ/nycmlTRO++YTIFt9KUDx5c9gdaTFbk4oB5wA=
+github.com/gophercloud/gophercloud/v2 v2.0.0-beta.4.0.20240422125343-8b1eebe1fa46 h1:RCFJPDcv22rnyX38Te7xO4Nw45hshCtdA85AlFLoCZY=
+github.com/gophercloud/gophercloud/v2 v2.0.0-beta.4.0.20240422125343-8b1eebe1fa46/go.mod h1:HbB8pJ/nycmlTRO++YTIFt9KUDx5c9gdaTFbk4oB5wA=
 github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
 github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
 github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8=
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index 152aedd2..e63f8e0e 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -176,18 +176,18 @@ func (p *ironicProvisioner) getNode() (*nodes.Node, error) {
 	}
 
 	ironicNode, err := nodes.Get(p.ctx, p.client, p.nodeID).Extract()
-	switch err.(type) {
-	case nil:
+	if err == nil {
 		p.debugLog.Info("found existing node by ID")
 		return ironicNode, nil
-	case gophercloud.ErrDefault404:
+	}
+
+	if gophercloud.ResponseCodeIs(err, 404) {
 		// Look by ID failed, trying to lookup by hostname in case it was
 		// previously created
 		return nil, provisioner.ErrNeedsRegistration
-	default:
-		return nil, errors.Wrap(err,
-			fmt.Sprintf("failed to find node by ID %s", p.nodeID))
 	}
+
+	return nil, fmt.Errorf("failed to find node by ID %s: %w", p.nodeID, err)
 }
 
 // Verifies that node has port assigned by Ironic.
@@ -251,16 +251,15 @@ func (p *ironicProvisioner) findExistingHost(bootMACAddress string) (ironicNode
 	for _, nodeName := range nodeSearchList {
 		p.debugLog.Info("looking for existing node by name", "name", nodeName)
 		ironicNode, err = nodes.Get(p.ctx, p.client, nodeName).Extract()
-		switch err.(type) {
-		case nil:
+		if err == nil {
 			p.debugLog.Info("found existing node by name", "name", nodeName, "node", ironicNode.UUID)
 			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))
+		}
+
+		if gophercloud.ResponseCodeIs(err, 404) {
+			p.log.Info(fmt.Sprintf("node with name %s doesn't exist", nodeName))
+		} else {
+			return nil, fmt.Errorf("failed to find node by name %s: %w", nodeName, err)
 		}
 	}
 
@@ -276,8 +275,7 @@ func (p *ironicProvisioner) findExistingHost(bootMACAddress string) (ironicNode
 	if len(allPorts) > 0 {
 		nodeUUID := allPorts[0].NodeUUID
 		ironicNode, err = nodes.Get(p.ctx, p.client, nodeUUID).Extract()
-		switch err.(type) {
-		case nil:
+		if err == nil {
 			p.debugLog.Info("found existing node by MAC", "MAC", bootMACAddress, "node", ironicNode.UUID, "name", ironicNode.Name)
 
 			// If the node has a name, this means we didn't find it above.
@@ -286,17 +284,14 @@ func (p *ironicProvisioner) findExistingHost(bootMACAddress string) (ironicNode
 			}
 
 			return ironicNode, nil
-		case gophercloud.ErrDefault404:
-			return nil, errors.Wrap(err,
-				fmt.Sprintf("port %s exists but linked node doesn't %s", bootMACAddress, nodeUUID))
-		default:
-			return nil, errors.Wrap(err,
-				fmt.Sprintf("port %s exists but failed to find linked node by ID %s", bootMACAddress, nodeUUID))
 		}
-	} else {
-		p.log.Info("port with address doesn't exist", "MAC", bootMACAddress)
+		if gophercloud.ResponseCodeIs(err, 404) {
+			return nil, fmt.Errorf("port %s exists but linked node %s doesn't: %w", bootMACAddress, nodeUUID, err)
+		}
+		return nil, fmt.Errorf("port %s exists but failed to find linked node %s by ID: %w", bootMACAddress, nodeUUID, err)
 	}
 
+	p.log.Info("port with address doesn't exist", "MAC", bootMACAddress)
 	// Either the node was never created or the Ironic database has
 	// been dropped.
 	return nil, nil
@@ -415,14 +410,13 @@ func (p *ironicProvisioner) ValidateManagementAccess(data provisioner.Management
 		}
 
 		ironicNode, err = nodes.Create(p.ctx, p.client, nodeCreateOpts).Extract()
-		switch err.(type) {
-		case nil:
+		if err == nil {
 			p.publisher("Registered", "Registered new host")
-		case gophercloud.ErrDefault409:
+		} else if gophercloud.ResponseCodeIs(err, 409) {
 			p.log.Info("could not register host in ironic, busy")
 			result, err = retryAfterDelay(provisionRequeueDelay)
 			return
-		default:
+		} else {
 			result, err = transientError(errors.Wrap(err, "failed to register host in ironic"))
 			return
 		}
@@ -764,14 +758,13 @@ func (p *ironicProvisioner) tryUpdateNode(ironicNode *nodes.Node, updater *nodeU
 
 	p.log.Info("updating node settings in ironic", "updateCount", len(updater.Updates))
 	updatedNode, err = nodes.Update(p.ctx, p.client, ironicNode.UUID, updater.Updates).Extract()
-	switch err.(type) {
-	case nil:
+	if err == nil {
 		success = true
-	case gophercloud.ErrDefault409:
+	} else if gophercloud.ResponseCodeIs(err, 409) {
 		p.log.Info("could not update node settings in ironic, busy or update cannot be applied in the current state")
 		result, err = retryAfterDelay(provisionRequeueDelay)
-	default:
-		result, err = transientError(errors.Wrap(err, "failed to update host settings in ironic"))
+	} else {
+		result, err = transientError(fmt.Errorf("failed to update host settings in ironic: %w", err))
 	}
 
 	return
@@ -797,16 +790,14 @@ func (p *ironicProvisioner) tryChangeNodeProvisionState(ironicNode *nodes.Node,
 	}
 
 	changeResult := nodes.ChangeProvisionState(p.ctx, p.client, ironicNode.UUID, opts)
-	switch changeResult.Err.(type) {
-	case nil:
+	if changeResult.Err == nil {
 		success = true
-	case gophercloud.ErrDefault409:
+	} else if gophercloud.ResponseCodeIs(changeResult.Err, 409) {
 		p.log.Info("could not change state of host, busy")
 		result, err = retryAfterDelay(provisionRequeueDelay)
 		return
-	default:
-		result, err = transientError(errors.Wrap(changeResult.Err,
-			fmt.Sprintf("failed to change provisioning state to %q", opts.Target)))
+	} else {
+		result, err = transientError(fmt.Errorf("failed to change provisioning state to %q: %w", opts.Target, changeResult.Err))
 		return
 	}
 
@@ -916,12 +907,12 @@ func (p *ironicProvisioner) InspectHardware(data provisioner.InspectData, restar
 	response := nodes.GetInventory(p.ctx, p.client, ironicNode.UUID)
 	introData, err := response.Extract()
 	if err != nil {
-		if _, isNotFound := err.(gophercloud.ErrDefault404); isNotFound {
+		if gophercloud.ResponseCodeIs(err, 404) {
 			// The node has just been enrolled, inspection hasn't been started yet.
 			result, started, err = p.startInspection(data, ironicNode)
 			return
 		}
-		result, err = transientError(errors.Wrap(err, "failed to retrieve hardware introspection data"))
+		result, err = transientError(fmt.Errorf("failed to retrieve hardware introspection data: %w", err))
 		return
 	}
 
@@ -1242,13 +1233,11 @@ func (p *ironicProvisioner) setUpForProvisioning(ironicNode *nodes.Node, data pr
 	p.log.Info("validating host settings")
 
 	errorMessage, err := p.validateNode(ironicNode)
-	switch err.(type) {
-	case nil:
-	case gophercloud.ErrDefault409:
+	if gophercloud.ResponseCodeIs(err, 409) {
 		p.log.Info("could not validate host during registration, busy")
 		return retryAfterDelay(provisionRequeueDelay)
-	default:
-		return transientError(errors.Wrap(err, "failed to validate host during registration"))
+	} else if err != nil {
+		return transientError(fmt.Errorf("failed to validate host during registration: %w", err))
 	}
 	if errorMessage != "" {
 		return operationFailed(errorMessage)
@@ -1779,13 +1768,12 @@ func (p *ironicProvisioner) setMaintenanceFlag(ironicNode *nodes.Node, value boo
 		err = nodes.UnsetMaintenance(p.ctx, p.client, ironicNode.UUID).ExtractErr()
 	}
 
-	switch err.(type) {
-	case nil:
+	if err == nil {
 		result, err = operationContinuing(0)
-	case gophercloud.ErrDefault409:
+	} else if gophercloud.ResponseCodeIs(err, 409) {
 		p.log.Info("could not update maintenance in ironic, busy")
 		result, err = retryAfterDelay(provisionRequeueDelay)
-	default:
+	} else {
 		err = fmt.Errorf("failed to set host maintenance flag to %v (%w)", value, err)
 		result, err = transientError(err)
 	}
@@ -1944,16 +1932,15 @@ func (p *ironicProvisioner) Delete() (result provisioner.Result, err error) {
 
 	p.log.Info("host ready to be removed")
 	err = nodes.Delete(p.ctx, p.client, ironicNode.UUID).ExtractErr()
-	switch err.(type) {
-	case nil:
+	if err == nil {
 		p.log.Info("removed")
-	case gophercloud.ErrDefault409:
+	} else if gophercloud.ResponseCodeIs(err, 409) {
 		p.log.Info("could not remove host, busy")
 		return retryAfterDelay(provisionRequeueDelay)
-	case gophercloud.ErrDefault404:
+	} else if gophercloud.ResponseCodeIs(err, 404) {
 		p.log.Info("did not find host to delete, OK")
-	default:
-		return transientError(errors.Wrap(err, "failed to remove host"))
+	} else {
+		return transientError(fmt.Errorf("failed to remove host: %w", err))
 	}
 
 	return operationContinuing(0)
@@ -2007,8 +1994,7 @@ func (p *ironicProvisioner) changePower(ironicNode *nodes.Node, target nodes.Tar
 		ironicNode.UUID,
 		powerStateOpts)
 
-	switch changeResult.Err.(type) {
-	case nil:
+	if changeResult.Err == nil {
 		p.log.Info("power change OK")
 		event := map[nodes.TargetPowerState]struct{ Event, Reason string }{
 			nodes.PowerOn:      {Event: "PowerOn", Reason: "Host powered on"},
@@ -2017,18 +2003,17 @@ func (p *ironicProvisioner) changePower(ironicNode *nodes.Node, target nodes.Tar
 		}[target]
 		p.publisher(event.Event, event.Reason)
 		return operationContinuing(0)
-	case gophercloud.ErrDefault409:
+	} else if gophercloud.ResponseCodeIs(changeResult.Err, 409) {
 		p.log.Info("host is locked, trying again after delay", "delay", powerRequeueDelay)
 		return retryAfterDelay(powerRequeueDelay)
-	case gophercloud.ErrDefault400:
+	} else if gophercloud.ResponseCodeIs(changeResult.Err, 400) {
 		// Error 400 Bad Request means target power state is not supported by vendor driver
 		if target == nodes.SoftPowerOff {
 			changeResult.Err = softPowerOffUnsupportedError{changeResult.Err}
 		}
 	}
 	p.log.Info("power change error", "message", changeResult.Err)
-	return transientError(errors.Wrap(changeResult.Err,
-		fmt.Sprintf("failed to %s node", target)))
+	return transientError(fmt.Errorf("failed to %s node: %w", target, changeResult.Err))
 }
 
 // PowerOn ensures the server is powered on independently of any image


Test patch:
diff --git a/pkg/provisioner/ironic/delete_test.go b/pkg/provisioner/ironic/delete_test.go
index 76d14e06..fa2e52be 100644
--- a/pkg/provisioner/ironic/delete_test.go
+++ b/pkg/provisioner/ironic/delete_test.go
@@ -92,7 +92,7 @@ func deleteTest(t *testing.T, detach bool) {
 			hostName: "worker-0",
 			ironic:   testserver.NewIronic(t).NodeError(nodeUUID, http.StatusGatewayTimeout),
 
-			expectedError: "failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58: Gateway Timeout.*",
+			expectedError: "failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58: .*",
 		},
 		{
 			name:   "not-ironic-node",
diff --git a/pkg/provisioner/ironic/inspecthardware_test.go b/pkg/provisioner/ironic/inspecthardware_test.go
index 32d4bd0a..9feb7e49 100644
--- a/pkg/provisioner/ironic/inspecthardware_test.go
+++ b/pkg/provisioner/ironic/inspecthardware_test.go
@@ -84,7 +84,7 @@ func TestInspectHardware(t *testing.T) {
 				ProvisionState: "manageable",
 			}).WithInventoryFailed(nodeUUID, http.StatusBadRequest),
 
-			expectedError: "failed to retrieve hardware introspection data: Bad request with: \\[GET http://127.0.0.1:.*/v1/nodes/33ce8659-7400-4c68-9535-d10766f07a58/inventory\\], error message: An error\\\n",
+			expectedError: "failed to retrieve hardware introspection data: .*: An error",
 		},
 		{
 			name: "introspection-status-retry-on-wait",
diff --git a/pkg/provisioner/ironic/power_test.go b/pkg/provisioner/ironic/power_test.go
index 518e35cf..60ee1802 100644
--- a/pkg/provisioner/ironic/power_test.go
+++ b/pkg/provisioner/ironic/power_test.go
@@ -322,5 +322,5 @@ func TestSoftPowerOffFallback(t *testing.T) {
 
 	_, err = prov.changePower(&node, nodes.SoftPowerOff)
 	assert.Error(t, err)
-	assert.True(t, errors.As(err, &softPowerOffUnsupportedError{}))
+	assert.ErrorAs(t, err, &softPowerOffUnsupportedError{})
 }
diff --git a/pkg/provisioner/ironic/updatehardwarestate_test.go b/pkg/provisioner/ironic/updatehardwarestate_test.go
index b3c0e03d..a077ea70 100644
--- a/pkg/provisioner/ironic/updatehardwarestate_test.go
+++ b/pkg/provisioner/ironic/updatehardwarestate_test.go
@@ -80,7 +80,7 @@ func TestUpdateHardwareState(t *testing.T) {
 			hostName: "worker-0",
 			ironic:   testserver.NewIronic(t).NodeError(nodeUUID, http.StatusGatewayTimeout),
 
-			expectedError: "failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58: Gateway Timeout.*",
+			expectedError: "failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58:.*",
 
 			expectUnreadablePower: true,
 		},
diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/validatemanagementaccess_test.go
index 4121808d..f2b36bb0 100644
--- a/pkg/provisioner/ironic/validatemanagementaccess_test.go
+++ b/pkg/provisioner/ironic/validatemanagementaccess_test.go
@@ -1,7 +1,6 @@
 package ironic
 
 import (
-	"fmt"
 	"net/http"
 	"testing"
 
@@ -737,9 +736,8 @@ func TestValidateManagementAccessExistingPortWithWrongUUID(t *testing.T) {
 	}
 
 	_, _, err = prov.ValidateManagementAccess(provisioner.ManagementAccessData{}, false, false)
-	endpoint := ironic.MockServer.Endpoint()
-	expected := fmt.Sprintf("failed to find existing host: port 11:11:11:11:11:11 exists but linked node doesn't random-wrong-id: Resource not found: [GET %snodes/random-wrong-id], error message: ", endpoint)
-	assert.EqualError(t, err, expected)
+	expected := "failed to find existing host: port 11:11:11:11:11:11 exists but linked node random-wrong-id doesn't:"
+	assert.ErrorContains(t, err, expected)
 }
 
 func TestValidateManagementAccessExistingPortButHasName(t *testing.T) {