diff --git a/pkg/apihelpers/machineosbuild.go b/pkg/apihelpers/machineosbuild.go index db600300b0..3bb93b727a 100644 --- a/pkg/apihelpers/machineosbuild.go +++ b/pkg/apihelpers/machineosbuild.go @@ -78,3 +78,219 @@ func IsMachineOSBuildConditionPresentAndEqual(conditions []metav1.Condition, con } return false } + +// Represents the successful conditions for a MachineOSBuild. +func MachineOSBuildSucceededConditions() []metav1.Condition { + return []metav1.Condition{ + { + Type: string(mcfgv1.MachineOSBuildPrepared), + Status: metav1.ConditionFalse, + Reason: "Prepared", + Message: "Build Prepared and Pending", + }, + { + Type: string(mcfgv1.MachineOSBuilding), + Status: metav1.ConditionFalse, + Reason: "Building", + Message: "Image Build In Progress", + }, + { + Type: string(mcfgv1.MachineOSBuildFailed), + Status: metav1.ConditionFalse, + Reason: "Failed", + Message: "Build Failed", + }, + { + Type: string(mcfgv1.MachineOSBuildInterrupted), + Status: metav1.ConditionFalse, + Reason: "Interrupted", + Message: "Build Interrupted", + }, + { + Type: string(mcfgv1.MachineOSBuildSucceeded), + Status: metav1.ConditionTrue, + Reason: "Ready", + Message: "Build Ready", + }, + } +} + +// Represents the pending conditions for a MachineOSBuild. +func MachineOSBuildPendingConditions() []metav1.Condition { + return []metav1.Condition{ + { + Type: string(mcfgv1.MachineOSBuildPrepared), + Status: metav1.ConditionTrue, + Reason: "Prepared", + Message: "Build Prepared and Pending", + }, + { + Type: string(mcfgv1.MachineOSBuilding), + Status: metav1.ConditionFalse, + Reason: "Building", + Message: "Image Build In Progress", + }, + { + Type: string(mcfgv1.MachineOSBuildFailed), + Status: metav1.ConditionFalse, + Reason: "Failed", + Message: "Build Failed", + }, + { + Type: string(mcfgv1.MachineOSBuildInterrupted), + Status: metav1.ConditionFalse, + Reason: "Interrupted", + Message: "Build Interrupted", + }, + { + Type: string(mcfgv1.MachineOSBuildSucceeded), + Status: metav1.ConditionFalse, + Reason: "Ready", + Message: "Build Ready", + }, + } +} + +// Represents the running conditions for a MachineOSBuild. +func MachineOSBuildRunningConditions() []metav1.Condition { + return []metav1.Condition{ + { + Type: string(mcfgv1.MachineOSBuildPrepared), + Status: metav1.ConditionFalse, + Reason: "Prepared", + Message: "Build Prepared and Pending", + }, + { + Type: string(mcfgv1.MachineOSBuilding), + Status: metav1.ConditionTrue, + Reason: "Building", + Message: "Image Build In Progress", + }, + { + Type: string(mcfgv1.MachineOSBuildFailed), + Status: metav1.ConditionFalse, + Reason: "Failed", + Message: "Build Failed", + }, + { + Type: string(mcfgv1.MachineOSBuildInterrupted), + Status: metav1.ConditionFalse, + Reason: "Interrupted", + Message: "Build Interrupted", + }, + { + Type: string(mcfgv1.MachineOSBuildSucceeded), + Status: metav1.ConditionFalse, + Reason: "Ready", + Message: "Build Ready", + }, + } +} + +// Represents the failure conditions for a MachineOSBuild. +func MachineOSBuildFailedConditions() []metav1.Condition { + return []metav1.Condition{ + { + Type: string(mcfgv1.MachineOSBuildPrepared), + Status: metav1.ConditionFalse, + Reason: "Prepared", + Message: "Build Prepared and Pending", + }, + { + Type: string(mcfgv1.MachineOSBuilding), + Status: metav1.ConditionFalse, + Reason: "Building", + Message: "Image Build In Progress", + }, + { + Type: string(mcfgv1.MachineOSBuildFailed), + Status: metav1.ConditionTrue, + Reason: "Failed", + Message: "Build Failed", + }, + { + Type: string(mcfgv1.MachineOSBuildInterrupted), + Status: metav1.ConditionFalse, + Reason: "Interrupted", + Message: "Build Interrupted", + }, + { + Type: string(mcfgv1.MachineOSBuildSucceeded), + Status: metav1.ConditionFalse, + Reason: "Ready", + Message: "Build Ready", + }, + } +} + +// Represents the interrupted conditions for a MachineOSBuild. +func MachineOSBuildInterruptedConditions() []metav1.Condition { + return []metav1.Condition{ + { + Type: string(mcfgv1.MachineOSBuildPrepared), + Status: metav1.ConditionFalse, + Reason: "Prepared", + Message: "Build Prepared and Pending", + }, + { + Type: string(mcfgv1.MachineOSBuilding), + Status: metav1.ConditionFalse, + Reason: "Building", + Message: "Image Build In Progress", + }, + { + Type: string(mcfgv1.MachineOSBuildFailed), + Status: metav1.ConditionFalse, + Reason: "Failed", + Message: "Build Failed", + }, + { + Type: string(mcfgv1.MachineOSBuildInterrupted), + Status: metav1.ConditionTrue, + Reason: "Interrupted", + Message: "Build Interrupted", + }, + { + Type: string(mcfgv1.MachineOSBuildSucceeded), + Status: metav1.ConditionFalse, + Reason: "Ready", + Message: "Build Ready", + }, + } +} + +// Represents the initial MachineOSBuild state (all conditions false). +func MachineOSBuildInitialConditions() []metav1.Condition { + return []metav1.Condition{ + { + Type: string(mcfgv1.MachineOSBuildPrepared), + Status: metav1.ConditionFalse, + Reason: "Prepared", + Message: "Build Prepared and Pending", + }, + { + Type: string(mcfgv1.MachineOSBuilding), + Status: metav1.ConditionFalse, + Reason: "Building", + Message: "Image Build In Progress", + }, + { + Type: string(mcfgv1.MachineOSBuildFailed), + Status: metav1.ConditionFalse, + Reason: "Failed", + Message: "Build Failed", + }, + { + Type: string(mcfgv1.MachineOSBuildInterrupted), + Status: metav1.ConditionFalse, + Reason: "Interrupted", + Message: "Build Interrupted", + }, + { + Type: string(mcfgv1.MachineOSBuildSucceeded), + Status: metav1.ConditionFalse, + Reason: "Ready", + Message: "Build Ready", + }, + } +} diff --git a/pkg/controller/build/helpers.go b/pkg/controller/build/helpers.go index bae9535cb7..4ee1ea8ee5 100644 --- a/pkg/controller/build/helpers.go +++ b/pkg/controller/build/helpers.go @@ -141,9 +141,11 @@ func isMachineOSBuildStatusUpdateNeeded(oldStatus, curStatus mcfgv1.MachineOSBui return true, fmt.Sprintf("transitioned from initial state -> transient state (%s)", curTransientState) } - // From pending -> building. + // From pending -> building, but not building -> pending. if oldState.IsInTransientState() && curState.IsInTransientState() && oldTransientState != curTransientState { - return true, fmt.Sprintf("transitioned from transient state (%s) -> transient state (%s)", 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 } oldTerminalState := oldState.GetTerminalState() diff --git a/pkg/controller/build/helpers_test.go b/pkg/controller/build/helpers_test.go index 7251ec1efa..7a51e7c75c 100644 --- a/pkg/controller/build/helpers_test.go +++ b/pkg/controller/build/helpers_test.go @@ -2,9 +2,11 @@ package build import ( "context" + "fmt" "testing" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/machine-config-operator/pkg/apihelpers" "github.com/openshift/machine-config-operator/pkg/controller/build/fixtures" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" "github.com/stretchr/testify/assert" @@ -80,3 +82,119 @@ func TestValidateOnClusterBuildConfig(t *testing.T) { }) } } + +// This test validates that we have correctly identified if the MachineOSBuild +// should be updated based upon comparing the old and current status of the +// MachineOSBuild. It is worth noting that the current MachineOSBuild status +// can come from the imagebuilder.MachineOSBuildStatus() method which maps the +// current job state to the MachineOSBuild state. +func TestIsMachineOSBuildStatusUpdateNeeded(t *testing.T) { + t.Parallel() + + initialConditions := func() map[mcfgv1.BuildProgress][]metav1.Condition { + return map[mcfgv1.BuildProgress][]metav1.Condition{ + // This value is not part of the OCL API and is here solely for testing purposes. + "Initial": apihelpers.MachineOSBuildInitialConditions(), + } + } + + testCases := []struct { + name string + old map[mcfgv1.BuildProgress][]metav1.Condition + current map[mcfgv1.BuildProgress][]metav1.Condition + expected bool + }{ + // These are valid state transitions. In other words, when one of these + // state transitions is identified, the MachineOSBuild status object should + // be updated. + { + name: "Initial -> Terminal", + old: initialConditions(), + current: ctrlcommon.MachineOSBuildTerminalStates(), + expected: true, + }, + { + name: "Initial -> Transient", + old: initialConditions(), + current: ctrlcommon.MachineOSBuildTransientStates(), + expected: true, + }, + { + name: "Transient -> Terminal", + old: ctrlcommon.MachineOSBuildTransientStates(), + current: ctrlcommon.MachineOSBuildTerminalStates(), + expected: true, + }, + { + name: "Pending -> Running", + old: map[mcfgv1.BuildProgress][]metav1.Condition{ + mcfgv1.MachineOSBuildPrepared: ctrlcommon.MachineOSBuildTransientStates()[mcfgv1.MachineOSBuildPrepared], + }, + current: map[mcfgv1.BuildProgress][]metav1.Condition{ + mcfgv1.MachineOSBuilding: ctrlcommon.MachineOSBuildTransientStates()[mcfgv1.MachineOSBuilding], + }, + expected: true, + }, + // These are invalid state transitions. In other words, when one of these + // state transitions is observed, the MachineOSBuild object should not be + // updated because they are invalid and make no sense. + { + name: "Terminal -> Initial", + old: ctrlcommon.MachineOSBuildTerminalStates(), + current: initialConditions(), + expected: false, + }, + { + name: "Transient -> Initial", + old: ctrlcommon.MachineOSBuildTransientStates(), + current: initialConditions(), + expected: false, + }, + { + name: "Initial -> Initial", + old: initialConditions(), + current: initialConditions(), + expected: false, + }, + { + name: "Terminal -> Terminal", + old: ctrlcommon.MachineOSBuildTerminalStates(), + current: ctrlcommon.MachineOSBuildTerminalStates(), + expected: false, + }, + { + name: "Running -> Pending", + old: map[mcfgv1.BuildProgress][]metav1.Condition{ + mcfgv1.MachineOSBuilding: ctrlcommon.MachineOSBuildTransientStates()[mcfgv1.MachineOSBuilding], + }, + current: map[mcfgv1.BuildProgress][]metav1.Condition{ + mcfgv1.MachineOSBuildPrepared: ctrlcommon.MachineOSBuildTransientStates()[mcfgv1.MachineOSBuildPrepared], + }, + expected: false, + }, + } + + for _, testCase := range testCases { + for oldName, old := range testCase.old { + for currentName, current := range testCase.current { + t.Run(fmt.Sprintf("%s: %s -> %s", testCase.name, oldName, currentName), func(t *testing.T) { + oldStatus := mcfgv1.MachineOSBuildStatus{ + Conditions: old, + } + + curStatus := mcfgv1.MachineOSBuildStatus{ + Conditions: current, + } + + result, reason := isMachineOSBuildStatusUpdateNeeded(oldStatus, curStatus) + + if testCase.expected { + assert.True(t, result, reason) + } else { + assert.False(t, result, reason) + } + }) + } + } + } +} diff --git a/pkg/controller/build/imagebuilder/base.go b/pkg/controller/build/imagebuilder/base.go index c30471d5c1..7b9941af50 100644 --- a/pkg/controller/build/imagebuilder/base.go +++ b/pkg/controller/build/imagebuilder/base.go @@ -53,222 +53,6 @@ func newBaseImageBuilderWithCleaner(kubeclient clientset.Interface, mcfgclient m } } -// Represents the successful conditions for a MachineOSBuild. -func (b *baseImageBuilder) succeededConditions() []metav1.Condition { - return []metav1.Condition{ - { - Type: string(mcfgv1.MachineOSBuildPrepared), - Status: metav1.ConditionFalse, - Reason: "Prepared", - Message: "Build Prepared and Pending", - }, - { - Type: string(mcfgv1.MachineOSBuilding), - Status: metav1.ConditionFalse, - Reason: "Building", - Message: "Image Build In Progress", - }, - { - Type: string(mcfgv1.MachineOSBuildFailed), - Status: metav1.ConditionFalse, - Reason: "Failed", - Message: "Build Failed", - }, - { - Type: string(mcfgv1.MachineOSBuildInterrupted), - Status: metav1.ConditionFalse, - Reason: "Interrupted", - Message: "Build Interrupted", - }, - { - Type: string(mcfgv1.MachineOSBuildSucceeded), - Status: metav1.ConditionTrue, - Reason: "Ready", - Message: "Build Ready", - }, - } -} - -// Represents the pending conditions for a MachineOSBuild. -func (b *baseImageBuilder) pendingConditions() []metav1.Condition { - return []metav1.Condition{ - { - Type: string(mcfgv1.MachineOSBuildPrepared), - Status: metav1.ConditionTrue, - Reason: "Prepared", - Message: "Build Prepared and Pending", - }, - { - Type: string(mcfgv1.MachineOSBuilding), - Status: metav1.ConditionFalse, - Reason: "Building", - Message: "Image Build In Progress", - }, - { - Type: string(mcfgv1.MachineOSBuildFailed), - Status: metav1.ConditionFalse, - Reason: "Failed", - Message: "Build Failed", - }, - { - Type: string(mcfgv1.MachineOSBuildInterrupted), - Status: metav1.ConditionFalse, - Reason: "Interrupted", - Message: "Build Interrupted", - }, - { - Type: string(mcfgv1.MachineOSBuildSucceeded), - Status: metav1.ConditionFalse, - Reason: "Ready", - Message: "Build Ready", - }, - } -} - -// Represents the running conditions for a MachineOSBuild. -func (b *baseImageBuilder) runningConditions() []metav1.Condition { - return []metav1.Condition{ - { - Type: string(mcfgv1.MachineOSBuildPrepared), - Status: metav1.ConditionFalse, - Reason: "Prepared", - Message: "Build Prepared and Pending", - }, - { - Type: string(mcfgv1.MachineOSBuilding), - Status: metav1.ConditionTrue, - Reason: "Building", - Message: "Image Build In Progress", - }, - { - Type: string(mcfgv1.MachineOSBuildFailed), - Status: metav1.ConditionFalse, - Reason: "Failed", - Message: "Build Failed", - }, - { - Type: string(mcfgv1.MachineOSBuildInterrupted), - Status: metav1.ConditionFalse, - Reason: "Interrupted", - Message: "Build Interrupted", - }, - { - Type: string(mcfgv1.MachineOSBuildSucceeded), - Status: metav1.ConditionFalse, - Reason: "Ready", - Message: "Build Ready", - }, - } -} - -// Represents the failure conditions for a MachineOSBuild. -func (b *baseImageBuilder) failedConditions() []metav1.Condition { - return []metav1.Condition{ - { - Type: string(mcfgv1.MachineOSBuildPrepared), - Status: metav1.ConditionFalse, - Reason: "Prepared", - Message: "Build Prepared and Pending", - }, - { - Type: string(mcfgv1.MachineOSBuilding), - Status: metav1.ConditionFalse, - Reason: "Building", - Message: "Image Build In Progress", - }, - { - Type: string(mcfgv1.MachineOSBuildFailed), - Status: metav1.ConditionTrue, - Reason: "Failed", - Message: "Build Failed", - }, - { - Type: string(mcfgv1.MachineOSBuildInterrupted), - Status: metav1.ConditionFalse, - Reason: "Interrupted", - Message: "Build Interrupted", - }, - { - Type: string(mcfgv1.MachineOSBuildSucceeded), - Status: metav1.ConditionFalse, - Reason: "Ready", - Message: "Build Ready", - }, - } -} - -// Represents the interrupted conditions for a MachineOSBuild. -func (b *baseImageBuilder) interruptedConditions() []metav1.Condition { - return []metav1.Condition{ - { - Type: string(mcfgv1.MachineOSBuildPrepared), - Status: metav1.ConditionFalse, - Reason: "Prepared", - Message: "Build Prepared and Pending", - }, - { - Type: string(mcfgv1.MachineOSBuilding), - Status: metav1.ConditionFalse, - Reason: "Building", - Message: "Image Build In Progress", - }, - { - Type: string(mcfgv1.MachineOSBuildFailed), - Status: metav1.ConditionFalse, - Reason: "Failed", - Message: "Build Failed", - }, - { - Type: string(mcfgv1.MachineOSBuildInterrupted), - Status: metav1.ConditionTrue, - Reason: "Interrupted", - Message: "Build Interrupted", - }, - { - Type: string(mcfgv1.MachineOSBuildSucceeded), - Status: metav1.ConditionFalse, - Reason: "Ready", - Message: "Build Ready", - }, - } -} - -// Represents the initial MachineOSBuild state (all conditions false). -func (b *baseImageBuilder) initialConditions() []metav1.Condition { - return []metav1.Condition{ - { - Type: string(mcfgv1.MachineOSBuildPrepared), - Status: metav1.ConditionFalse, - Reason: "Prepared", - Message: "Build Prepared and Pending", - }, - { - Type: string(mcfgv1.MachineOSBuilding), - Status: metav1.ConditionFalse, - Reason: "Building", - Message: "Image Build In Progress", - }, - { - Type: string(mcfgv1.MachineOSBuildFailed), - Status: metav1.ConditionFalse, - Reason: "Failed", - Message: "Build Failed", - }, - { - Type: string(mcfgv1.MachineOSBuildInterrupted), - Status: metav1.ConditionFalse, - Reason: "Interrupted", - Message: "Build Interrupted", - }, - { - Type: string(mcfgv1.MachineOSBuildSucceeded), - Status: metav1.ConditionFalse, - Reason: "Ready", - Message: "Build Ready", - }, - } -} - // Represents a builder object that has a GroupVersionKind method on it; which // anything that has metav1.TypeMeta instance included should have.. type kubeObject interface { @@ -290,6 +74,16 @@ func (b *baseImageBuilder) getMachineOSBuildStatus(ctx context.Context, obj kube out.BuildEnd = &now } + // In this scenario, the build is in a terminal state, but we don't know + // when it started since the machine-os-builder pod may have been offline. + // In this case, we should get the creation timestamp from the builder + // object and use that as the start time instead of now since the buildEnd + // must be after the buildStart time. + if out.BuildStart == &now && out.BuildEnd == &now { + jobCreationTimestamp := obj.GetCreationTimestamp() + out.BuildStart = &jobCreationTimestamp + } + if buildStatus == mcfgv1.MachineOSBuildSucceeded { pullspec, err := b.getFinalImagePullspec(ctx) if err != nil { diff --git a/pkg/controller/build/imagebuilder/base_test.go b/pkg/controller/build/imagebuilder/base_test.go deleted file mode 100644 index b0ff14e9bc..0000000000 --- a/pkg/controller/build/imagebuilder/base_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package imagebuilder - -import ( - "context" - "testing" - "time" - - "github.com/openshift/machine-config-operator/pkg/controller/build/fixtures" - ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" - "github.com/stretchr/testify/assert" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func TestBase(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) - t.Cleanup(cancel) - - kubeclient, mcfgclient, lobj, kubeassert := fixtures.GetClientsForTest(t) - kubeassert = kubeassert.WithContext(ctx) - - base := newBaseImageBuilder(kubeclient, mcfgclient, lobj.MachineOSBuild, lobj.MachineOSConfig, nil) - - terminalConditions := [][]metav1.Condition{ - base.succeededConditions(), - base.failedConditions(), - base.interruptedConditions(), - } - - for _, terminalCondition := range terminalConditions { - mosb := lobj.MachineOSBuild.DeepCopy() - mosb.Status.Conditions = terminalCondition - mosbState := ctrlcommon.NewMachineOSBuildState(mosb) - assert.True(t, mosbState.IsInTerminalState()) - assert.False(t, mosbState.IsInTransientState()) - assert.False(t, mosbState.IsInInitialState()) - } - - transientConditions := [][]metav1.Condition{ - base.pendingConditions(), - base.runningConditions(), - } - - for _, transientCondition := range transientConditions { - mosb := lobj.MachineOSBuild.DeepCopy() - mosb.Status.Conditions = transientCondition - mosbState := ctrlcommon.NewMachineOSBuildState(mosb) - assert.True(t, mosbState.IsInTransientState()) - assert.False(t, mosbState.IsInInitialState()) - assert.False(t, mosbState.IsInTerminalState()) - } - - mosb := lobj.MachineOSBuild.DeepCopy() - mosb.Status.Conditions = base.initialConditions() - mosbState := ctrlcommon.NewMachineOSBuildState(mosb) - assert.False(t, mosbState.IsInTransientState()) - assert.True(t, mosbState.IsInInitialState()) - assert.False(t, mosbState.IsInTerminalState()) -} diff --git a/pkg/controller/build/imagebuilder/jobimagebuilder.go b/pkg/controller/build/imagebuilder/jobimagebuilder.go index ee172ea354..c4d2f32b76 100644 --- a/pkg/controller/build/imagebuilder/jobimagebuilder.go +++ b/pkg/controller/build/imagebuilder/jobimagebuilder.go @@ -7,6 +7,7 @@ import ( mcfgv1 "github.com/openshift/api/machineconfiguration/v1" mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" + "github.com/openshift/machine-config-operator/pkg/apihelpers" "github.com/openshift/machine-config-operator/pkg/controller/build/buildrequest" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" batchv1 "k8s.io/api/batch/v1" @@ -221,26 +222,26 @@ func (j *jobImageBuilder) mapJobStatusToBuildStatus(job *batchv1.Job) (mcfgv1.Bu // If the job is being deleted and it was not in either a successful or failed state // then the MachineOSBuild should be considered "interrupted" if job.DeletionTimestamp != nil && job.Status.Succeeded == 0 && job.Status.Failed == 0 { - return mcfgv1.MachineOSBuildInterrupted, j.interruptedConditions() + return mcfgv1.MachineOSBuildInterrupted, apihelpers.MachineOSBuildInterruptedConditions() } if job.Status.Active == 0 && job.Status.Succeeded == 0 && job.Status.Failed == 0 && job.Status.UncountedTerminatedPods == nil { - return mcfgv1.MachineOSBuildPrepared, j.pendingConditions() + return mcfgv1.MachineOSBuildPrepared, apihelpers.MachineOSBuildPendingConditions() } // The build job is still running till it succeeds or maxes out it retries on failures if job.Status.Active >= 0 && job.Status.Failed >= 0 && job.Status.Failed < 4 && job.Status.Succeeded == 0 { - return mcfgv1.MachineOSBuilding, j.runningConditions() + return mcfgv1.MachineOSBuilding, apihelpers.MachineOSBuildRunningConditions() } if job.Status.Succeeded > 0 { - return mcfgv1.MachineOSBuildSucceeded, j.succeededConditions() + return mcfgv1.MachineOSBuildSucceeded, apihelpers.MachineOSBuildSucceededConditions() } // Only return failed if there have been 4 pod failures as the backoffLimit is set to 3 if job.Status.Failed > 3 { - return mcfgv1.MachineOSBuildFailed, j.failedConditions() + return mcfgv1.MachineOSBuildFailed, apihelpers.MachineOSBuildFailedConditions() } - return "", j.initialConditions() + return "", apihelpers.MachineOSBuildInitialConditions() } // Stops the running build by deleting the build job. diff --git a/pkg/controller/build/imagebuilder/jobimagebuilder_test.go b/pkg/controller/build/imagebuilder/jobimagebuilder_test.go index 6c91050245..e9d17d4cf3 100644 --- a/pkg/controller/build/imagebuilder/jobimagebuilder_test.go +++ b/pkg/controller/build/imagebuilder/jobimagebuilder_test.go @@ -211,6 +211,8 @@ func assertObserverCanGetJobStatus(ctx context.Context, t *testing.T, obs ImageB assert.NotNil(t, mosbStatus.Builder) + assert.NotNil(t, mosbStatus.BuildStart) + if jobPhase == jobSucceeded { assert.NotNil(t, mosbStatus.BuildEnd) assert.Equal(t, "registry.hostname.com/org/repo@sha256:e1992921cba73d9e74e46142eca5946df8a895bfd4419fc8b5c6422d5e7192e6", string(mosbStatus.DigestedImagePushSpec)) @@ -243,3 +245,39 @@ func TestJobImageBuilderCanCleanWithOnlyMachineOSBuild(t *testing.T) { kubeassert.JobDoesNotExist(buildJobName) assertObjectsAreRemovedByCleaner(ctx, t, kubeassert, jim.(*jobImageBuilder).buildrequest) } + +func TestJobImageBuilderSetsBuildStartAndEndTimestamp(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) + t.Cleanup(cancel) + + kubeclient, mcfgclient, lobj, kubeassert := fixtures.GetClientsForTest(t) + kubeassert = kubeassert.WithContext(ctx) + + jim := NewJobImageBuilder(kubeclient, mcfgclient, lobj.MachineOSBuild, lobj.MachineOSConfig) + + assert.NoError(t, jim.Start(ctx)) + + job, err := kubeclient.BatchV1().Jobs(ctrlcommon.MCONamespace).Get(ctx, utils.GetBuildJobName(lobj.MachineOSBuild), metav1.GetOptions{}) + require.NoError(t, err) + + // Set -60 seconds so that we can get a time that is one minute before the current time. + jobStartTime := time.Now().Add(time.Second * -60) + job.SetCreationTimestamp(metav1.NewTime(jobStartTime)) + + _, err = kubeclient.BatchV1().Jobs(ctrlcommon.MCONamespace).Update(ctx, job, metav1.UpdateOptions{}) + require.NoError(t, err) + + fixtures.SetJobStatus(ctx, t, kubeclient, lobj.MachineOSBuild, fixtures.JobStatus{Succeeded: 1}) + + status, err := jim.MachineOSBuildStatus(ctx) + require.NoError(t, err) + + assert.NotNil(t, status.BuildStart) + assert.NotNil(t, status.BuildEnd) + + assert.True(t, status.BuildStart.Before(status.BuildEnd)) + assert.GreaterOrEqual(t, status.BuildEnd.Time.Sub(status.BuildStart.Time), time.Second*60) + assert.Equal(t, status.BuildStart.Time, jobStartTime) +} diff --git a/pkg/controller/build/osbuildcontroller_test.go b/pkg/controller/build/osbuildcontroller_test.go index 668665d0af..8c71adca05 100644 --- a/pkg/controller/build/osbuildcontroller_test.go +++ b/pkg/controller/build/osbuildcontroller_test.go @@ -11,6 +11,7 @@ import ( mcfgv1 "github.com/openshift/api/machineconfiguration/v1" mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned" fakeclientmachineconfigv1 "github.com/openshift/client-go/machineconfiguration/clientset/versioned/fake" + "github.com/openshift/machine-config-operator/pkg/apihelpers" "github.com/openshift/machine-config-operator/pkg/controller/build/buildrequest" "github.com/openshift/machine-config-operator/pkg/controller/build/constants" "github.com/openshift/machine-config-operator/pkg/controller/build/fixtures" @@ -19,6 +20,7 @@ import ( testhelpers "github.com/openshift/machine-config-operator/test/helpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + batchv1 "k8s.io/api/batch/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" @@ -517,6 +519,108 @@ func TestOSBuildControllerBuildFailedDoesNotCascade(t *testing.T) { } } +// This scenario tests the case where the controller restarts and a running job +// completes before the reconcilation loop can run. To simulate that, this test +// performs all of the setup steps and creates a successful Job before starting +// the controller. +func TestOSBuildControllerReconcilesAfterRestart(t *testing.T) { + mainCtx, mainCancel := context.WithTimeout(context.Background(), time.Second*5) + t.Cleanup(mainCancel) + + testCases := []struct { + name string + jobStatus fixtures.JobStatus + conditions []metav1.Condition + assertions func(*testhelpers.Assertions, *mcfgv1.MachineOSBuild) + }{ + { + name: "Empty MOSB conditions -> Running", + jobStatus: fixtures.JobStatus{Active: 1}, + conditions: []metav1.Condition{}, + assertions: func(kubeassert *testhelpers.Assertions, mosb *mcfgv1.MachineOSBuild) { + kubeassert.MachineOSBuildIsRunning(mosb) + kubeassert.JobExists(utils.GetBuildJobName(mosb)) + }, + }, + { + name: "Initial MOSB -> Running", + jobStatus: fixtures.JobStatus{Active: 1}, + conditions: apihelpers.MachineOSBuildInitialConditions(), + assertions: func(kubeassert *testhelpers.Assertions, mosb *mcfgv1.MachineOSBuild) { + kubeassert.MachineOSBuildIsRunning(mosb) + kubeassert.JobExists(utils.GetBuildJobName(mosb)) + }, + }, + { + name: "Running MOSB -> Succeeded", + jobStatus: fixtures.JobStatus{Succeeded: 1}, + conditions: apihelpers.MachineOSBuildRunningConditions(), + assertions: func(kubeassert *testhelpers.Assertions, mosb *mcfgv1.MachineOSBuild) { + kubeassert.MachineOSBuildIsSuccessful(mosb) + kubeassert.JobDoesNotExist(utils.GetBuildJobName(mosb)) + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(mainCtx) + t.Cleanup(cancel) + + poolName := "worker" + + kubeclient, mcfgclient, lobj, kubeassert := fixtures.GetClientsForTest(t) + + kubeassert = kubeassert.Eventually().WithContext(ctx).WithPollInterval(time.Millisecond) + mcp := lobj.MachineConfigPool + mosc := lobj.MachineOSConfig + mosc.Name = fmt.Sprintf("%s-os-config", poolName) + + _, err := mcfgclient.MachineconfigurationV1().MachineOSConfigs().Create(ctx, mosc, metav1.CreateOptions{}) + require.NoError(t, err) + + mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, mosc, mcp) + apiMosb, err := mcfgclient.MachineconfigurationV1().MachineOSBuilds().Create(ctx, mosb, metav1.CreateOptions{}) + require.NoError(t, err) + + // This represents the state of the MachineOSBuild before the build + // controller comes back up after a restart. A job that is in a terminal + // state will produce a different set of conditions which these conditions + // will be compared to. + apiMosb.Status.Conditions = testCase.conditions + + _, err = mcfgclient.MachineconfigurationV1().MachineOSBuilds().UpdateStatus(ctx, apiMosb, metav1.UpdateOptions{}) + require.NoError(t, err) + + br, err := buildrequest.NewBuildRequestFromAPI(ctx, kubeclient, mcfgclient, apiMosb, mosc) + require.NoError(t, err) + + buildJob := br.Builder().GetObject().(*batchv1.Job) + + _, err = kubeclient.BatchV1().Jobs(ctrlcommon.MCONamespace).Create(ctx, buildJob, metav1.CreateOptions{}) + require.NoError(t, err) + + fixtures.SetJobStatus(ctx, t, kubeclient, mosb, testCase.jobStatus) + + cfg := Config{ + MaxRetries: 1, + UpdateDelay: 0, + } + + ctrl := newOSBuildController(cfg, mcfgclient, kubeclient) + + // Use a work queue which is tuned for testing. + ctrl.execQueue = ctrlcommon.NewWrappedQueueForTesting(t) + + // Start the build controller + go ctrl.Run(ctx, 5) + + kubeassert.MachineOSBuildExists(mosb) + testCase.assertions(kubeassert, mosb) + }) + } +} + func assertBuildObjectsAreCreated(ctx context.Context, t *testing.T, kubeassert *testhelpers.Assertions, mosb *mcfgv1.MachineOSBuild) { t.Helper() diff --git a/pkg/controller/build/reconciler.go b/pkg/controller/build/reconciler.go index 862179c779..6bdffcd892 100644 --- a/pkg/controller/build/reconciler.go +++ b/pkg/controller/build/reconciler.go @@ -176,6 +176,11 @@ func (b *buildReconciler) deleteMachineOSConfig(ctx context.Context, mosc *mcfgv func (b *buildReconciler) AddJob(ctx context.Context, job *batchv1.Job) error { return b.timeObjectOperation(job, addingVerb, func() error { klog.Infof("Adding build job %q", job.Name) + + if err := b.updateMachineOSBuildWithStatus(ctx, job); err != nil { + return fmt.Errorf("could not update job status for %q: %w", job.Name, err) + } + return b.syncAll(ctx) }) } diff --git a/pkg/controller/common/mos_state.go b/pkg/controller/common/mos_state.go index 056938a524..0a52a7fe32 100644 --- a/pkg/controller/common/mos_state.go +++ b/pkg/controller/common/mos_state.go @@ -121,12 +121,7 @@ func (b *MachineOSBuildState) IsInTransientState() bool { // Gets the transient state, if any is set. Otherwise, returns an empty string. func (b *MachineOSBuildState) GetTransientState() mcfgv1.BuildProgress { - transientStates := []mcfgv1.BuildProgress{ - mcfgv1.MachineOSBuilding, - mcfgv1.MachineOSBuildPrepared, - } - - for _, transientState := range transientStates { + for transientState := range MachineOSBuildTransientStates() { if apihelpers.IsMachineOSBuildConditionTrue(b.Build.Status.Conditions, transientState) { return transientState } @@ -137,13 +132,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 { - terminalStates := []mcfgv1.BuildProgress{ - mcfgv1.MachineOSBuildSucceeded, - mcfgv1.MachineOSBuildFailed, - mcfgv1.MachineOSBuildInterrupted, - } - - for _, terminalState := range terminalStates { + for terminalState := range MachineOSBuildTerminalStates() { if apihelpers.IsMachineOSBuildConditionTrue(b.Build.Status.Conditions, terminalState) { return terminalState } @@ -181,6 +170,27 @@ func (b *MachineOSBuildState) SetBuildConditions(conditions []metav1.Condition) } } +// Returns a map of the buildprogress states to their expected conditions for +// transient states. That is, the MachineOSBuild is expected to transition from +// one state to another. +func MachineOSBuildTransientStates() map[mcfgv1.BuildProgress][]metav1.Condition { + return map[mcfgv1.BuildProgress][]metav1.Condition{ + mcfgv1.MachineOSBuilding: apihelpers.MachineOSBuildRunningConditions(), + mcfgv1.MachineOSBuildPrepared: apihelpers.MachineOSBuildPendingConditions(), + } +} + +// Returns a map of the buildprogress states to their expected conditions for +// terminal states; meaning that the MachineOSBuild cannot transition from one +// state to another. +func MachineOSBuildTerminalStates() map[mcfgv1.BuildProgress][]metav1.Condition { + return map[mcfgv1.BuildProgress][]metav1.Condition{ + mcfgv1.MachineOSBuildSucceeded: apihelpers.MachineOSBuildSucceededConditions(), + mcfgv1.MachineOSBuildFailed: apihelpers.MachineOSBuildFailedConditions(), + mcfgv1.MachineOSBuildInterrupted: apihelpers.MachineOSBuildInterruptedConditions(), + } +} + // Determines if two conditions are equal. Note: I purposely do not include the // timestamp in the equality test, since we do not directly set it. func isConditionEqual(cond1, cond2 metav1.Condition) bool { diff --git a/pkg/controller/common/mos_state_test.go b/pkg/controller/common/mos_state_test.go index 4c68184870..2b23e4e06a 100644 --- a/pkg/controller/common/mos_state_test.go +++ b/pkg/controller/common/mos_state_test.go @@ -3,7 +3,9 @@ package common import ( "testing" + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" v1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/machine-config-operator/pkg/apihelpers" "github.com/stretchr/testify/assert" ) @@ -18,3 +20,108 @@ func TestMachineOSConfigState(t *testing.T) { assert.Equal(t, "registry.host.com/org/repo:tag", mosc.GetOSImage()) } + +// This test validates that the MachineOSBuild conditions are correctly +// identified as initial, transient, or terminal. +func TestMachineOSBuildState(t *testing.T) { + t.Parallel() + + // Determines if the helper associated with the given state returns true + // while others associated with other states returns false. If an empty state + // is given, it will not match any of these states and will therefore be + // false. + assertBuildState := func(t *testing.T, mosbState *MachineOSBuildState, givenState mcfgv1.BuildProgress) { + t.Helper() + + states := map[mcfgv1.BuildProgress]func() bool{ + mcfgv1.MachineOSBuildPrepared: mosbState.IsBuildPrepared, + mcfgv1.MachineOSBuilding: mosbState.IsBuilding, + mcfgv1.MachineOSBuildSucceeded: mosbState.IsBuildSuccess, + mcfgv1.MachineOSBuildFailed: mosbState.IsBuildFailure, + mcfgv1.MachineOSBuildInterrupted: mosbState.IsBuildInterrupted, + } + + for state, helper := range states { + // If the current state matches the given state, then that helper should + // return true. Otherwise, it should be false. + if state == givenState { + assert.True(t, helper()) + } else { + assert.False(t, helper()) + } + } + + degradedStates := map[mcfgv1.BuildProgress]struct{}{ + mcfgv1.MachineOSBuildFailed: struct{}{}, + mcfgv1.MachineOSBuildInterrupted: struct{}{}, + } + + if _, isDegradedState := degradedStates[givenState]; isDegradedState { + assert.True(t, mosbState.IsAnyDegraded()) + } else { + assert.False(t, mosbState.IsAnyDegraded()) + } + } + + // For the initial condition, ensure that it is correctly identified as an + // initial condition and not a transient or terminal condition. + t.Run("Initial Conditions", func(t *testing.T) { + t.Parallel() + + mosbState := NewMachineOSBuildState(&mcfgv1.MachineOSBuild{}) + mosbState.SetBuildConditions(apihelpers.MachineOSBuildInitialConditions()) + + assert.True(t, mosbState.IsInInitialState()) + assert.False(t, mosbState.IsInTransientState()) + assert.False(t, mosbState.IsInTerminalState()) + + assert.Equal(t, mosbState.GetTerminalState(), mcfgv1.BuildProgress("")) + assert.Equal(t, mosbState.GetTransientState(), mcfgv1.BuildProgress("")) + assertBuildState(t, mosbState, mcfgv1.BuildProgress("")) + }) + + // For each transient condition, ensure that it is correctly identified as + // a transient condition and not an initial or terminal condition. + // Additionally, check that each helper associated with a given condition + // correctly identifies that condition as being true and all others false. + t.Run("Transient Conditions", func(t *testing.T) { + t.Parallel() + + mosbState := NewMachineOSBuildState(&mcfgv1.MachineOSBuild{}) + + for transientState, transientCondition := range MachineOSBuildTransientStates() { + mosbState.SetBuildConditions(transientCondition) + + assert.False(t, mosbState.IsInInitialState()) + assert.True(t, mosbState.IsInTransientState()) + assert.False(t, mosbState.IsInTerminalState()) + + assert.Equal(t, mosbState.GetTransientState(), transientState) + assert.Equal(t, mosbState.GetTerminalState(), mcfgv1.BuildProgress("")) + assertBuildState(t, mosbState, transientState) + } + }) + + // For each terminal condition, ensure that it is correctly identified as a + // terminal condition and not as a transient or initial condition. + // Additionally, check that each helper associated with a given condition + // correctly identifies that condition as being true and all others false. + t.Run("Terminal Conditions", func(t *testing.T) { + t.Parallel() + + mosbState := NewMachineOSBuildState(&mcfgv1.MachineOSBuild{}) + + for terminalState, terminalCondition := range MachineOSBuildTerminalStates() { + mosbState.SetBuildConditions(terminalCondition) + + assert.False(t, mosbState.IsInInitialState()) + assert.False(t, mosbState.IsInTransientState()) + assert.True(t, mosbState.IsInTerminalState()) + + assert.Equal(t, mosbState.GetTransientState(), mcfgv1.BuildProgress("")) + assert.Equal(t, mosbState.GetTerminalState(), terminalState) + + assertBuildState(t, mosbState, terminalState) + } + }) +} diff --git a/test/e2e-ocl/helpers_test.go b/test/e2e-ocl/helpers_test.go index f03d356aed..3b48d6ffd7 100644 --- a/test/e2e-ocl/helpers_test.go +++ b/test/e2e-ocl/helpers_test.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strings" "testing" "time" @@ -32,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" aggerrs "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/util/retry" "sigs.k8s.io/yaml" ) @@ -878,3 +880,93 @@ func resolveTaggedPullspecToDigestedPullspec(ctx context.Context, pullspec strin return canonical.String(), nil } + +// TODO: Deduplicate this definition from machine-config-operator/devex/internal/pkg/rollout/rollout.go +// Having "internal" in the module path prevents us from reusing it here since +// it is internal to the devex directory. +func setDeploymentReplicas(t *testing.T, cs *framework.ClientSet, deployment metav1.ObjectMeta, replicas int32) error { + return retry.RetryOnConflict(retry.DefaultBackoff, func() error { + t.Logf("Setting replicas for %s/%s to %d", deployment.Namespace, deployment.Name, replicas) + scale, err := cs.AppsV1Interface.Deployments(deployment.Namespace).GetScale(context.TODO(), deployment.Name, metav1.GetOptions{}) + if err != nil { + return err + } + + scale.Spec.Replicas = replicas + + _, err = cs.AppsV1Interface.Deployments(deployment.Namespace).UpdateScale(context.TODO(), deployment.Name, scale, metav1.UpdateOptions{}) + return err + }) +} + +// Scales down the machine-os-builder, machine-config-opreator, and +// cluster-version-operator deployments. Registers and returns an idempotent +// function that will scale the deployments back to their original values. +func scaleDownDeployments(t *testing.T, cs *framework.ClientSet) func() { + deployments := []metav1.ObjectMeta{ + // Scale down the cluster-version-operator since it could set the desired + // replicas for the MCO to 1. + { + Name: "cluster-version-operator", + Namespace: "openshift-cluster-version", + }, + // Scale down the machine-config-operator since it could set the desired + // replicas for the build controller to 1. + { + Name: "machine-config-operator", + Namespace: ctrlcommon.MCONamespace, + }, + // Scale down the machine-os-builder since we want to simulate its pod + // being rescheduled. + { + Name: "machine-os-builder", + Namespace: ctrlcommon.MCONamespace, + }, + } + + restoreFuncs := []func(){} + + for _, deployment := range deployments { + restoreFuncs = append(restoreFuncs, scaleDownDeployment(t, cs, deployment)) + } + + return helpers.MakeIdempotentAndRegister(t, func() { + // Restore the deployments in the reverse order by which we disabled them. + // Not really necessary, but we want to ensure that the machine-os-builder + // deployment starts back up as soon as possible. + slices.Reverse(restoreFuncs) + + for _, restoreFunc := range restoreFuncs { + restoreFunc() + } + }) +} + +// Scales down a given deployment unless that deployment is already set to zero +// replicas, in which case it no-ops. Registers and returns an idempotent +// restoral function that will revert the deployment back to its original +// setting. +func scaleDownDeployment(t *testing.T, cs *framework.ClientSet, deployment metav1.ObjectMeta) func() { + ctx := context.TODO() + + originalDeployment, err := cs.AppsV1Interface.Deployments(deployment.Namespace).Get(ctx, deployment.Name, metav1.GetOptions{}) + require.NoError(t, err) + + originalReplicas := *originalDeployment.Spec.Replicas + + // We check if the original replica count is zero. This is because it is very + // common for a dev sandbox cluster to at least have the CVO disabled. + if originalReplicas == 0 { + t.Logf("Original replica count for deployment %s/%s set to 0, skipping scale down", deployment.Namespace, deployment.Name) + + return helpers.MakeIdempotentAndRegister(t, func() { + t.Logf("Original replica count for deployment %s/%s set to 0, skipping restore", deployment.Namespace, deployment.Name) + }) + } + + require.NoError(t, setDeploymentReplicas(t, cs, deployment, 0)) + + return helpers.MakeIdempotentAndRegister(t, func() { + require.NoError(t, setDeploymentReplicas(t, cs, deployment, originalReplicas)) + }) +} diff --git a/test/e2e-ocl/onclusterlayering_test.go b/test/e2e-ocl/onclusterlayering_test.go index 632ac9b3f1..c66c4206de 100644 --- a/test/e2e-ocl/onclusterlayering_test.go +++ b/test/e2e-ocl/onclusterlayering_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" ign3types "github.com/coreos/ignition/v2/config/v3_4/types" @@ -955,3 +956,102 @@ func TestSSHKeyAndPasswordForOSBuilder(t *testing.T) { mcCleanupFunc() }) } + +// This test starts a build and then immediately scales down the +// machine-os-builder deployment until the underlying build job has completed. +// The rationale behind this test is so that if the machine-os-builder pod gets +// rescheduled onto a different node while a build is occurring that the +// MachineOSBuild object will eventually be reconciled, even if the build +// completed during the rescheduling operation. +func TestControllerEventuallyReconciles(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + cs := framework.NewClientSet("") + + poolName := layeredMCPName + + mosc := prepareForOnClusterLayeringTest(t, cs, onClusterLayeringTestOpts{ + poolName: poolName, + customDockerfiles: map[string]string{ + layeredMCPName: cowsayDockerfile, + }, + }) + + mcp, err := cs.MachineconfigurationV1Interface.MachineConfigPools().Get(ctx, poolName, metav1.GetOptions{}) + require.NoError(t, err) + + mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, cs.GetKubeclient(), mosc, mcp) + + jobName := utils.GetBuildJobName(mosb) + + createMachineOSConfig(t, cs, mosc) + + // Wait for the MachineOSBuild to exist. + kubeassert := helpers.AssertClientSet(t, cs).WithContext(ctx).Eventually() + kubeassert.MachineOSBuildExists(mosb) + kubeassert.JobExists(utils.GetBuildJobName(mosb)) + + t.Logf("MachineOSBuild %q exists, stopping machine-os-builder", mosb.Name) + + // As soon as the MachineOSBuild exists, scale down the machine-os-builder + // deployment and any other deployments which may inadvertantly cause its + // replica count to increase. This is done to simulate the machine-os-builder + // pod being scheduled onto a different node. + restoreDeployments := scaleDownDeployments(t, cs) + + // Wait for the job to start running. + waitForJobToReachCondition(ctx, t, cs, jobName, func(job *batchv1.Job) (bool, error) { + return job.Status.Active == 1, nil + }) + + t.Logf("Job %s has started running, starting machine-os-builder", jobName) + + // Restore the deployments. + restoreDeployments() + + // Ensure that the MachineOSBuild object eventually gets updated. + kubeassert.MachineOSBuildIsRunning(mosb) + + t.Logf("MachineOSBuild %s is now running, stopping machine-os-builder", mosb.Name) + + // Stop the deployments again. + restoreDeployments = scaleDownDeployments(t, cs) + + // Wait for the job to complete. + waitForJobToReachCondition(ctx, t, cs, utils.GetBuildJobName(mosb), func(job *batchv1.Job) (bool, error) { + return job.Status.Succeeded == 1, nil + }) + + t.Logf("Job %q finished, starting machine-os-builder", jobName) + + // Restore the deployments again. + restoreDeployments() + + // At this point, the machine-os-builder is running, so we wait for the build + // itself to complete and be updated. + mosb = waitForBuildToComplete(t, cs, mosb) + + // Wait until the MachineOSConfig gets the digested pullspec from the MachineOSBuild. + require.NoError(t, wait.PollImmediate(1*time.Second, 5*time.Minute, func() (bool, error) { + mosc, err := cs.MachineconfigurationV1Interface.MachineOSConfigs().Get(ctx, mosc.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + + return mosc.Status.CurrentImagePullSpec != "" && mosc.Status.CurrentImagePullSpec == mosb.Status.DigestedImagePushSpec, nil + })) +} + +// Waits for a job object to reach a given state. +// TOOD: Add this to the Asserts helper struct. +func waitForJobToReachCondition(ctx context.Context, t *testing.T, cs *framework.ClientSet, jobName string, condFunc func(*batchv1.Job) (bool, error)) { + require.NoError(t, wait.PollImmediate(1*time.Second, 10*time.Minute, func() (bool, error) { + job, err := cs.BatchV1Interface.Jobs(ctrlcommon.MCONamespace).Get(ctx, jobName, metav1.GetOptions{}) + if err != nil { + return false, err + } + + return condFunc(job) + })) +}