diff --git a/controllers/metal3.io/baremetalhost_controller.go b/controllers/metal3.io/baremetalhost_controller.go index 0dc94e885c..5186ee98f2 100644 --- a/controllers/metal3.io/baremetalhost_controller.go +++ b/controllers/metal3.io/baremetalhost_controller.go @@ -1471,7 +1471,6 @@ func (r *BareMetalHostReconciler) handleDataImageActions(prov provisioner.Provis // TODO(hroyrh) : Should we fail after the error count exceeds a // given constant ? dataImageRetryBackoff := max(dataImageUpdateDelay, calculateBackoff(dataImage.Status.Error.Count)) - info.log.Info("Current dataImage reconcile delay", "dataImageRetryBackoff", dataImageRetryBackoff) // Check if any attach/detach action is pending or failed to attach // We are assuming that the action will have completed by the time @@ -1598,7 +1597,7 @@ func ownerReferenceExists(owner metav1.Object, resource metav1.Object) bool { // Attach the DataImage to the BareMetalHost. func (r *BareMetalHostReconciler) attachDataImage(prov provisioner.Provisioner, info *reconcileInfo, dataImage *metal3api.DataImage) error { if err := prov.AttachDataImage(dataImage.Spec.URL); err != nil { - info.log.Info("Error while attaching DataImage", "DataImage", dataImage.Name, "Error", err.Error()) + info.log.Info("Error while attaching DataImage", "URL", dataImage.Spec.URL, "Error", err.Error()) dataImage.Status.Error.Count++ dataImage.Status.Error.Message = err.Error() @@ -1610,8 +1609,6 @@ func (r *BareMetalHostReconciler) attachDataImage(prov provisioner.Provisioner, return fmt.Errorf("failed to attach dataImage, %w", err) } - info.log.Info("Attach dataImage initiated", "DataImage", dataImage.Name) - // Update attached.URL here, we will mark it dirty in case any node errors // are encountered dataImage.Status.AttachedImage.URL = dataImage.Spec.URL @@ -1638,8 +1635,6 @@ func (r *BareMetalHostReconciler) detachDataImage(prov provisioner.Provisioner, return fmt.Errorf("failed to detach dataImage, %w", err) } - info.log.Info("Detach dataImage initiated", "DataImage", dataImage.Name) - // Update attached.URL here, we will mark it dirty in case any node errors // are encountered dataImage.Status.AttachedImage.URL = "" diff --git a/controllers/metal3.io/dataimage_controller.go b/controllers/metal3.io/dataimage_controller.go index 5861f9578e..5b9b78feb8 100644 --- a/controllers/metal3.io/dataimage_controller.go +++ b/controllers/metal3.io/dataimage_controller.go @@ -93,12 +93,9 @@ func (info *rdiInfo) publishEvent(reason, message string) { // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.14.4/pkg/reconcile func (r *DataImageReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { reqLogger := r.Log.WithValues("dataimage", req.NamespacedName) - reqLogger.Info("start dataImage reconciliation V1") + reqLogger.Info("start dataImage reconciliation") di := &metal3api.DataImage{} if err := r.Get(ctx, req.NamespacedName, di); err != nil { @@ -132,11 +129,22 @@ func (r *DataImageReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // If the reconciliation is paused, requeue annotations := bmh.GetAnnotations() - if annotations != nil { - if _, ok := annotations[metal3api.PausedAnnotation]; ok { - reqLogger.Info("host is paused, no work to do") - return ctrl.Result{Requeue: false}, nil + if _, ok := annotations[metal3api.PausedAnnotation]; ok { + reqLogger.Info("host is paused, no work to do") + return ctrl.Result{Requeue: false}, nil + } + + // Add finalizer for newly created DataImage + if di.DeletionTimestamp.IsZero() && !utils.StringInList(di.Finalizers, metal3api.DataImageFinalizer) { + reqLogger.Info("adding finalizer") + di.Finalizers = append(di.Finalizers, metal3api.DataImageFinalizer) + + // Update dataImage after adding finalizer, requeue in case of failure + err := r.Update(ctx, di) + if err != nil { + return ctrl.Result{RequeueAfter: dataImageUpdateDelay}, fmt.Errorf("failed to update resource after add finalizer, %w", err) } + return ctrl.Result{Requeue: true}, nil } // Create a provisioner that can access Ironic API @@ -157,19 +165,6 @@ func (r *DataImageReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{Requeue: true, RequeueAfter: provisionerRetryDelay}, nil } - // Add finalizer for newly created DataImage - if di.DeletionTimestamp.IsZero() && !utils.StringInList(di.Finalizers, metal3api.DataImageFinalizer) { - reqLogger.Info("adding finalizer") - di.Finalizers = append(di.Finalizers, metal3api.DataImageFinalizer) - - // Update dataImage after adding finalizer, requeue in case of failure - err := r.Update(ctx, di) - if err != nil { - return ctrl.Result{RequeueAfter: dataImageUpdateDelay}, fmt.Errorf("failed to update resource after add finalizer, %w", err) - } - return ctrl.Result{Requeue: true}, nil - } - // Check if any attach/detach action is pending or failed to attach _, nodeError := prov.IsDataImageReady() diff --git a/main.go b/main.go index 31a8cc1d92..db13d88918 100644 --- a/main.go +++ b/main.go @@ -177,7 +177,7 @@ func main() { logOpts.Development = true logOpts.TimeEncoder = zapcore.ISO8601TimeEncoder } else { - logOpts.TimeEncoder = zapcore.ISO8601TimeEncoder // zapcore.EpochTimeEncoder + 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 0e07354ec4..6890e13b4d 100644 --- a/pkg/provisioner/ironic/ironic.go +++ b/pkg/provisioner/ironic/ironic.go @@ -2201,7 +2201,13 @@ func (p *ironicProvisioner) RemoveBMCEventSubscriptionForNode(subscription metal // successful of not. func (p *ironicProvisioner) IsDataImageReady() (isNodeBusy bool, nodeError error) { // TODO(hroyrh) - // Get BareMetalHost VirtualMedia details and handle errors + // Get BareMetalHost VirtualMedia details using a GET api to vmedia + + // Check if Ironic API version supports DataImage API + // Needs version >= 1.89 + if !p.availableFeatures.HasDataImage() { + return true, fmt.Errorf("ironic version=%d doesn't support DataImage API, needs version>=1.89", p.availableFeatures.MaxVersion) + } node, err := p.getNode() if err != nil { @@ -2220,6 +2226,12 @@ func (p *ironicProvisioner) IsDataImageReady() (isNodeBusy bool, nodeError error } func (p *ironicProvisioner) AttachDataImage(url string) (err error) { + // Check if Ironic API version supports DataImage API + // Needs version >= 1.89 + if !p.availableFeatures.HasDataImage() { + return fmt.Errorf("ironic version=%d doesn't support DataImage API, needs version>=1.89", p.availableFeatures.MaxVersion) + } + err = nodes.AttachVirtualMedia(p.ctx, p.client, p.nodeID, nodes.AttachVirtualMediaOpts{ DeviceType: nodes.VirtualMediaCD, ImageURL: url, @@ -2232,6 +2244,12 @@ func (p *ironicProvisioner) AttachDataImage(url string) (err error) { } func (p *ironicProvisioner) DetachDataImage() (err error) { + // Check if Ironic API version supports DataImage API + // Needs version >= 1.89 + if !p.availableFeatures.HasDataImage() { + return fmt.Errorf("ironic version=%d doesn't support DataImage API, needs version>=1.89", p.availableFeatures.MaxVersion) + } + err = nodes.DetachVirtualMedia(p.ctx, p.client, p.nodeID, nodes.DetachVirtualMediaOpts{ DeviceTypes: []nodes.VirtualMediaDeviceType{nodes.VirtualMediaCD}, }).ExtractErr()