Repository: metal3-io/baremetal-operator
Pull request #1212 (2023-02-03 13:22:33)

🌱  set maintenance reason when turning maintenance on
Redo of https://github.com/metal3-io/baremetal-operator/pull/1088



Patch:
diff --git a/go.mod b/go.mod
index 1f729188..f3b53942 100644
--- a/go.mod
+++ b/go.mod
@@ -4,7 +4,7 @@ go 1.19
 
 require (
 	github.com/go-logr/logr v1.2.3
-	github.com/gophercloud/gophercloud v0.22.0
+	github.com/gophercloud/gophercloud v1.2.0
 	github.com/metal3-io/baremetal-operator/apis v0.1.2
 	github.com/metal3-io/baremetal-operator/pkg/hardwareutils v0.1.2
 	github.com/pkg/errors v0.9.1
diff --git a/go.sum b/go.sum
index 681c64fc..46bae2ae 100644
--- a/go.sum
+++ b/go.sum
@@ -211,8 +211,8 @@ github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+
 github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
 github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
 github.com/googleapis/gax-go/v2 v2.1.0/go.mod h1:Q3nei7sK6ybPYH7twZdmQpAd1MKb7pfu6SK+H1/DsU0=
-github.com/gophercloud/gophercloud v0.22.0 h1:9lFISNLafZcecT0xUveIMt3IafexC6DIV9ek1SZdSMw=
-github.com/gophercloud/gophercloud v0.22.0/go.mod h1:wRtmUelyIIv3CSSDI47aUwbs075O6i+LY+pXsKCBsb4=
+github.com/gophercloud/gophercloud v1.2.0 h1:1oXyj4g54KBg/kFtCdMM6jtxSzeIyg8wv4z1HoGPp1E=
+github.com/gophercloud/gophercloud v1.2.0/go.mod h1:aAVqcocTSXh2vYFZ1JTvx4EQmfgzxRcNupUfxZbBNDM=
 github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
@@ -351,7 +351,7 @@ golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8U
 golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
-golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
+golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190510132918-efd6b22b2522/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
@@ -426,6 +426,7 @@ golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLd
 golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
 golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
+golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
 golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
 golang.org/x/net v0.4.0 h1:Q5QPcMlvfxFTAPV0+07Xz/MpK9NTXu2VDUuy0FeMfaU=
@@ -471,7 +472,6 @@ golang.org/x/sys v0.0.0-20190606165138-5da285871e9c/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20190726091711-fc99dfbffb4e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191001151750-bb3f8db39f24/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191228213918-04cbcbbfeed8/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200106162015-b016eb3dc98e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
@@ -519,7 +519,6 @@ golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ=
 golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.3.0 h1:qoo4akIqOcDME5bhc/NgxUdovd6BSS2uMsVjB56q1xI=
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index 9b760a95..cd894d85 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -723,7 +723,7 @@ func (p *ironicProvisioner) tryChangeNodeProvisionState(ironicNode *nodes.Node,
 	}
 	if ironicNode.Maintenance {
 		p.log.Info("trying to change a provision state for a node in maintenance, removing maintenance first", "reason", ironicNode.MaintenanceReason)
-		result, err = p.setMaintenanceFlag(ironicNode, false)
+		result, err = p.setMaintenanceFlag(ironicNode, false, "")
 		return
 	}
 
@@ -1197,7 +1197,7 @@ func (p *ironicProvisioner) Adopt(data provisioner.AdoptData, force bool) (resul
 		// Empty Fault means that maintenance was set manually, not by Ironic
 		if ironicNode.Maintenance && ironicNode.Fault == "" && data.State != metal3v1alpha1.StateDeleting {
 			p.log.Info("active node was found to be in maintenance, updating", "state", data.State)
-			return p.setMaintenanceFlag(ironicNode, false)
+			return p.setMaintenanceFlag(ironicNode, false, "")
 		}
 	default:
 	}
@@ -1408,7 +1408,7 @@ func (p *ironicProvisioner) Prepare(data provisioner.PrepareData, unprepared boo
 		}
 		if ironicNode.Maintenance {
 			p.log.Info("clearing maintenance flag")
-			result, err = p.setMaintenanceFlag(ironicNode, false)
+			result, err = p.setMaintenanceFlag(ironicNode, false, "")
 			return
 		}
 		result, err = p.changeNodeProvisionState(
@@ -1554,7 +1554,7 @@ func (p *ironicProvisioner) Provision(data provisioner.ProvisionData) (result pr
 	case nodes.CleanFail:
 		if ironicNode.Maintenance {
 			p.log.Info("clearing maintenance flag")
-			return p.setMaintenanceFlag(ironicNode, false)
+			return p.setMaintenanceFlag(ironicNode, false, "")
 		}
 		return p.changeNodeProvisionState(
 			ironicNode,
@@ -1600,16 +1600,25 @@ func (p *ironicProvisioner) Provision(data provisioner.ProvisionData) (result pr
 	}
 }
 
-func (p *ironicProvisioner) setMaintenanceFlag(ironicNode *nodes.Node, value bool) (result provisioner.Result, err error) {
-	_, success, result, err := p.tryUpdateNode(ironicNode,
-		updateOptsBuilder(p.log).SetTopLevelOpt("maintenance", value, nil))
-	if err != nil {
-		err = fmt.Errorf("failed to set host maintenance flag to %v (%w)", value, err)
+func (p *ironicProvisioner) setMaintenanceFlag(ironicNode *nodes.Node, value bool, reason string) (result provisioner.Result, err error) {
+	p.log.Info("updating maintenance in ironic", "newValue", value, "reason", reason)
+	if value {
+		err = nodes.SetMaintenance(p.client, ironicNode.UUID, nodes.MaintenanceOpts{Reason: reason}).ExtractErr()
+	} else {
+		err = nodes.UnsetMaintenance(p.client, ironicNode.UUID).ExtractErr()
 	}
-	if !success {
-		return
+
+	switch err.(type) {
+	case nil:
+		result, err = operationContinuing(0)
+	case gophercloud.ErrDefault409:
+		p.log.Info("could not update maintenance in ironic, busy")
+		result, err = retryAfterDelay(provisionRequeueDelay)
+	default:
+		err = fmt.Errorf("failed to set host maintenance flag to %v (%w)", value, err)
+		result, err = transientError(err)
 	}
-	return operationContinuing(0)
+	return
 }
 
 // Deprovision removes the host from the image. It may be called
@@ -1654,7 +1663,7 @@ func (p *ironicProvisioner) Deprovision(force bool) (result provisioner.Result,
 		p.log.Info("cleaning failed")
 		if ironicNode.Maintenance {
 			p.log.Info("clearing maintenance flag")
-			return p.setMaintenanceFlag(ironicNode, false)
+			return p.setMaintenanceFlag(ironicNode, false, "")
 		}
 		// This will return us to the manageable state without completing
 		// cleaning. Because cleaning happens in the process of moving from
@@ -1756,7 +1765,7 @@ func (p *ironicProvisioner) Delete() (result provisioner.Result, err error) {
 		// delete while bypassing Ironic's internal checks related to
 		// Nova.
 		p.log.Info("setting host maintenance flag to force image delete")
-		return p.setMaintenanceFlag(ironicNode, true)
+		return p.setMaintenanceFlag(ironicNode, true, "forcing deletion in baremetal-operator")
 	}
 
 	p.log.Info("host ready to be removed")


Test patch:
diff --git a/pkg/provisioner/ironic/delete_test.go b/pkg/provisioner/ironic/delete_test.go
index 3c7eba10..6603b36a 100644
--- a/pkg/provisioner/ironic/delete_test.go
+++ b/pkg/provisioner/ironic/delete_test.go
@@ -93,7 +93,7 @@ func deleteTest(t *testing.T, detach bool) {
 			hostName: "worker-0",
 			ironic:   testserver.NewIronic(t).Ready().NodeError(nodeUUID, http.StatusGatewayTimeout),
 
-			expectedError: "failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58: Expected HTTP response code \\[200\\].*",
+			expectedError: "failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58: Gateway Timeout.*",
 		},
 		{
 			name:   "not-ironic-node",
@@ -137,7 +137,7 @@ func deleteTest(t *testing.T, detach bool) {
 					ProvisionState: "active",
 					Maintenance:    false,
 				},
-			).NodeUpdateError(nodeUUID, http.StatusInternalServerError),
+			).NodeMaintenanceError(nodeUUID, http.StatusInternalServerError),
 
 			expectedError: "failed to set host maintenance flag",
 		},
@@ -149,7 +149,7 @@ func deleteTest(t *testing.T, detach bool) {
 					ProvisionState: "active",
 					Maintenance:    false,
 				},
-			).NodeUpdateError(nodeUUID, http.StatusConflict),
+			).NodeMaintenanceError(nodeUUID, http.StatusConflict),
 
 			expectedDirty:        true,
 			expectedRequestAfter: provisionRequeueDelay,
@@ -162,16 +162,11 @@ func deleteTest(t *testing.T, detach bool) {
 					ProvisionState: "active",
 					Maintenance:    false,
 				},
-			).NodeUpdate(nodes.Node{
+			).NodeMaintenance(nodes.Node{
 				UUID: nodeUUID,
-			}),
+			}, true),
 			expectedDirty:        true,
 			expectedRequestAfter: 0,
-			expectedUpdate: &nodes.UpdateOperation{
-				Op:    nodes.AddOp,
-				Path:  "/maintenance",
-				Value: true,
-			},
 		},
 	}
 
diff --git a/pkg/provisioner/ironic/provision_test.go b/pkg/provisioner/ironic/provision_test.go
index 13c63ab9..de8366b8 100644
--- a/pkg/provisioner/ironic/provision_test.go
+++ b/pkg/provisioner/ironic/provision_test.go
@@ -105,7 +105,9 @@ func TestProvision(t *testing.T) {
 				ProvisionState: string(nodes.Manageable),
 				UUID:           nodeUUID,
 				Maintenance:    true,
-			}),
+			}).NodeMaintenance(nodes.Node{
+				UUID: nodeUUID,
+			}, false),
 			expectedRequestAfter: 0,
 			expectedDirty:        true,
 		},
diff --git a/pkg/provisioner/ironic/testserver/ironic.go b/pkg/provisioner/ironic/testserver/ironic.go
index 7dec8774..46af5927 100644
--- a/pkg/provisioner/ironic/testserver/ironic.go
+++ b/pkg/provisioner/ironic/testserver/ironic.go
@@ -142,6 +142,33 @@ func (m *IronicMock) GetLastNodeUpdateRequestFor(id string) (updates []nodes.Upd
 	return
 }
 
+// NodeMaintenanceError configures configures the server with an error response for [PUT] /v1/nodes/{id}/maintenance
+func (m *IronicMock) NodeMaintenanceError(id string, errorCode int) *IronicMock {
+	m.ResponseWithCode(m.buildURL("/v1/nodes/"+id+"/maintenance", http.MethodPut), "", errorCode)
+	return m
+}
+
+// NodeUpdate configures the server with a valid response for PATCH
+// for /v1/nodes/{name,uuid}
+func (m *IronicMock) NodeMaintenance(node nodes.Node, expected bool) *IronicMock {
+	var url, method string
+
+	if expected {
+		method = http.MethodPut
+	} else {
+		method = http.MethodDelete
+	}
+
+	if node.Name != "" {
+		url = m.buildURL("/v1/nodes/"+node.Name+"/maintenance", method)
+	} else {
+		url = m.buildURL("/v1/nodes/"+node.UUID+"/maintenance", method)
+	}
+
+	m.ResponseWithCode(url, "{}", http.StatusAccepted)
+	return m
+}
+
 func (m *IronicMock) withNodeStatesProvision(nodeUUID string, method string) *IronicMock {
 	m.ResponseWithCode(m.buildURL("/v1/nodes/"+nodeUUID+"/states/provision", method), "{}", http.StatusAccepted)
 	return m
diff --git a/pkg/provisioner/ironic/updatehardwarestate_test.go b/pkg/provisioner/ironic/updatehardwarestate_test.go
index f2df0bc5..44a8442e 100644
--- a/pkg/provisioner/ironic/updatehardwarestate_test.go
+++ b/pkg/provisioner/ironic/updatehardwarestate_test.go
@@ -82,7 +82,7 @@ func TestUpdateHardwareState(t *testing.T) {
 			hostName: "worker-0",
 			ironic:   testserver.NewIronic(t).Ready().NodeError(nodeUUID, http.StatusGatewayTimeout),
 
-			expectedError: "failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58: Expected HTTP response code \\[200\\].*",
+			expectedError: "failed to find node by ID 33ce8659-7400-4c68-9535-d10766f07a58: Gateway Timeout.*",
 
 			expectUnreadablePower: true,
 		},