Repository: metal3-io/baremetal-operator
Pull request #1439 (2023-11-10 03:08:20)

:bug: Don't munge timestamp in structured logs, again
Structured logging tools expect a POSIX timestamp. 0a887a16fb26dd26a2b372953ec8d860a3a251b5 reverted to changing the timestamp formatting only in development log mode, after 9ecf1718dfe3f785e151b6d2b5a01ba04e0a00a9 had explicitly and unconditionally set the format to ISO-8601.

However, almost immediately after this, 5bf044038b17aec6c2265d325046f475bc4bb52f bumped the controller-runtime version from 0.13.1 to 0.14.5. In 0.14, the default zap options in controller-runtime [changed to RFC3339](https://github.com/kubernetes-sigs/controller-runtime/commit/60e524b7f4a6264662adfd1a2bd3e40b75393270) - which is the same as ISO-8601 but even worse because subsecond precision (which can be very important to debugging) is lost as well.

This change explicitly restores the original default.

metal3-io-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please ask for approval from [zaneb](https://github.com/zaneb). For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=metal3-io%2Fbaremetal-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/metal3-io/baremetal-operator/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["zaneb"]} -->

Patch:
diff --git a/apis/go.mod b/apis/go.mod
index f90eaec1..49a9062e 100644
--- a/apis/go.mod
+++ b/apis/go.mod
@@ -5,6 +5,7 @@ go 1.20
 require (
 	github.com/google/uuid v1.4.0
 	github.com/metal3-io/baremetal-operator/pkg/hardwareutils v0.4.0
+	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.8.4
 	golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1
 	k8s.io/api v0.27.7
@@ -33,7 +34,6 @@ require (
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
-	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_golang v1.15.1 // indirect
 	github.com/prometheus/client_model v0.4.0 // indirect
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_validation.go b/apis/metal3.io/v1alpha1/baremetalhost_validation.go
index e1073d85..6b453bae 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_validation.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_validation.go
@@ -2,7 +2,6 @@ package v1alpha1
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net"
 	"net/url"
@@ -10,20 +9,19 @@ import (
 	"strings"
 
 	"github.com/google/uuid"
+	"github.com/pkg/errors"
 	"golang.org/x/exp/slices"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/metal3-io/baremetal-operator/pkg/hardwareutils/bmc"
 )
 
-var (
-	supportedRebootModes           = []string{"hard", "soft", ""}
-	supportedRebootModesString     = "\"hard\", \"soft\" or \"\""
-	inspectAnnotationAllowed       = []string{"disabled", ""}
-	inspectAnnotationAllowedString = "\"disabled\" or \"\""
-)
+// log is for logging in this package.
+var log = logf.Log.WithName("baremetalhost-validation")
 
 // validateHost validates BareMetalHost resource for creation
 func (host *BareMetalHost) validateHost() []error {
+	log.Info("validate create", "name", host.Name)
 	var errs []error
 	var bmcAccess bmc.AccessDetails
 
@@ -35,8 +33,8 @@ func (host *BareMetalHost) validateHost() []error {
 		}
 	}
 
-	if raidErrors := validateRAID(host.Spec.RAID); raidErrors != nil {
-		errs = append(errs, raidErrors...)
+	if raid_errors := validateRAID(host.Spec.RAID); raid_errors != nil {
+		errs = append(errs, raid_errors...)
 	}
 
 	errs = append(errs, validateBMCAccess(host.Spec, bmcAccess)...)
@@ -69,6 +67,7 @@ func (host *BareMetalHost) validateHost() []error {
 // validateChanges validates BareMetalHost resource on changes
 // but also covers the validations of creation
 func (host *BareMetalHost) validateChanges(old *BareMetalHost) []error {
+	log.Info("validate update", "name", host.Name)
 	var errs []error
 
 	if err := host.validateHost(); err != nil {
@@ -76,11 +75,11 @@ func (host *BareMetalHost) validateChanges(old *BareMetalHost) []error {
 	}
 
 	if old.Spec.BMC.Address != "" && host.Spec.BMC.Address != old.Spec.BMC.Address {
-		errs = append(errs, errors.New("BMC address can not be changed once it is set"))
+		errs = append(errs, fmt.Errorf("BMC address can not be changed once it is set"))
 	}
 
 	if old.Spec.BootMACAddress != "" && host.Spec.BootMACAddress != old.Spec.BootMACAddress {
-		errs = append(errs, errors.New("bootMACAddress can not be changed once it is set"))
+		errs = append(errs, fmt.Errorf("bootMACAddress can not be changed once it is set"))
 	}
 
 	return errs
@@ -124,7 +123,7 @@ func validateBMCAccess(s BareMetalHostSpec, bmcAccess bmc.AccessDetails) []error
 }
 
 func validateRAID(r *RAIDConfig) []error {
-	var errs []error
+	var errors []error
 
 	if r == nil {
 		return nil
@@ -132,37 +131,37 @@ func validateRAID(r *RAIDConfig) []error {
 
 	// check if both hardware and software RAID are specified
 	if len(r.HardwareRAIDVolumes) > 0 && len(r.SoftwareRAIDVolumes) > 0 {
-		errs = append(errs, errors.New("hardwareRAIDVolumes and softwareRAIDVolumes can not be set at the same time"))
+		errors = append(errors, fmt.Errorf("hardwareRAIDVolumes and softwareRAIDVolumes can not be set at the same time"))
 	}
 
 	for index, volume := range r.HardwareRAIDVolumes {
 		// check if physicalDisks are specified without a controller
 		if len(volume.PhysicalDisks) != 0 {
 			if volume.Controller == "" {
-				errs = append(errs, fmt.Errorf("'physicalDisks' specified without 'controller' in hardware RAID volume %d", index))
+				errors = append(errors, fmt.Errorf("'physicalDisks' specified without 'controller' in hardware RAID volume %d", index))
 			}
 		}
 		// check if numberOfPhysicalDisks is not same as len(physicalDisks)
 		if volume.NumberOfPhysicalDisks != nil && len(volume.PhysicalDisks) != 0 {
 			if *volume.NumberOfPhysicalDisks != len(volume.PhysicalDisks) {
-				errs = append(errs, fmt.Errorf("the 'numberOfPhysicalDisks'[%d] and number of 'physicalDisks'[%d] is not same for volume %d", *volume.NumberOfPhysicalDisks, len(volume.PhysicalDisks), index))
+				errors = append(errors, fmt.Errorf("the 'numberOfPhysicalDisks'[%d] and number of 'physicalDisks'[%d] is not same for volume %d", *volume.NumberOfPhysicalDisks, len(volume.PhysicalDisks), index))
 			}
 		}
 	}
 
-	return errs
+	return errors
 }
 
 func validateBMHName(bmhname string) error {
 
 	invalidname, _ := regexp.MatchString(`[^A-Za-z0-9\.\-\_]`, bmhname)
 	if invalidname {
-		return errors.New("BareMetalHost resource name cannot contain characters other than [A-Za-z0-9._-]")
+		return fmt.Errorf("BareMetalHost resource name cannot contain characters other than [A-Za-z0-9._-]")
 	}
 
 	_, err := uuid.Parse(bmhname)
 	if err == nil {
-		return errors.New("BareMetalHost resource name cannot be a UUID")
+		return fmt.Errorf("BareMetalHost resource name cannot be a UUID")
 	}
 
 	return nil
@@ -175,7 +174,11 @@ func validateDNSName(hostaddress string) error {
 	}
 
 	_, err := bmc.GetParsedURL(hostaddress)
-	return err // the error has enough context already
+	if err != nil {
+		return errors.Wrap(err, "BMO validation")
+	}
+
+	return nil
 }
 
 func validateAnnotations(host *BareMetalHost) []error {
@@ -213,7 +216,7 @@ func validateStatusAnnotation(statusAnnotation string) error {
 		deco := json.NewDecoder(strings.NewReader(statusAnnotation))
 		deco.DisallowUnknownFields()
 		if err := deco.Decode(objBMHStatus); err != nil {
-			return fmt.Errorf("error decoding status annotation: %w", err)
+			return fmt.Errorf("error decoding status annotation, error=%w", err)
 		}
 
 		if err := checkStatusAnnotation(objBMHStatus); err != nil {
@@ -228,7 +231,7 @@ func validateImageURL(imageURL string) error {
 
 	_, err := url.ParseRequestURI(imageURL)
 	if err != nil {
-		return fmt.Errorf("image URL %s is invalid: %w", imageURL, err)
+		return fmt.Errorf("Image URL %s is an invalid URL", imageURL)
 	}
 
 	return nil
@@ -241,12 +244,12 @@ func validateRootDeviceHints(rdh *RootDeviceHints) error {
 
 	subpath := strings.TrimPrefix(rdh.DeviceName, "/dev/")
 	if rdh.DeviceName == subpath {
-		return fmt.Errorf("device name of root device hint must be a /dev/ path, not \"%s\"", rdh.DeviceName)
+		return fmt.Errorf("Device Name of root device hint must be a /dev/ path, not \"%s\"", rdh.DeviceName)
 	}
 
 	subpath = strings.TrimPrefix(subpath, "disk/by-path/")
 	if strings.Contains(subpath, "/") {
-		return fmt.Errorf("device name of root device hint must be path in /dev/ or /dev/disk/by-path/, not \"%s\"", rdh.DeviceName)
+		return fmt.Errorf("Device Name of root device hint must be path in /dev/ or /dev/disk/by-path/, not \"%s\"", rdh.DeviceName)
 	}
 	return nil
 }
@@ -258,11 +261,11 @@ func validateRootDeviceHints(rdh *RootDeviceHints) error {
 func checkStatusAnnotation(bmhStatus *BareMetalHostStatus) error {
 
 	if !slices.Contains(OperationalStatusAllowed, string(bmhStatus.OperationalStatus)) {
-		return fmt.Errorf("invalid operationalStatus '%s' in the %s annotation", string(bmhStatus.OperationalStatus), StatusAnnotation)
+		return fmt.Errorf("invalid OperationalStatus='%s' in StatusAnnotation", string(bmhStatus.OperationalStatus))
 	}
 
 	if !slices.Contains(ErrorTypeAllowed, string(bmhStatus.ErrorType)) {
-		return fmt.Errorf("invalid errorType '%s' in the %s annotation", string(bmhStatus.ErrorType), StatusAnnotation)
+		return fmt.Errorf("invalid ErrorType='%s' in StatusAnnotation", string(bmhStatus.ErrorType))
 	}
 
 	return nil
@@ -274,7 +277,7 @@ func validateHwdDetailsAnnotation(hwdDetAnnotation string, inspect string) error
 	}
 
 	if inspect != "disabled" {
-		return errors.New("when hardware details are provided, the inspect.metal3.io annotation must be set to disabled")
+		return fmt.Errorf("inspection has to be disabled for HardwareDetailsAnnotation, check if {'inspect.metal3.io' : 'disabled'}")
 	}
 
 	objHwdDet := &HardwareDetails{}
@@ -282,15 +285,18 @@ func validateHwdDetailsAnnotation(hwdDetAnnotation string, inspect string) error
 	deco := json.NewDecoder(strings.NewReader(hwdDetAnnotation))
 	deco.DisallowUnknownFields()
 	if err := deco.Decode(objHwdDet); err != nil {
-		return fmt.Errorf("error decoding the %s annotation: %w", HardwareDetailsAnnotation, err)
+		return fmt.Errorf("error decoding hardware details annotation, error=%w", err)
 	}
 
 	return nil
 }
 
 func validateInspectAnnotation(inspectAnnotation string) error {
+
+	inspectAnnotationAllowed := []string{"disabled", ""}
+
 	if !slices.Contains(inspectAnnotationAllowed, inspectAnnotation) {
-		return fmt.Errorf("invalid value for the %s annotation, allowed are %v", InspectAnnotationPrefix, inspectAnnotationAllowedString)
+		return fmt.Errorf("invalid value for Inspect Annotation")
 	}
 
 	return nil
@@ -304,11 +310,13 @@ func validateRebootAnnotation(rebootAnnotation string) error {
 	objStatus := &RebootAnnotationArguments{}
 	err := json.Unmarshal([]byte(rebootAnnotation), objStatus)
 	if err != nil {
-		return fmt.Errorf("failed to unmarshal the data from the %s annotation: %w", RebootAnnotationPrefix, err)
+		return fmt.Errorf("failed to fetch Reboot from annotation, %w", err)
 	}
 
+	supportedRebootModes := []string{"hard", "soft", ""}
+
 	if !slices.Contains(supportedRebootModes, string(objStatus.Mode)) {
-		return fmt.Errorf("invalid mode in the %s annotation, allowed are %v", RebootAnnotationPrefix, supportedRebootModesString)
+		return fmt.Errorf("invalid RebootMode in RebootAnnotation")
 	}
 
 	return nil
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_webhook.go b/apis/metal3.io/v1alpha1/baremetalhost_webhook.go
index 0e5f646f..55acaaaf 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_webhook.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_webhook.go
@@ -23,22 +23,22 @@ import (
 )
 
 // log is for logging in this package.
-var baremetalhostlog = logf.Log.WithName("webhooks").WithName("BareMetalHost")
+var baremetalhostlog = logf.Log.WithName("baremetalhost-resource")
 
 //+kubebuilder:webhook:verbs=create;update,path=/validate-metal3-io-v1alpha1-baremetalhost,mutating=false,failurePolicy=fail,sideEffects=none,admissionReviewVersions=v1;v1beta,groups=metal3.io,resources=baremetalhosts,versions=v1alpha1,name=baremetalhost.metal3.io
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (r *BareMetalHost) ValidateCreate() (admission.Warnings, error) {
-	baremetalhostlog.Info("validate create", "namespace", r.Namespace, "name", r.Name)
+	baremetalhostlog.Info("validate create", "name", r.Name)
 	return nil, errors.NewAggregate(r.validateHost())
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *BareMetalHost) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	baremetalhostlog.Info("validate update", "namespace", r.Namespace, "name", r.Name)
+	baremetalhostlog.Info("validate update", "name", r.Name)
 	bmh, casted := old.(*BareMetalHost)
 	if !casted {
-		baremetalhostlog.Error(fmt.Errorf("old object conversion error for %s/%s", r.Namespace, r.Name), "validate update error")
+		baremetalhostlog.Error(fmt.Errorf("old object conversion error"), "validate update error")
 		return nil, nil
 	}
 	return nil, errors.NewAggregate(r.validateChanges(bmh))
diff --git a/apis/metal3.io/v1alpha1/bmceventsubscription_validation.go b/apis/metal3.io/v1alpha1/bmceventsubscription_validation.go
index 96ba7544..73b86dd6 100644
--- a/apis/metal3.io/v1alpha1/bmceventsubscription_validation.go
+++ b/apis/metal3.io/v1alpha1/bmceventsubscription_validation.go
@@ -16,29 +16,36 @@ limitations under the License.
 package v1alpha1
 
 import (
-	"errors"
 	"fmt"
 	"net/url"
+
+	"github.com/pkg/errors"
+
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+// bmclog is for logging in this package.
+var bmclog = logf.Log.WithName("bmceventsubscription-validation")
+
 // validateSubscription validates BMCEventSubscription resource for creation
 func (s *BMCEventSubscription) validateSubscription() []error {
+	bmclog.Info("validate create", "name", s.Name)
 	var errs []error
 
 	if s.Spec.HostName == "" {
-		errs = append(errs, errors.New("hostName cannot be empty"))
+		errs = append(errs, fmt.Errorf("HostName cannot be empty"))
 	}
 
 	if s.Spec.Destination == "" {
-		errs = append(errs, errors.New("destination cannot be empty"))
+		errs = append(errs, fmt.Errorf("Destination cannot be empty"))
 	} else {
 		destinationUrl, err := url.ParseRequestURI(s.Spec.Destination)
 
 		if err != nil {
-			errs = append(errs, fmt.Errorf("destination is invalid: %w", err))
+			errs = append(errs, errors.Wrap(err, "Destination is an invalid URL"))
 		} else {
 			if destinationUrl.Path == "" {
-				errs = append(errs, errors.New("hostname-only destination must have a trailing slash"))
+				errs = append(errs, fmt.Errorf("Hostname-only destination must have a trailing slash"))
 			}
 		}
 	}
diff --git a/apis/metal3.io/v1alpha1/bmceventsubscription_webhook.go b/apis/metal3.io/v1alpha1/bmceventsubscription_webhook.go
index 43a06a4c..766820be 100644
--- a/apis/metal3.io/v1alpha1/bmceventsubscription_webhook.go
+++ b/apis/metal3.io/v1alpha1/bmceventsubscription_webhook.go
@@ -23,13 +23,13 @@ import (
 )
 
 // bmcsubscriptionlog is for logging in this package.
-var bmcsubscriptionlog = logf.Log.WithName("webhooks").WithName("BMCEventSubscription")
+var bmcsubscriptionlog = logf.Log.WithName("bmceventsubscription-resource")
 
 //+kubebuilder:webhook:verbs=create;update,path=/validate-metal3-io-v1alpha1-bmceventsubscription,mutating=false,failurePolicy=fail,sideEffects=none,admissionReviewVersions=v1;v1beta,groups=metal3.io,resources=bmceventsubscriptions,versions=v1alpha1,name=bmceventsubscription.metal3.io
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (s *BMCEventSubscription) ValidateCreate() (admission.Warnings, error) {
-	bmcsubscriptionlog.Info("validate create", "namespace", s.Namespace, "name", s.Name)
+	bmcsubscriptionlog.Info("validate create", "name", s.Name)
 	return nil, errors.NewAggregate(s.validateSubscription())
 }
 
@@ -37,11 +37,11 @@ func (s *BMCEventSubscription) ValidateCreate() (admission.Warnings, error) {
 //
 // We prevent updates to the spec.  All other updates (e.g. status, finalizers) are allowed.
 func (s *BMCEventSubscription) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	bmcsubscriptionlog.Info("validate update", "namespace", s.Namespace, "name", s.Name)
+	bmcsubscriptionlog.Info("validate update", "name", s.Name)
 
 	bes, casted := old.(*BMCEventSubscription)
 	if !casted {
-		bmcsubscriptionlog.Error(fmt.Errorf("old object conversion error for %s/%s", s.Namespace, s.Name), "validate update error")
+		bmcsubscriptionlog.Error(fmt.Errorf("old object conversion error"), "validate update error")
 		return nil, nil
 	}
 
diff --git a/main.go b/main.go
index b3db5ae3..8ea82bf3 100644
--- a/main.go
+++ b/main.go
@@ -175,6 +175,8 @@ func main() {
 	if devLogging {
 		logOpts.Development = true
 		logOpts.TimeEncoder = zapcore.ISO8601TimeEncoder
+	} else {
+		logOpts.TimeEncoder = zapcore.EpochTimeEncoder
 	}
 	ctrl.SetLogger(zap.New(zap.UseFlagOptions(&logOpts)))
 
diff --git a/pkg/provisioner/ironic/ironic.go b/pkg/provisioner/ironic/ironic.go
index 2dc7e6d1..8d4f952a 100644
--- a/pkg/provisioner/ironic/ironic.go
+++ b/pkg/provisioner/ironic/ironic.go
@@ -546,8 +546,7 @@ func (p *ironicProvisioner) configureImages(data provisioner.ManagementAccessDat
 	deployImageInfo := setDeployImage(p.config, bmcAccess, data.PreprovisioningImage)
 	updater.SetDriverInfoOpts(deployImageInfo, ironicNode)
 
-	// NOTE(dtantsur): It is risky to update image information for active nodes since it may affect the ability to clean up.
-	if (data.CurrentImage != nil || data.HasCustomDeploy) && ironicNode.ProvisionState != string(nodes.Active) {
+	if data.CurrentImage != nil || data.HasCustomDeploy {
 		p.getImageUpdateOptsForNode(ironicNode, data.CurrentImage, data.BootMode, data.HasCustomDeploy, updater)
 	}
 	updater.SetTopLevelOpt("automated_clean",


Test patch:
diff --git a/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go b/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go
index bad5ec27..e5dd5aa3 100644
--- a/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go
+++ b/apis/metal3.io/v1alpha1/baremetalhost_validation_test.go
@@ -392,7 +392,7 @@ func TestValidateCreate(t *testing.T) {
 					BMC: BMCDetails{
 						Address: "ipmi://-host.example.com.org"}}},
 			oldBMH:    nil,
-			wantedErr: "failed to parse BMC address information: BMC address hostname/IP : [-host.example.com.org] is invalid",
+			wantedErr: "BMO validation: failed to parse BMC address information: BMC address hostname/IP : [-host.example.com.org] is invalid",
 		},
 		{
 			name: "invalidDNSNameinvalidcharacter",
@@ -403,7 +403,7 @@ func TestValidateCreate(t *testing.T) {
 					BMC: BMCDetails{
 						Address: "ipmi://host+1.example.com.org"}}},
 			oldBMH:    nil,
-			wantedErr: "failed to parse BMC address information: BMC address hostname/IP : [host+1.example.com.org] is invalid",
+			wantedErr: "BMO validation: failed to parse BMC address information: BMC address hostname/IP : [host+1.example.com.org] is invalid",
 		},
 		{
 			name: "invalidDNSNameinvalidformat",
@@ -414,7 +414,7 @@ func TestValidateCreate(t *testing.T) {
 					BMC: BMCDetails{
 						Address: "[@]host.example.com"}}},
 			oldBMH:    nil,
-			wantedErr: "failed to parse BMC address information: parse \"ipmi://[@]host.example.com\": net/url: invalid userinfo",
+			wantedErr: "BMO validation: failed to parse BMC address information: parse \"ipmi://[@]host.example.com\": net/url: invalid userinfo",
 		},
 		{
 			name: "invalidDNSNameinvalidbmc",
@@ -436,7 +436,7 @@ func TestValidateCreate(t *testing.T) {
 					BMC: BMCDetails{
 						Address: "ipmi://[fe80::fc33:62ff:fe33:8xff]:6223"}}},
 			oldBMH:    nil,
-			wantedErr: "failed to parse BMC address information: BMC address hostname/IP : [fe80::fc33:62ff:fe33:8xff] is invalid",
+			wantedErr: "BMO validation: failed to parse BMC address information: BMC address hostname/IP : [fe80::fc33:62ff:fe33:8xff] is invalid",
 		},
 		{
 			name: "validRootDeviceHint",
@@ -478,7 +478,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "device name of root device hint must be path in /dev/ or /dev/disk/by-path/, not \"/dev/disk/by-uuid/cdaacd50-3a4c-421c-91c0-fe9ba7b8b2f1\"",
+			wantedErr: "Device Name of root device hint must be path in /dev/ or /dev/disk/by-path/, not \"/dev/disk/by-uuid/cdaacd50-3a4c-421c-91c0-fe9ba7b8b2f1\"",
 		},
 		{
 			name: "invalidRootDeviceHintNoPath",
@@ -492,7 +492,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "device name of root device hint must be a /dev/ path, not \"sda\"",
+			wantedErr: "Device Name of root device hint must be a /dev/ path, not \"sda\"",
 		},
 		{
 			name: "invalidImageURL",
@@ -510,7 +510,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "image URL test1 is invalid: parse \"test1\": invalid URI for request",
+			wantedErr: "Image URL test1 is an invalid URL",
 		},
 		{
 			name: "validStatusAnnotation",
@@ -540,7 +540,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "error decoding status annotation: json: unknown field \"InvalidField\"",
+			wantedErr: "error decoding status annotation, error=json: unknown field \"InvalidField\"",
 		},
 		{
 			name: "invalidOpstatusStatusAnnotation",
@@ -555,7 +555,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "invalid operationalStatus 'NotOK' in the baremetalhost.metal3.io/status annotation",
+			wantedErr: "invalid OperationalStatus='NotOK' in StatusAnnotation",
 		},
 		{
 			name: "invalidErrtypeStatusAnnotation",
@@ -570,7 +570,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "invalid errorType 'No Error' in the baremetalhost.metal3.io/status annotation",
+			wantedErr: "invalid ErrorType='No Error' in StatusAnnotation",
 		},
 		{
 			name: "invalidFormatStatusAnnotation",
@@ -585,7 +585,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "error decoding status annotation: unexpected EOF",
+			wantedErr: "error decoding status annotation, error=unexpected EOF",
 		},
 		{
 			name: "invalidValueRebootAnnotationPrefix",
@@ -600,7 +600,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "invalid mode in the reboot.metal3.io annotation, allowed are \"hard\", \"soft\" or \"\"",
+			wantedErr: "invalid RebootMode in RebootAnnotation",
 		},
 		{
 			name: "invalidValueRebootAnnotationWithKey",
@@ -615,7 +615,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "invalid mode in the reboot.metal3.io annotation, allowed are \"hard\", \"soft\" or \"\"",
+			wantedErr: "invalid RebootMode in RebootAnnotation",
 		},
 		{
 			name: "inspectionNotDisabledHardwareDetailsAnnotation",
@@ -630,7 +630,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "when hardware details are provided, the inspect.metal3.io annotation must be set to disabled",
+			wantedErr: "inspection has to be disabled for HardwareDetailsAnnotation, check if {'inspect.metal3.io' : 'disabled'}",
 		},
 		{
 			name: "invalidFieldHardwareDetailsAnnotation",
@@ -646,7 +646,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "error decoding the inspect.metal3.io/hardwaredetails annotation: json: unknown field \"INVALIDField\"",
+			wantedErr: "error decoding hardware details annotation, error=json: unknown field \"INVALIDField\"",
 		},
 		{
 			name: "invalidJsonHardwareDetailsAnnotation",
@@ -661,7 +661,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "error decoding the inspect.metal3.io/hardwaredetails annotation: unexpected EOF",
+			wantedErr: "error decoding hardware details annotation, error=unexpected EOF",
 		},
 		{
 			name: "invalidValueInspectAnnotation",
@@ -676,7 +676,7 @@ func TestValidateCreate(t *testing.T) {
 				},
 			},
 			oldBMH:    nil,
-			wantedErr: "invalid value for the inspect.metal3.io annotation, allowed are \"disabled\" or \"\"",
+			wantedErr: "invalid value for Inspect Annotation",
 		},
 	}
 
diff --git a/apis/metal3.io/v1alpha1/bmceventsubscription_validation_test.go b/apis/metal3.io/v1alpha1/bmceventsubscription_validation_test.go
index c10514b7..712f53d4 100644
--- a/apis/metal3.io/v1alpha1/bmceventsubscription_validation_test.go
+++ b/apis/metal3.io/v1alpha1/bmceventsubscription_validation_test.go
@@ -37,7 +37,7 @@ func TestBMCEventSubscriptionValidateCreate(t *testing.T) {
 				Spec:       BMCEventSubscriptionSpec{Destination: "http://localhost/abc/abc"},
 			},
 			oldS:      nil,
-			wantedErr: "hostName cannot be empty",
+			wantedErr: "HostName cannot be empty",
 		},
 		{
 			name: "missingDestination",
@@ -47,7 +47,7 @@ func TestBMCEventSubscriptionValidateCreate(t *testing.T) {
 				Spec:       BMCEventSubscriptionSpec{HostName: "worker-01"},
 			},
 			oldS:      nil,
-			wantedErr: "destination cannot be empty",
+			wantedErr: "Destination cannot be empty",
 		},
 		{
 			name: "destinationNotUrl",
@@ -57,7 +57,7 @@ func TestBMCEventSubscriptionValidateCreate(t *testing.T) {
 				Spec:       BMCEventSubscriptionSpec{HostName: "worker-01", Destination: "abc"},
 			},
 			oldS:      nil,
-			wantedErr: "destination is invalid: parse \"abc\": invalid URI for request",
+			wantedErr: "Destination is an invalid URL: parse \"abc\": invalid URI for request",
 		},
 		{
 			name: "destinationMissingTrailingSlash",
@@ -67,7 +67,7 @@ func TestBMCEventSubscriptionValidateCreate(t *testing.T) {
 				Spec:       BMCEventSubscriptionSpec{HostName: "worker-01", Destination: "http://localhost"},
 			},
 			oldS:      nil,
-			wantedErr: "hostname-only destination must have a trailing slash",
+			wantedErr: "Hostname-only destination must have a trailing slash",
 		},
 	}
 
diff --git a/hack/ci-e2e.sh b/hack/ci-e2e.sh
index e544e8f0..5158f939 100755
--- a/hack/ci-e2e.sh
+++ b/hack/ci-e2e.sh
@@ -1,21 +1,10 @@
 #!/usr/bin/env bash
 
-# -----------------------------------------------------------------------------
-# Description: This script sets up the environment and runs E2E tests for the
-#              BMO project. It uses either vbmc or sushy-tools based on 
-#              the BMO_E2E_EMULATOR environment variable.
-# Usage:       export BMO_E2E_EMULATOR="vbmc"  # Or "sushy-tools"
-#              ./ci-e2e.sh
-# -----------------------------------------------------------------------------
-
 set -eux
 
 REPO_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
 cd "${REPO_ROOT}" || exit 1
 
-# BMO_E2E_EMULATOR can be set to either "vbmc" or "sushy-tools"
-BMO_E2E_EMULATOR=${BMO_E2E_EMULATOR:-"sushy-tools"}
-
 # Ensure requirements are installed
 "${REPO_ROOT}/hack/e2e/ensure_go.sh"
 export PATH="${PATH}:/usr/local/go/bin"
@@ -43,83 +32,58 @@ minikube start
 # Load the BMO e2e image into it
 minikube image load quay.io/metal3-io/baremetal-operator:e2e
 
+# Start VBMC
+docker run --name vbmc --network host -d \
+  -v /var/run/libvirt/libvirt-sock:/var/run/libvirt/libvirt-sock \
+  -v /var/run/libvirt/libvirt-sock-ro:/var/run/libvirt/libvirt-sock-ro \
+  quay.io/metal3-io/vbmc
+
 # Create libvirt domain
 VM_NAME="bmo-e2e-0"
-export BOOT_MAC_ADDRESS="00:60:2f:31:81:01"
-
-virt-install \
-  --connect qemu:///system \
-  --name "${VM_NAME}" \
-  --description "Virtualized BareMetalHost" \
-  --osinfo=ubuntu-lts-latest \
-  --ram=4096 \
-  --vcpus=2 \
-  --disk size=20 \
-  --graphics=none \
-  --console pty \
-  --serial pty \
-  --pxe \
-  --network network=baremetal-e2e,mac="${BOOT_MAC_ADDRESS}" \
-  --noautoconsole
+BOOT_MAC_ADDRESS="00:60:2f:31:81:01"
+VBMC_PORT="16230"
+virt-install --connect qemu:///system -n "${VM_NAME}" --description "Virtualized BareMetalHost" --osinfo=ubuntu-lts-latest \
+  --ram=4096 --vcpus=2 --disk size=20 --graphics=none --console pty --serial pty --pxe \
+  --network network=baremetal-e2e,mac="${BOOT_MAC_ADDRESS}" --noautoconsole
 
-# This IP is defined by the network we created above.
-IP_ADDRESS="192.168.222.1"
+# Add BMH VM to VBMC
+docker exec vbmc vbmc add "${VM_NAME}" --port "${VBMC_PORT}"
+docker exec vbmc vbmc start "${VM_NAME}"
+docker exec vbmc vbmc list
 
 # These variables are used by the tests. They override variables in the config file.
+# This IP is defined by the network we created above.
+# Together with the VBMC_PORT this becomes the BMC_ADDRESS used by the BMH in the test.
+IP_ADDRESS="192.168.222.1"
+export BMC_ADDRESS="ipmi://${IP_ADDRESS}:${VBMC_PORT}"
+export BOOT_MAC_ADDRESS
 # These are the VBMC defaults (used since we did not specify anything else for `vbmc add`).
 export BMC_USER=admin
 export BMC_PASSWORD=password
-
-if [[ "${BMO_E2E_EMULATOR}" == "vbmc" ]]; then
-	# VBMC variables
-  VBMC_PORT="16230"
-  export BMC_ADDRESS="ipmi://${IP_ADDRESS}:${VBMC_PORT}"
-
-  # Start VBMC
-  docker run --name vbmc --network host -d \
-    -v /var/run/libvirt/libvirt-sock:/var/run/libvirt/libvirt-sock \
-    -v /var/run/libvirt/libvirt-sock-ro:/var/run/libvirt/libvirt-sock-ro \
-    quay.io/metal3-io/vbmc
-
-  # Add BMH VM to VBMC
-  docker exec vbmc vbmc add "${VM_NAME}" --port "${VBMC_PORT}"
-  docker exec vbmc vbmc start "${VM_NAME}"
-  docker exec vbmc vbmc list
-
-elif [[ "${BMO_E2E_EMULATOR}" == "sushy-tools" ]]; then
-  # Sushy-tools variables
-  SUSHY_EMULATOR_FILE="${REPO_ROOT}"/test/e2e/sushy-tools/sushy-emulator.conf
-  SUSHY_PORT="8000"
-  export BMC_ADDRESS="redfish+http://${IP_ADDRESS}:${SUSHY_PORT}/redfish/v1/Systems/${VM_NAME}"
-
-  # Start sushy-tools
-  docker run --name sushy-tools -d --network host \
-    -v "${SUSHY_EMULATOR_FILE}":/etc/sushy/sushy-emulator.conf:Z \
-    -v /var/run/libvirt:/var/run/libvirt:Z \
-    -e SUSHY_EMULATOR_CONFIG=/etc/sushy/sushy-emulator.conf \
-    quay.io/metal3-io/sushy-tools:latest sushy-emulator
-
-else
-  echo "Invalid e2e emulator specified: ${BMO_E2E_EMULATOR}"
-  exit 1
-fi
-
-# Image server variables
 CIRROS_VERSION="0.6.2"
 IMAGE_FILE="cirros-${CIRROS_VERSION}-x86_64-disk.img"
 export IMAGE_CHECKSUM="c8fc807773e5354afe61636071771906"
 export IMAGE_URL="http://${IP_ADDRESS}/${IMAGE_FILE}"
-IMAGE_DIR="${REPO_ROOT}/test/e2e/images"
+IMAGE_FOLDER="${REPO_ROOT}/test/e2e/images"
+
+## Setup image server
+# Create a directory for images
+
+mkdir -p "${IMAGE_FOLDER}"
+pushd "${IMAGE_FOLDER}"
+
+## Setup image server
+# Check if IMAGE_FILE already exists
+if [[ ! -f "${IMAGE_FILE}" ]]; then
+    wget "https://download.cirros-cloud.net/${CIRROS_VERSION}/${IMAGE_FILE}"
+else
+    echo "${IMAGE_FILE} already exists. Skipping download."
+fi
 
-## Download and run image server
-mkdir -p "${IMAGE_DIR}"
-pushd "${IMAGE_DIR}"
-wget --quiet "https://download.cirros-cloud.net/${CIRROS_VERSION}/${IMAGE_FILE}"
 popd
 
-docker run --name image-server-e2e -d \
-  -p 80:8080 \
-  -v "${IMAGE_DIR}:/usr/share/nginx/html" nginxinc/nginx-unprivileged
+# Run image server
+docker run --rm --name image-server-e2e -d -p 80:8080 -v "${IMAGE_FOLDER}:/usr/share/nginx/html" nginxinc/nginx-unprivileged
 
 # We need to gather artifacts/logs before exiting also if there are errors
 set +e
diff --git a/hack/clean-e2e.sh b/hack/clean-e2e.sh
index 75c4a597..fe35202e 100755
--- a/hack/clean-e2e.sh
+++ b/hack/clean-e2e.sh
@@ -6,7 +6,6 @@ cd "${REPO_ROOT}" || exit 1
 minikube delete
 docker rm -f vbmc
 docker rm -f image-server-e2e
-docker rm -f sushy-tools
 virsh -c qemu:///system destroy --domain bmo-e2e-0
 virsh -c qemu:///system undefine --domain bmo-e2e-0 --remove-all-storage
 virsh -c qemu:///system net-destroy baremetal-e2e
diff --git a/pkg/provisioner/ironic/validatemanagementaccess_test.go b/pkg/provisioner/ironic/validatemanagementaccess_test.go
index 819f88ec..bb914877 100644
--- a/pkg/provisioner/ironic/validatemanagementaccess_test.go
+++ b/pkg/provisioner/ironic/validatemanagementaccess_test.go
@@ -330,10 +330,8 @@ func TestValidateManagementAccessExistingSteadyStateNoUpdate(t *testing.T) {
 	imageTypes := []struct {
 		DeployInterface string
 		Image           *metal3api.Image
-		HasCustomDeploy bool
 		InstanceInfo    map[string]interface{}
 		DriverInfo      map[string]interface{}
-		ProvisionState  string
 	}{
 		{
 			DeployInterface: "",
@@ -391,47 +389,6 @@ func TestValidateManagementAccessExistingSteadyStateNoUpdate(t *testing.T) {
 				"test_port":                    "42",
 			},
 		},
-		{
-			DeployInterface: "custom-agent",
-			HasCustomDeploy: true,
-			InstanceInfo: map[string]interface{}{
-				"capabilities": map[string]interface{}{},
-			},
-			DriverInfo: map[string]interface{}{
-				"force_persistent_boot_device": "Default",
-				"deploy_kernel":                "http://deploy.test/ipa.kernel",
-				"deploy_ramdisk":               "http://deploy.test/ipa.initramfs",
-				"test_address":                 "test.bmc",
-				"test_username":                "",
-				"test_password":                "******", // ironic returns a placeholder
-				"test_port":                    "42",
-			},
-		},
-		// NOTE(dtantsur): This is a corner case. If the node is active, do not change any instance information until it's deprovisioned.
-		// Otherwise, clean up may not work correctly (and updates to deploy_interface will be rejected anyway).
-		{
-			ProvisionState: string(nodes.Active),
-			Image: &metal3api.Image{
-				URL:        "theimage",
-				DiskFormat: &liveFormat,
-			},
-			InstanceInfo: map[string]interface{}{
-				"image_source":        "theimage",
-				"image_os_hash_algo":  "md5",
-				"image_os_hash_value": "thechecksum",
-				"image_checksum":      "thechecksum",
-				"capabilities":        map[string]interface{}{},
-			},
-			DriverInfo: map[string]interface{}{
-				"force_persistent_boot_device": "Default",
-				"deploy_kernel":                "http://deploy.test/ipa.kernel",
-				"deploy_ramdisk":               "http://deploy.test/ipa.initramfs",
-				"test_address":                 "test.bmc",
-				"test_username":                "",
-				"test_password":                "******", // ironic returns a placeholder
-				"test_port":                    "42",
-			},
-		},
 	}
 	clean := true
 
@@ -447,14 +404,10 @@ func TestValidateManagementAccessExistingSteadyStateNoUpdate(t *testing.T) {
 				t.Fatal("create callback should not be invoked for existing node")
 			}
 
-			provisionState := imageType.ProvisionState
-			if provisionState == "" {
-				provisionState = string(nodes.Manageable)
-			}
 			ironic := testserver.NewIronic(t).Ready().CreateNodes(createCallback).Node(nodes.Node{
 				Name:            host.Namespace + nameSeparator + host.Name,
 				UUID:            "uuid", // to match status in host
-				ProvisionState:  provisionState,
+				ProvisionState:  string(nodes.Manageable),
 				AutomatedClean:  &clean,
 				InstanceUUID:    string(host.UID),
 				DeployInterface: imageType.DeployInterface,
@@ -472,8 +425,7 @@ func TestValidateManagementAccessExistingSteadyStateNoUpdate(t *testing.T) {
 				t.Fatalf("could not create provisioner: %s", err)
 			}
 
-			data := provisioner.ManagementAccessData{CurrentImage: imageType.Image, HasCustomDeploy: imageType.HasCustomDeploy}
-			result, _, err := prov.ValidateManagementAccess(data, false, false)
+			result, _, err := prov.ValidateManagementAccess(provisioner.ManagementAccessData{CurrentImage: imageType.Image}, false, false)
 			if err != nil {
 				t.Fatalf("error from ValidateManagementAccess: %s", err)
 			}
diff --git a/test/e2e/config/ironic.yaml b/test/e2e/config/ironic.yaml
index 339305c8..a2cb5a62 100644
--- a/test/e2e/config/ironic.yaml
+++ b/test/e2e/config/ironic.yaml
@@ -23,7 +23,7 @@ variables:
   # Test credentials. The tests will create a BMH with these.
   BMC_USER: admin
   BMC_PASSWORD: password
-  BMC_ADDRESS: "ipmi://192.168.222.1:16230"
+  BMC_ADDRESS: ipmi://192.168.222.1:16230
   BOOT_MAC_ADDRESS: "00:60:2f:31:81:01"
   IMAGE_URL: "http://192.168.222.1/cirros-0.6.2-x86_64-disk.img"
   IMAGE_CHECKSUM: "c8fc807773e5354afe61636071771906"
@@ -34,7 +34,7 @@ intervals:
   inspection/wait-registration-error: ["1m", "5s"]
   external-inspection/wait-available: ["20s", "1s"]
   default/wait-inspecting: ["2m", "10s"]
-  default/wait-available: ["10m", "1s"]
+  default/wait-available: ["10m", "10s"]
   default/wait-deployment: ["5m", "1s"]
   default/wait-namespace-deleted: ["10m", "1s"]
   ironic/wait-deployment: ["10m", "2s"]
diff --git a/test/e2e/sushy-tools/sushy-emulator.conf b/test/e2e/sushy-tools/sushy-emulator.conf
deleted file mode 100644
index 8921d438..00000000
--- a/test/e2e/sushy-tools/sushy-emulator.conf
+++ /dev/null
@@ -1,36 +0,0 @@
-# Listen on the local IP address 192.168.222.1
-SUSHY_EMULATOR_LISTEN_IP = u'192.168.222.1'
-
-# Bind to TCP port 8000
-SUSHY_EMULATOR_LISTEN_PORT = 8000
-
-# Serve this SSL certificate to the clients
-SUSHY_EMULATOR_SSL_CERT = None
-
-# If SSL certificate is being served, this is its RSA private key
-SUSHY_EMULATOR_SSL_KEY = None
-
-# The OpenStack cloud ID to use. This option enables OpenStack driver.
-SUSHY_EMULATOR_OS_CLOUD = None
-# The libvirt URI to use. This option enables libvirt driver.
-SUSHY_EMULATOR_LIBVIRT_URI = u'qemu:///system'
-
-# Instruct the libvirt driver to ignore any instructions to
-# set the boot device. Allowing the UEFI firmware to instead
-# rely on the EFI Boot Manager
-# Note: This sets the legacy boot element to dev="fd"
-# and relies on the floppy not existing, it likely wont work
-# your VM has a floppy drive.
-SUSHY_EMULATOR_IGNORE_BOOT_DEVICE = False
-
-# The map of firmware loaders dependant on the boot mode and
-# system architecture. Ideally the x86_64 loader will be capable
-# of secure boot or not based on the chosen nvram.
-SUSHY_EMULATOR_BOOT_LOADER_MAP = {
-    u'UEFI': {
-        u'x86_64': u'/usr/share/OVMF/OVMF_CODE.secboot.fd'
-    },
-    u'Legacy': {
-        u'x86_64': None
-    }
-}