diff --git a/pkg/apihelpers/machineosbuild.go b/pkg/apihelpers/machineosbuild.go index 3bb93b727a..4286945fca 100644 --- a/pkg/apihelpers/machineosbuild.go +++ b/pkg/apihelpers/machineosbuild.go @@ -4,6 +4,7 @@ import ( mcfgv1 "github.com/openshift/api/machineconfiguration/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" ) // NewMachineOSBuildCondition creates a new MachineOSBuild condition. @@ -71,7 +72,11 @@ func IsMachineOSBuildConditionTrue(conditions []metav1.Condition, conditionType // IsMachineOSBuildConditionPresentAndEqual returns true when conditionType is present and equal to status. func IsMachineOSBuildConditionPresentAndEqual(conditions []metav1.Condition, conditionType mcfgv1.BuildProgress, status metav1.ConditionStatus) bool { + klog.Errorf("in IsMachineOSBuildConditionPresentAndEqual") for _, condition := range conditions { + klog.Errorf("condition: %v", condition) + klog.Errorf("condition.Type: %v", condition.Type) + klog.Errorf("condition.Status: %v", condition.Status) if mcfgv1.BuildProgress(condition.Type) == conditionType { return condition.Status == status } diff --git a/pkg/controller/build/helpers.go b/pkg/controller/build/helpers.go index c1f3c64e4c..dde39d105d 100644 --- a/pkg/controller/build/helpers.go +++ b/pkg/controller/build/helpers.go @@ -126,11 +126,20 @@ func validateSecret(secretGetter func(string) (*corev1.Secret, error), mosc *mcf // primarily when we transition from the initial status -> transient state -> // terminal state. func isMachineOSBuildStatusUpdateNeeded(oldStatus, curStatus mcfgv1.MachineOSBuildStatus) (bool, string) { + klog.Errorf("In isMachineOSBuildStatusUpdateNeeded") + klog.Errorf("oldStatus %v", oldStatus) + klog.Errorf("curStatus %v", curStatus) oldState := ctrlcommon.NewMachineOSBuildStateFromStatus(oldStatus) curState := ctrlcommon.NewMachineOSBuildStateFromStatus(curStatus) + klog.Errorf("oldState %v", oldState) + klog.Errorf("curState %v", curState) // From having no build conditions to having the initial state set. + klog.Errorf("!oldState.HasBuildConditions() %t", !oldState.HasBuildConditions()) + klog.Errorf("curState.HasBuildConditions() %t", curState.HasBuildConditions()) + klog.Errorf("curState.IsInInitialState() %t", curState.IsInInitialState()) if !oldState.HasBuildConditions() && curState.HasBuildConditions() && curState.IsInInitialState() { + klog.Errorf("In !oldState.HasBuildConditions() && curState.HasBuildConditions() && curState.IsInInitialState()") return true, "in initial state" } @@ -138,12 +147,19 @@ func isMachineOSBuildStatusUpdateNeeded(oldStatus, curStatus mcfgv1.MachineOSBui curTransientState := curState.GetTransientState() // From initial state -> pending or building. + // klog.Errorf("oldState.IsInInitialState() %t", oldState.IsInInitialState()) + // klog.Errorf("curState.IsInTransientState() %t", curState.IsInTransientState()) if oldState.IsInInitialState() && curState.IsInTransientState() { + klog.Errorf("In oldState.IsInInitialState() && curState.IsInTransientState()") return true, fmt.Sprintf("transitioned from initial state -> transient state (%s)", curTransientState) } // From pending -> building, but not building -> pending. + // klog.Errorf("oldState.IsInTransientState() %t", oldState.IsInTransientState()) + // klog.Errorf("curState.IsInTransientState() %t", curState.IsInTransientState()) + // klog.Errorf("oldTransientState != curTransientState %t", oldTransientState != curTransientState) if oldState.IsInTransientState() && curState.IsInTransientState() && oldTransientState != curTransientState { + klog.Errorf("In oldState.IsInTransientState() && curState.IsInTransientState() && oldTransientState != curTransientState") reason := fmt.Sprintf("transitioned from transient state (%s) -> transient state (%s)", oldTransientState, curTransientState) isValid := oldTransientState == mcfgv1.MachineOSBuildPrepared && curTransientState == mcfgv1.MachineOSBuilding return isValid, reason @@ -151,15 +167,23 @@ func isMachineOSBuildStatusUpdateNeeded(oldStatus, curStatus mcfgv1.MachineOSBui oldTerminalState := oldState.GetTerminalState() curTerminalState := curState.GetTerminalState() + klog.Errorf("oldTerminalState 168 %v", oldTerminalState) + klog.Errorf("curTerminalState 168 %v", curTerminalState) // From building -> {success, failure, interrupted} + klog.Errorf("oldState.IsInTransientState() 170 %t", oldState.IsInTransientState()) + klog.Errorf("curState.IsInTerminalState() 171 %t", curState.IsInTerminalState()) if oldState.IsInTransientState() && curState.IsInTerminalState() { + klog.Errorf("In oldState.IsInTransientState() && curState.IsInTerminalState()") return true, fmt.Sprintf("transitioned from transient state (%s) -> terminal state (%s)", oldTransientState, curTerminalState) } // From initial state -> {success, failure, interrupted} // It's rare that this could occur, but better to be explicit that it can occur. + klog.Errorf("oldState.IsInInitialState() %t", oldState.IsInInitialState()) + klog.Errorf("curState.IsInTerminalState() %t", curState.IsInTerminalState()) if oldState.IsInInitialState() && curState.IsInTerminalState() { + klog.Errorf("In oldState.IsInInitialState() && curState.IsInTerminalState()") return true, fmt.Sprintf("transitioned from initial state -> terminal state (%s)", curTerminalState) } @@ -168,19 +192,24 @@ func isMachineOSBuildStatusUpdateNeeded(oldStatus, curStatus mcfgv1.MachineOSBui // From {success, failure, interrupted} -> {success, failure, interrupted} if oldState.IsInTerminalState() && curState.IsInTerminalState() { + klog.Errorf("In oldState.IsInTerminalState() && curState.IsInTerminalState()") return false, fmt.Sprintf("transitioned from terminal state (%s) -> terminal state (%s)", oldTerminalState, curTerminalState) } // From {success, failure, interrupted} -> {pending, running} if oldState.IsInTerminalState() && curState.IsInTransientState() { + klog.Errorf("In oldState.IsInTerminalState() && curState.IsInTransientState()") return false, fmt.Sprintf("transitioned from terminal state (%s) -> transient state (%s)", oldTerminalState, curTransientState) } // From {sucecss, failure, interrupted} -> initial state if oldState.IsInTerminalState() && curState.IsInInitialState() { + klog.Errorf("In oldState.IsInTerminalState() && curState.IsInInitialState()") return false, fmt.Sprintf("transitioned from terminal state (%s) -> initial state", oldTerminalState) } + klog.Errorf("At the end of isMachineOSBuildStatusUpdateNeeded :(") + // Everything else return false, "" } diff --git a/pkg/controller/build/ocl_events.go b/pkg/controller/build/ocl_events.go index a26c2d9fde..b9dd8fcccb 100644 --- a/pkg/controller/build/ocl_events.go +++ b/pkg/controller/build/ocl_events.go @@ -7,6 +7,7 @@ import ( batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" "k8s.io/client-go/tools/record" + "k8s.io/klog/v2" ) // Event types for OCL processes @@ -84,6 +85,7 @@ func (r *OCLEventRecorder) RecordBuildCompleted(mosb *mcfgv1.MachineOSBuild, ima // RecordBuildFailed records when a build fails func (r *OCLEventRecorder) RecordBuildFailed(mosb *mcfgv1.MachineOSBuild) { + klog.Errorf("in RecordBuildFailed") r.recorder.Event(mosb, corev1.EventTypeWarning, EventBuildFailed, fmt.Sprintf("Build failed; see MachineOSBuild %q status conditions for details", mosb.Name)) } diff --git a/pkg/controller/build/reconciler.go b/pkg/controller/build/reconciler.go index dbf150db1a..0dcef3ea3b 100644 --- a/pkg/controller/build/reconciler.go +++ b/pkg/controller/build/reconciler.go @@ -252,37 +252,45 @@ func (b *buildReconciler) AddJob(ctx context.Context, job *batchv1.Job) error { // Executes whenever a build Job is updated func (b *buildReconciler) UpdateJob(ctx context.Context, oldJob, curJob *batchv1.Job) error { + klog.Errorf("in UpdateJob") return b.timeObjectOperation(curJob, updatingVerb, func() error { mosb, err := b.getMachineOSBuildForJob(curJob) if err == nil && mosb != nil { if curJob.Status.Succeeded > 0 && (oldJob.Status.Succeeded == 0) { + klog.Errorf("in curJob.Status.Succeeded > 0 && (oldJob.Status.Succeeded == 0)") b.eventRecorder.RecordJobCompleted(mosb, curJob) } if curJob.Status.Failed > 0 && (oldJob.Status.Failed == 0) { + klog.Errorf("in curJob.Status.Failed > 0 && (oldJob.Status.Failed == 0)") b.eventRecorder.RecordJobFailed(mosb, curJob) } if curJob.Status.Active > 0 && (oldJob.Status.Active == 0) { + klog.Errorf("in curJob.Status.Active > 0 && (oldJob.Status.Active == 0)") b.eventRecorder.RecordJobStarted(mosb, curJob) b.eventRecorder.RecordBuildBuilding(mosb) } mosc, err := utils.GetMachineOSConfigForMachineOSBuild(mosb, b.utilListers()) if err == nil { + klog.Errorf("in err == nil") poolName := mosc.Spec.MachineConfigPool.Name if curJob.Status.Succeeded > 0 && (oldJob.Status.Succeeded == 0) { + klog.Errorf("in curJob.Status.Succeeded > 0 && (oldJob.Status.Succeeded == 0)") RecordBuildJobState(poolName, StateSucceeded) RecordImagePushCompleted(poolName) } if curJob.Status.Failed > 0 && (oldJob.Status.Failed == 0) { + klog.Errorf("in curJob.Status.Failed > 0 && (oldJob.Status.Failed == 0)") RecordBuildJobState(poolName, StateFailed) RecordImagePushFailed(poolName) } if curJob.Status.Failed > oldJob.Status.Failed && curJob.Status.Failed <= constants.JobMaxRetries { + klog.Errorf("in curJob.Status.Failed > oldJob.Status.Failed && curJob.Status.Failed <= constants.JobMaxRetries") RecordBuildRetry(poolName) } } @@ -614,7 +622,6 @@ func (b *buildReconciler) startBuild(ctx context.Context, mosb *mcfgv1.MachineOS // Retrieves a deep-copy of the MachineOSConfig from the lister so that the cache is not mutated during the update. func (b *buildReconciler) getMachineOSConfigForUpdate(mosc *mcfgv1.MachineOSConfig) (*mcfgv1.MachineOSConfig, error) { out, err := b.machineOSConfigLister.Get(mosc.Name) - if err != nil { return nil, err } @@ -635,7 +642,6 @@ func (b *buildReconciler) getMachineOSBuildForJob(job *batchv1.Job) (*mcfgv1.Mac // Retrieves a deep-copy of the MachineOSBuild from the lister so that the cache is not mutated during the update. func (b *buildReconciler) getMachineOSBuildForUpdate(mosb *mcfgv1.MachineOSBuild) (*mcfgv1.MachineOSBuild, error) { out, err := b.machineOSBuildLister.Get(mosb.Name) - if err != nil { return nil, err } @@ -730,7 +736,6 @@ func (b *buildReconciler) createNewMachineOSBuildOrReuseExisting(ctx context.Con MachineOSConfig: mosc, MachineConfigPool: mcp, }) - if err != nil { return fmt.Errorf("could not instantiate new MachineOSBuild: %w", err) } @@ -861,7 +866,9 @@ func (b *buildReconciler) getMachineOSBuildStatusForBuilder(ctx context.Context, // the decision off to setStatusOnMachineOSBuildIfNeeded. func (b *buildReconciler) updateMachineOSBuildWithStatusIfNeeded(ctx context.Context, oldBuilder, curBuilder metav1.Object) error { oldStatus, _, err := b.getMachineOSBuildStatusForBuilder(ctx, oldBuilder) + klog.Errorf("oldStatus: %v", oldStatus) if err != nil { + klog.Errorf("got err for getMachineOSBuildStatusForBuilder for old") // If we can't find the MachineOSConfig, MachineOSBuild, or any of the // ephemeral build objects, it means that it was probably deleted. Instead // of trying to reconcile the status, we'll return nil here to avoid @@ -870,7 +877,9 @@ func (b *buildReconciler) updateMachineOSBuildWithStatusIfNeeded(ctx context.Con } curStatus, mosb, err := b.getMachineOSBuildStatusForBuilder(ctx, curBuilder) + klog.Errorf("curStatus: %v", curStatus) if err != nil { + klog.Errorf("got err for getMachineOSBuildStatusForBuilder for cur") // If we can't find the MachineOSConfig, MachineOSBuild, or any of the // ephemeral build objects, it means that it was probably deleted. Instead // of trying to reconcile the status, we'll return nil here to avoid @@ -909,9 +918,11 @@ func (b *buildReconciler) updateMachineOSBuildWithStatusIfNeeded(ctx context.Con // Sets the status on the MachineOSBuild object after comparing the statuses according to very specific state transitions. func (b *buildReconciler) setStatusOnMachineOSBuildIfNeeded(ctx context.Context, mosb *mcfgv1.MachineOSBuild, oldStatus, curStatus mcfgv1.MachineOSBuildStatus) error { + klog.Errorf("in setStatusOnMachineOSBuildIfNeeded") // Compare the old status and the current status to determine if an update is // needed. This is handled according to very specific state transitions. isUpdateNeeded, reason := isMachineOSBuildStatusUpdateNeeded(oldStatus, curStatus) + klog.Errorf("isUpdateNeeded: %v", isUpdateNeeded) if !isUpdateNeeded { if reason != "" { klog.Infof("MachineOSBuild %q %s; skipping update because of invalid transition", mosb.Name, reason) @@ -1212,7 +1223,6 @@ func (b *buildReconciler) syncAll(ctx context.Context) error { return nil }) - if err != nil { return fmt.Errorf("could not sync all: %w", err) } @@ -1236,7 +1246,6 @@ func (b *buildReconciler) syncMachineOSBuilds(ctx context.Context) error { return nil }) - if err != nil { return fmt.Errorf("could not sync MachineOSBuilds: %w", err) } @@ -1249,7 +1258,6 @@ func (b *buildReconciler) syncMachineOSBuilds(ctx context.Context) error { // builder associated with it that one should be created. func (b *buildReconciler) syncMachineOSBuild(ctx context.Context, mosb *mcfgv1.MachineOSBuild) error { return b.timeObjectOperation(mosb, syncingVerb, func() error { - // It could be the case that the MCP the mosb in queue was targeting no longer is valid mcp, err := b.machineConfigPoolLister.Get(mosb.ObjectMeta.Labels[constants.TargetMachineConfigPoolLabelKey]) if err != nil { @@ -1392,7 +1400,6 @@ func (b *buildReconciler) syncMachineOSConfigs(ctx context.Context) error { return nil }) - if err != nil { return fmt.Errorf("could not sync MachineOSConfigs: %w", err) } @@ -1476,7 +1483,6 @@ func (b *buildReconciler) syncMachineConfigPools(ctx context.Context) error { return nil }) - if err != nil { return fmt.Errorf("could not sync MachineConfigPools: %w", err) } @@ -1612,7 +1618,6 @@ func (b *buildReconciler) reconcilePoolChange(ctx context.Context, mcp *mcfgv1.M return b.reuseImageForNewMOSB(ctx, mosc, oldMOSB) } return b.createNewMachineOSBuildOrReuseExisting(ctx, mosc, needsImageRebuild) - } // reuseImageForNewMOSB creates a new MOSB (for the new rendered-MC name) @@ -1637,7 +1642,6 @@ func (b *buildReconciler) reuseImageForNewMOSB(ctx context.Context, mosc *mcfgv1 MachineOSConfig: mosc, MachineConfigPool: mcp, }) - if err != nil { return err } @@ -1805,7 +1809,6 @@ func (b *buildReconciler) shouldPreventBuildDueToDegradation(mcp *mcfgv1.Machine // reconcileImageRebuild calls RequiresRebuild to see if an MC changes the kernel args, ext, or osimageurl. // if it does, we build a new image in our new MOSB func (b *buildReconciler) reconcileImageRebuild(oldMCP, curMCP *mcfgv1.MachineConfigPool) (bool, error) { - curr, err := b.machineConfigLister.Get(oldMCP.Spec.Configuration.Name) if err != nil { return false, err @@ -1982,7 +1985,6 @@ func (b *buildReconciler) seedMachineOSConfigWithExistingImage(ctx context.Conte MachineConfigPool: mcp, MachineOSConfig: mosc, }) - if err != nil { return fmt.Errorf("could not generate MachineOSBuild template for MachineOSConfig %q: %w", mosc.Name, err) } diff --git a/pkg/controller/common/mos_state.go b/pkg/controller/common/mos_state.go index 474a563bb6..8bfc385b1e 100644 --- a/pkg/controller/common/mos_state.go +++ b/pkg/controller/common/mos_state.go @@ -4,6 +4,7 @@ import ( mcfgv1 "github.com/openshift/api/machineconfiguration/v1" "github.com/openshift/machine-config-operator/pkg/apihelpers" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" ) // This is intended to provide a singular way to interrogate MachineConfigPool @@ -115,7 +116,9 @@ func (b *MachineOSBuildState) IsInInitialState() bool { // Determines if an OS image build is in its terminal state where build success, build failure, or build interrupted condition is set. func (b *MachineOSBuildState) IsInTerminalState() bool { - return b.GetTerminalState() != "" + test := b.GetTerminalState() + klog.Errorf("test: %v", test) + return test != "" } // Determines if an OS image build is in a transient state where it is either prepared, pending, or running. @@ -137,6 +140,7 @@ func (b *MachineOSBuildState) GetTransientState() mcfgv1.BuildProgress { // Gets the current terminal state, if any is set. Otherwise, returns an empty string. func (b *MachineOSBuildState) GetTerminalState() mcfgv1.BuildProgress { for terminalState := range MachineOSBuildTerminalStates() { + klog.Errorf("terminalState: %s", terminalState) if apihelpers.IsMachineOSBuildConditionTrue(b.Build.Status.Conditions, terminalState) { return terminalState } @@ -165,11 +169,13 @@ func (b *MachineOSBuildState) SetBuildConditions(conditions []metav1.Condition) for _, condition := range conditions { condition := condition currentCondition := apihelpers.GetMachineOSBuildCondition(b.Build.Status, mcfgv1.BuildProgress(condition.Type)) + klog.Errorf("SetBuildConditions currentCondition %v", currentCondition) if currentCondition != nil && isConditionEqual(*currentCondition, condition) { continue } mosbCondition := apihelpers.NewMachineOSBuildCondition(condition.Type, condition.Status, condition.Reason, condition.Message) + klog.Errorf("SetBuildConditions mosbCondition %v", mosbCondition) apihelpers.SetMachineOSBuildCondition(&b.Build.Status, *mosbCondition) } } @@ -262,7 +268,6 @@ func HasBuildObjectForCurrentMachineConfig(pool *mcfgv1.MachineConfigPool, mosb // Determines if we should do a build based upon the state of our // MachineConfigPool, the presence of a build pod, etc. func BuildDueToPoolChange(oldPool, curPool *mcfgv1.MachineConfigPool, moscNew *mcfgv1.MachineOSConfig, mosbNew *mcfgv1.MachineOSBuild) bool { - moscState := NewMachineOSConfigState(moscNew) mosbState := NewMachineOSBuildState(mosbNew) @@ -273,7 +278,6 @@ func BuildDueToPoolChange(oldPool, curPool *mcfgv1.MachineConfigPool, moscNew *m (IsPoolConfigChange(oldPool, curPool) || !moscState.HasOSImage()) return poolStateSuggestsBuild - } // Checks our pool to see if we can do a build. We base this off of a few criteria: