Bug 1932452: Backport ensure adoption is retried upon failure

apis/metal3.io/v1alpha1/baremetalhost_types.go

@@ -123,6 +123,9 @@ const (
 type ErrorType string
 
 const (
+	// ProvisionedRegistrationError is an error condition occurring when the controller
+	// is unable to re-register an already provisioned host.
+	ProvisionedRegistrationError ErrorType = "provisioned registration error"
 	// RegistrationError is an error condition occurring when the
 	// controller is unable to connect to the Host's baseboard management
 	// controller.
@@ -521,7 +524,7 @@ type BareMetalHostStatus struct {
 
 	// ErrorType indicates the type of failure encountered when the
 	// OperationalStatus is OperationalStatusError
-	// +kubebuilder:validation:Enum=registration error;inspection error;provisioning error;power management error
+	// +kubebuilder:validation:Enum=provisioned registration error;registration error;inspection error;provisioning error;power management error
 	ErrorType ErrorType `json:"errorType,omitempty"`
 
 	// LastUpdated identifies when this status was last observed.
@@ -608,48 +611,6 @@ func (host *BareMetalHost) BootMode() BootMode {
 	return mode
 }
 
-// Available returns true if the host is available to be provisioned.
-func (host *BareMetalHost) Available() bool {
-	if host.Spec.ConsumerRef != nil {
-		return false
-	}
-	if host.GetDeletionTimestamp() != nil {
-		return false
-	}
-	if host.HasError() {
-		return false
-	}
-	return true
-}
-
-// SetErrorMessage updates the ErrorMessage in the host Status struct
-// and increases the ErrorCount
-func (host *BareMetalHost) SetErrorMessage(errType ErrorType, message string) {
-	host.Status.OperationalStatus = OperationalStatusError
-	host.Status.ErrorType = errType
-	host.Status.ErrorMessage = message
-	host.Status.ErrorCount++
-}
-
-// ClearError removes any existing error message.
-func (host *BareMetalHost) ClearError() (dirty bool) {
-	dirty = host.SetOperationalStatus(OperationalStatusOK)
-	var emptyErrType ErrorType = ""
-	if host.Status.ErrorType != emptyErrType {
-		host.Status.ErrorType = emptyErrType
-		dirty = true
-	}
-	if host.Status.ErrorMessage != "" {
-		host.Status.ErrorMessage = ""
-		dirty = true
-	}
-	if host.Status.ErrorCount != 0 {
-		host.Status.ErrorCount = 0
-		dirty = true
-	}
-	return dirty
-}
-
 // setLabel updates the given label when necessary and returns true
 // when a change is made or false when no change is made.
 func (host *BareMetalHost) setLabel(name, value string) bool {
@@ -713,12 +674,6 @@ func (host *BareMetalHost) OperationalStatus() OperationalStatus {
 	return host.Status.OperationalStatus
 }
 
-// HasError returns a boolean indicating whether there is an error
-// set for the host.
-func (host *BareMetalHost) HasError() bool {
-	return host.Status.ErrorMessage != ""
-}
-
 // CredentialsKey returns a NamespacedName suitable for loading the
 // Secret containing the credentials associated with the host.
 func (host *BareMetalHost) CredentialsKey() types.NamespacedName {

apis/metal3.io/v1alpha1/baremetalhost_types_test.go

@@ -2,79 +2,13 @@ package v1alpha1
 
 import (
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func TestHostAvailable(t *testing.T) {
-	hostWithError := BareMetalHost{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "myhost",
-			Namespace: "myns",
-		},
-	}
-	hostWithError.SetErrorMessage(RegistrationError, "oops something went wrong")
-
-	testCases := []struct {
-		Host        BareMetalHost
-		Expected    bool
-		FailMessage string
-	}{
-		{
-			Host: BareMetalHost{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "myhost",
-					Namespace: "myns",
-				},
-			},
-			Expected:    true,
-			FailMessage: "available host returned not available",
-		},
-		{
-			Host:        hostWithError,
-			Expected:    false,
-			FailMessage: "host with error returned as available",
-		},
-		{
-			Host: BareMetalHost{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "myhost",
-					Namespace: "myns",
-				},
-				Spec: BareMetalHostSpec{
-					ConsumerRef: &corev1.ObjectReference{
-						Name:      "mymachine",
-						Namespace: "myns",
-					},
-				},
-			},
-			Expected:    false,
-			FailMessage: "host with consumerref returned as available",
-		},
-		{
-			Host: BareMetalHost{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:              "myhost",
-					Namespace:         "myns",
-					DeletionTimestamp: &metav1.Time{Time: time.Now()},
-				},
-			},
-			Expected:    false,
-			FailMessage: "deleted host returned as available",
-		},
-	}
-
-	for _, tc := range testCases {
-		if tc.Host.Available() != tc.Expected {
-			t.Error(tc.FailMessage)
-		}
-	}
-}
-
 func TestHostNeedsHardwareInspection(t *testing.T) {
 
 	testCases := []struct {
@@ -554,39 +488,3 @@ func TestBootMode(t *testing.T) {
 		})
 	}
 }
-
-func TestErrorCountIncrementsAlways(t *testing.T) {
-
-	b := &BareMetalHost{}
-	assert.Equal(t, b.Status.ErrorCount, 0)
-
-	b.SetErrorMessage(RegistrationError, "An error message")
-	assert.Equal(t, b.Status.ErrorCount, 1)
-
-	b.SetErrorMessage(InspectionError, "Another error message")
-	assert.Equal(t, b.Status.ErrorCount, 2)
-}
-
-func TestClearErrorCount(t *testing.T) {
-
-	b := &BareMetalHost{
-		Status: BareMetalHostStatus{
-			ErrorCount: 5,
-		},
-	}
-
-	assert.True(t, b.ClearError())
-	assert.Equal(t, 0, b.Status.ErrorCount)
-}
-
-func TestClearErrorCountOnlyIfNotZero(t *testing.T) {
-
-	b := &BareMetalHost{
-		Status: BareMetalHostStatus{
-			ErrorCount: 5,
-		},
-	}
-
-	assert.True(t, b.ClearError())
-	assert.False(t, b.ClearError())
-}

config/crd/bases/metal3.io_baremetalhosts.yaml

@@ -262,6 +262,7 @@ spec:
               errorType:
                 description: ErrorType indicates the type of failure encountered when the OperationalStatus is OperationalStatusError
                 enum:
+                - provisioned registration error
                 - registration error
                 - inspection error
                 - provisioning error

config/render/capm3.yaml

@@ -260,6 +260,7 @@ spec:
               errorType:
                 description: ErrorType indicates the type of failure encountered when the OperationalStatus is OperationalStatusError
                 enum:
+                - provisioned registration error
                 - registration error
                 - inspection error
                 - provisioning error

controllers/metal3.io/action_result.go

@@ -25,17 +25,6 @@ type actionResult interface {
 	Dirty() bool
 }
 
-// actionContinueNoWrite is a result indicating that the current action is still
-// in progress, and that the resource should remain in the same provisioning
-// state without writing the status
-type actionContinueNoWrite struct {
-	actionContinue
-}
-
-func (r actionContinueNoWrite) Dirty() bool {
-	return false
-}
-
 // actionContinue is a result indicating that the current action is still
 // in progress, and that the resource should remain in the same provisioning
 // state.
@@ -51,6 +40,17 @@ func (r actionContinue) Result() (result reconcile.Result, err error) {
 }
 
 func (r actionContinue) Dirty() bool {
+	return false
+}
+
+// actionUpdate is a result indicating that the current action is still
+// in progress, and that the resource should remain in the same provisioning
+// state but write new Status data.
+type actionUpdate struct {
+	actionContinue
+}
+
+func (r actionUpdate) Dirty() bool {
 	return true
 }