Skip to content

provisioning controller: set correct status when platform=baremetal and no CR is found #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
stbenjam wants to merge 1 commit into from
Closed

provisioning controller: set correct status when platform=baremetal and no CR is found #75

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions controllers/provisioning_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,14 @@ func (r *ProvisioningReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error
}
if baremetalConfig == nil {
// Provisioning configuration not available at this time.
// Cannot proceed wtih metal3 deployment.
r.Log.V(1).Info("Provisioning CR not found")
return ctrl.Result{}, nil
// Cannot proceed with metal3 deployment.
err = r.updateCOStatus(ReasonUnsupported, "Provisioning CR not found", "")
if err != nil {
return ctrl.Result{}, fmt.Errorf("unable to put %q ClusterOperator in Disabled state: %v", clusterOperatorName, err)
}

// We're disabled, but requeue in case we get a provisioning CR
return ctrl.Result{}, fmt.Errorf("no provisioning CR found; will remain Disabled until CR available")
Copy link
Member Author

@stbenjam stbenjam Nov 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely clear if this is the right fix; will we hit the Reconcile loop if there's no CR at all? This might need to go to main.

Copy link
Contributor

@asalkeld asalkeld Nov 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return ctrl.Result{}, fmt.Errorf("no provisioning CR found; will remain Disabled until CR available")
return ctrl.Result{}, nil

We are watching the CR so we will get a Reconcile if one is created/updated. this will just cause a lot of unnecessary Reconcile calls

Copy link
Member Author

@stbenjam stbenjam Nov 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we need to check in main; assisted is still failing and looking at the logs it looks like it does start but we're never reconciling which makes sense if there's no CR at all yet.

Copy link
Contributor

@asalkeld asalkeld Nov 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, this will only hit if you delete the cr. We probably need to initialize the status to Unsupported in case the cr never gets created

}

// examine DeletionTimestamp to determine if object is under deletion
Expand Down