Merge pull request #4700 from umohnani8/jobs-final

MCO-1231: MCO-783: MCO-1232: Use Jobs for OCL builds

Author: openshift-merge-bot[bot]

manifests/machineosbuilder/clusterrole.yaml

@@ -64,3 +64,9 @@ rules:
   - leases
   verbs:
   - "*"
+- apiGroups:
+  - batch
+  resources:
+  - jobs
+  verbs:
+  - "*"

pkg/controller/build/buildrequest/assets/buildah-build.sh

@@ -43,6 +43,7 @@ function retry {
       echo "Retry $count/$MAX_RETRIES exited $exit, retrying..."
     else
       echo "Retry $count/$MAX_RETRIES exited $exit, no more retries left."
+	  echo $exit > /tmp/done/errorfile
       return $exit
     fi
   done

pkg/controller/build/buildrequest/assets/wait.sh

@@ -8,10 +8,22 @@
 # that the build operation is complete.
 set -euo
 
-# Wait until the done file appears.
-while [ ! -f "/tmp/done/digestfile" ]
-do
-	sleep 1
+# Wait for either the digestfile or an errorfile
+while true; do
+  if [ -f "/tmp/done/digestfile" ]; then
+    # If digestfile is found, break the loop and proceed
+    echo "Digest file found. Proceeding with ConfigMap creation."
+    break
+  elif [ -f "/tmp/done/errorfile" ]; then
+    # If errorfile is found, exit the script with an error
+    echo "Error file found. Exiting."
+    # Delete the error file so that when a build pod is scheduled again
+    # on the same node, it doesn't error out immediately
+    # /tmp should be automatically cleared up on reboot but let's manually clean up as well
+    rm /tmp/done/errorfile
+    exit 1
+  fi
+  sleep 1
 done
 
 # Inject the contents of the digestfile into a ConfigMap.

pkg/controller/build/buildrequest/buildrequest.go

@@ -14,6 +14,7 @@ import (
 	"github.com/openshift/machine-config-operator/pkg/controller/build/constants"
 	"github.com/openshift/machine-config-operator/pkg/controller/build/utils"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
+	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -33,7 +34,7 @@ var buildahBuildScript string
 var podmanBuildScript string
 
 const (
-	// Filename for the machineconfig JSON tarball expected by the build pod
+	// Filename for the machineconfig JSON tarball expected by the build job
 	machineConfigJSONFilename string = "machineconfig.json.gz"
 )
 
@@ -75,9 +76,9 @@ func (br buildRequestImpl) Opts() BuildRequestOpts {
 	return br.opts
 }
 
-// Creates the Build Pod object.
+// Creates the Build Job object.
 func (br buildRequestImpl) Builder() Builder {
-	return newBuilder(br.toBuildahPod())
+	return newBuilder(br.podToJob(br.toBuildahPod()))
 }
 
 // Takes the configured secrets and creates an ephemeral clone of them, canonicalizing them, if needed.
@@ -242,6 +243,30 @@ func (br buildRequestImpl) renderContainerfile() (string, error) {
 	return out.String(), nil
 }
 
+// podToJob creates a Job with the spec of the given Pod
+func (br buildRequestImpl) podToJob(pod *corev1.Pod) *batchv1.Job {
+	// Set the backoffLimit to 3 so the job will retry 4 times before reporting a failure
+	var backoffLimit int32 = 3
+	// Set completion to 1 so that as soon as the pod has completed successfully the job is
+	// considered a success
+	var completions int32 = 1
+	return &batchv1.Job{
+		ObjectMeta: pod.ObjectMeta,
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: "batch/v1",
+			Kind:       "Job",
+		},
+		Spec: batchv1.JobSpec{
+			BackoffLimit: &backoffLimit,
+			Completions:  &completions,
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{},
+				Spec:       pod.Spec,
+			},
+		},
+	}
+}
+
 // We're able to run the Buildah image in an unprivileged pod provided that the
 // machine-os-builder service account has the anyuid security constraint
 // context enabled to allow us to use UID 1000, which maps to the UID within
@@ -569,7 +594,7 @@ func (br buildRequestImpl) getMCConfigMapName() string {
 
 // Computes the build name based upon the MachineConfigPool name.
 func (br buildRequestImpl) getBuildName() string {
-	return utils.GetBuildPodName(br.opts.MachineOSBuild)
+	return utils.GetBuildJobName(br.opts.MachineOSBuild)
 }
 
 func (br buildRequestImpl) getDigestConfigMapName() string {

pkg/controller/build/buildrequest/buildrequest_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/openshift/machine-config-operator/pkg/controller/build/utils"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/stretchr/testify/assert"
+	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -141,22 +142,22 @@ func TestBuildRequest(t *testing.T) {
 				assert.NotContains(t, containerfile, content)
 			}
 
-			buildPod := br.Builder().GetObject().(*corev1.Pod)
+			buildJob := br.Builder().GetObject().(*batchv1.Job)
 
-			_, err = NewBuilder(buildPod)
+			_, err = NewBuilder(buildJob)
 			assert.NoError(t, err)
 
 			assert.Equal(t, "containerfile-worker-afc35db0f874c9bfdc586e6ba39f1504", configmaps[0].Name)
 			assert.Equal(t, "mc-worker-afc35db0f874c9bfdc586e6ba39f1504", configmaps[1].Name)
-			assert.Equal(t, "build-worker-afc35db0f874c9bfdc586e6ba39f1504", buildPod.Name)
+			assert.Equal(t, "build-worker-afc35db0f874c9bfdc586e6ba39f1504", buildJob.Name)
 
 			secrets, err := br.Secrets()
 			assert.NoError(t, err)
 
 			objects := []metav1.Object{
 				configmaps[0],
 				configmaps[1],
-				buildPod,
+				buildJob,
 				secrets[0],
 				secrets[1],
 			}
@@ -174,7 +175,7 @@ func TestBuildRequest(t *testing.T) {
 			assert.Equal(t, secrets[0].Name, "base-worker-afc35db0f874c9bfdc586e6ba39f1504")
 			assert.Equal(t, secrets[1].Name, "final-worker-afc35db0f874c9bfdc586e6ba39f1504")
 
-			assertBuildPodIsCorrect(t, buildPod, opts)
+			assertBuildJobIsCorrect(t, buildJob, opts)
 		})
 	}
 }
@@ -190,37 +191,37 @@ func assertSecretInCorrectFormat(t *testing.T, secret *corev1.Secret) {
 	assert.JSONEq(t, string(secret.Data[corev1.DockerConfigJsonKey]), `{"auths":{"registry.hostname.com": {"username": "user", "password": "s3kr1t", "auth": "s00pers3kr1t", "email": "user@hostname.com"}}}`)
 }
 
-func assertBuildPodIsCorrect(t *testing.T, buildPod *corev1.Pod, opts BuildRequestOpts) {
+func assertBuildJobIsCorrect(t *testing.T, buildJob *batchv1.Job, opts BuildRequestOpts) {
 	etcRpmGpgKeysOpts := optsForEtcRpmGpgKeys()
-	assertBuildPodMatchesExpectations(t, opts.HasEtcPkiRpmGpgKeys, buildPod,
+	assertBuildJobMatchesExpectations(t, opts.HasEtcPkiRpmGpgKeys, buildJob,
 		etcRpmGpgKeysOpts.envVar(),
 		etcRpmGpgKeysOpts.volumeForSecret(constants.EtcPkiRpmGpgSecretName),
 		etcRpmGpgKeysOpts.volumeMount(),
 	)
 
 	etcYumReposDOpts := optsForEtcYumReposD()
-	assertBuildPodMatchesExpectations(t, opts.HasEtcYumReposDConfigs, buildPod,
+	assertBuildJobMatchesExpectations(t, opts.HasEtcYumReposDConfigs, buildJob,
 		etcYumReposDOpts.envVar(),
 		etcYumReposDOpts.volumeForConfigMap(),
 		etcYumReposDOpts.volumeMount(),
 	)
 
 	etcPkiEntitlementKeysOpts := optsForEtcPkiEntitlements()
-	assertBuildPodMatchesExpectations(t, opts.HasEtcPkiEntitlementKeys, buildPod,
+	assertBuildJobMatchesExpectations(t, opts.HasEtcPkiEntitlementKeys, buildJob,
 		etcPkiEntitlementKeysOpts.envVar(),
 		etcPkiEntitlementKeysOpts.volumeForSecret(constants.EtcPkiEntitlementSecretName+"-"+opts.MachineOSConfig.Spec.MachineConfigPool.Name),
 		etcPkiEntitlementKeysOpts.volumeMount(),
 	)
 
-	assert.Equal(t, buildPod.Spec.Containers[0].Image, mcoImagePullspec)
+	assert.Equal(t, buildJob.Spec.Template.Spec.Containers[0].Image, mcoImagePullspec)
 	expectedPullspecs := []string{
 		"base-os-image-from-machineosconfig",
 		fixtures.OSImageURLConfig().BaseOSContainerImage,
 	}
 
-	assert.Contains(t, expectedPullspecs, buildPod.Spec.Containers[1].Image)
+	assert.Contains(t, expectedPullspecs, buildJob.Spec.Template.Spec.Containers[1].Image)
 
-	assertPodHasVolume(t, buildPod, corev1.Volume{
+	assertPodHasVolume(t, buildJob.Spec.Template.Spec, corev1.Volume{
 		Name: "final-image-push-creds",
 		VolumeSource: corev1.VolumeSource{
 			Secret: &corev1.SecretVolumeSource{
@@ -235,7 +236,7 @@ func assertBuildPodIsCorrect(t *testing.T, buildPod *corev1.Pod, opts BuildReque
 		},
 	})
 
-	assertPodHasVolume(t, buildPod, corev1.Volume{
+	assertPodHasVolume(t, buildJob.Spec.Template.Spec, corev1.Volume{
 		Name: "base-image-pull-creds",
 		VolumeSource: corev1.VolumeSource{
 			Secret: &corev1.SecretVolumeSource{
@@ -251,20 +252,20 @@ func assertBuildPodIsCorrect(t *testing.T, buildPod *corev1.Pod, opts BuildReque
 	})
 }
 
-func assertPodHasVolume(t *testing.T, pod *corev1.Pod, volume corev1.Volume) {
-	assert.Contains(t, pod.Spec.Volumes, volume)
+func assertPodHasVolume(t *testing.T, pod corev1.PodSpec, volume corev1.Volume) {
+	assert.Contains(t, pod.Volumes, volume)
 }
 
-func assertBuildPodMatchesExpectations(t *testing.T, shouldBePresent bool, buildPod *corev1.Pod, envvar corev1.EnvVar, volume corev1.Volume, volumeMount corev1.VolumeMount) {
-	for _, container := range buildPod.Spec.Containers {
+func assertBuildJobMatchesExpectations(t *testing.T, shouldBePresent bool, buildJob *batchv1.Job, envvar corev1.EnvVar, volume corev1.Volume, volumeMount corev1.VolumeMount) {
+	for _, container := range buildJob.Spec.Template.Spec.Containers {
 		if shouldBePresent {
 			assert.Contains(t, container.Env, envvar)
 			assert.Contains(t, container.VolumeMounts, volumeMount)
-			assertPodHasVolume(t, buildPod, volume)
+			assertPodHasVolume(t, buildJob.Spec.Template.Spec, volume)
 		} else {
 			assert.NotContains(t, container.Env, envvar)
 			assert.NotContains(t, container.VolumeMounts, volumeMount)
-			assert.NotContains(t, buildPod.Spec.Volumes, volume)
+			assert.NotContains(t, buildJob.Spec.Template.Spec.Volumes, volume)
 		}
 
 		assert.Contains(t, container.Env, corev1.EnvVar{

pkg/controller/build/clients.go

@@ -5,9 +5,9 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	coreinformers "k8s.io/client-go/informers"
-	coreinformersv1 "k8s.io/client-go/informers/core/v1"
+	batchinformersv1 "k8s.io/client-go/informers/batch/v1"
 	clientset "k8s.io/client-go/kubernetes"
-	corelistersv1 "k8s.io/client-go/listers/core/v1"
+	batchlisterv1 "k8s.io/client-go/listers/batch/v1"
 	"k8s.io/client-go/tools/cache"
 
 	mcfgclientset "github.com/openshift/client-go/machineconfiguration/clientset/versioned"
@@ -25,7 +25,7 @@ import (
 type informers struct {
 	controllerConfigInformer  mcfginformersv1.ControllerConfigInformer
 	machineConfigPoolInformer mcfginformersv1.MachineConfigPoolInformer
-	podInformer               coreinformersv1.PodInformer
+	jobInformer               batchinformersv1.JobInformer
 	machineOSBuildInformer    mcfginformersv1alpha1.MachineOSBuildInformer
 	machineOSConfigInformer   mcfginformersv1alpha1.MachineOSConfigInformer
 	toStart                   []interface{ Start(<-chan struct{}) }
@@ -45,7 +45,7 @@ func (i *informers) listers() *listers {
 		machineOSBuildLister:    i.machineOSBuildInformer.Lister(),
 		machineOSConfigLister:   i.machineOSConfigInformer.Lister(),
 		machineConfigPoolLister: i.machineConfigPoolInformer.Lister(),
-		podLister:               i.podInformer.Lister(),
+		jobLister:               i.jobInformer.Lister(),
 		controllerConfigLister:  i.controllerConfigInformer.Lister(),
 	}
 }
@@ -55,7 +55,7 @@ type listers struct {
 	machineOSBuildLister    mcfglistersv1alpha1.MachineOSBuildLister
 	machineOSConfigLister   mcfglistersv1alpha1.MachineOSConfigLister
 	machineConfigPoolLister mcfglistersv1.MachineConfigPoolLister
-	podLister               corelistersv1.PodLister
+	jobLister               batchlisterv1.JobLister
 	controllerConfigLister  mcfglistersv1.ControllerConfigLister
 }
 
@@ -87,22 +87,22 @@ func newInformers(mcfgclient mcfgclientset.Interface, kubeclient clientset.Inter
 	machineConfigPoolInformer := mcoInformerFactory.Machineconfiguration().V1().MachineConfigPools()
 	machineOSBuildInformer := mcoInformerFactory.Machineconfiguration().V1alpha1().MachineOSBuilds()
 	machineOSConfigInformer := mcoInformerFactory.Machineconfiguration().V1alpha1().MachineOSConfigs()
-	podInformer := coreInformerFactory.Core().V1().Pods()
+	jobInformer := coreInformerFactory.Batch().V1().Jobs()
 
 	return &informers{
 		controllerConfigInformer:  controllerConfigInformer,
 		machineConfigPoolInformer: machineConfigPoolInformer,
 		machineOSBuildInformer:    machineOSBuildInformer,
 		machineOSConfigInformer:   machineOSConfigInformer,
-		podInformer:               podInformer,
+		jobInformer:               jobInformer,
 		toStart: []interface{ Start(<-chan struct{}) }{
 			mcoInformerFactory,
 			coreInformerFactory,
 		},
 		hasSynced: []cache.InformerSynced{
 			controllerConfigInformer.Informer().HasSynced,
 			machineConfigPoolInformer.Informer().HasSynced,
-			podInformer.Informer().HasSynced,
+			jobInformer.Informer().HasSynced,
 			machineOSBuildInformer.Informer().HasSynced,
 			machineOSConfigInformer.Informer().HasSynced,
 		},

pkg/controller/build/fixtures/helpers.go

@@ -9,45 +9,61 @@ import (
 	mcfgv1alpha1 "github.com/openshift/api/machineconfiguration/v1alpha1"
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/stretchr/testify/require"
+	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	apitypes "k8s.io/apimachinery/pkg/types"
 	clientset "k8s.io/client-go/kubernetes"
 )
 
-// Sets the provided pod phase on a given pod under test. If successful, it will also insert the digestfile ConfigMap.
-func SetPodPhase(ctx context.Context, t *testing.T, kubeclient clientset.Interface, mosb *mcfgv1alpha1.MachineOSBuild, phase corev1.PodPhase) {
-	require.NoError(t, setPodPhase(ctx, kubeclient, mosb, phase))
+// JobStatus is used to set the active, succeeded, and failed fields for the k8s Job
+// Status so that we can parse those to imitate Job phases for testing purposes
+type JobStatus struct {
+	Active                        int32
+	Succeeded                     int32
+	Failed                        int32
+	UncountedTerminatedPodsFailed string
+}
+
+// Sets the provided job status on a given job under test. If successful, it will also insert the digestfile ConfigMap.
+func SetJobStatus(ctx context.Context, t *testing.T, kubeclient clientset.Interface, mosb *mcfgv1alpha1.MachineOSBuild, jobStatus JobStatus) {
+	require.NoError(t, setJobStatusFields(ctx, kubeclient, mosb, jobStatus))
 
-	if phase == corev1.PodSucceeded {
+	if jobStatus.Succeeded == 1 {
 		require.NoError(t, createDigestfileConfigMap(ctx, kubeclient, mosb))
 	} else {
 		require.NoError(t, deleteDigestfileConfigMap(ctx, kubeclient, mosb))
 	}
 }
 
-func SetPodDeletionTimestamp(ctx context.Context, t *testing.T, kubeclient clientset.Interface, mosb *mcfgv1alpha1.MachineOSBuild, timestamp *metav1.Time) {
-	podName := fmt.Sprintf("build-%s", mosb.Name)
+func SetJobDeletionTimestamp(ctx context.Context, t *testing.T, kubeclient clientset.Interface, mosb *mcfgv1alpha1.MachineOSBuild, timestamp *metav1.Time) {
+	jobName := fmt.Sprintf("build-%s", mosb.Name)
 
-	p, err := kubeclient.CoreV1().Pods(ctrlcommon.MCONamespace).Get(ctx, podName, metav1.GetOptions{})
+	j, err := kubeclient.BatchV1().Jobs(ctrlcommon.MCONamespace).Get(ctx, jobName, metav1.GetOptions{})
 	require.NoError(t, err)
 
-	p.SetDeletionTimestamp(timestamp)
+	j.SetDeletionTimestamp(timestamp)
 
-	_, err = kubeclient.CoreV1().Pods(ctrlcommon.MCONamespace).UpdateStatus(ctx, p, metav1.UpdateOptions{})
+	_, err = kubeclient.BatchV1().Jobs(ctrlcommon.MCONamespace).UpdateStatus(ctx, j, metav1.UpdateOptions{})
 	require.NoError(t, err)
 }
 
-func setPodPhase(ctx context.Context, kubeclient clientset.Interface, mosb *mcfgv1alpha1.MachineOSBuild, phase corev1.PodPhase) error {
-	podName := fmt.Sprintf("build-%s", mosb.Name)
+func setJobStatusFields(ctx context.Context, kubeclient clientset.Interface, mosb *mcfgv1alpha1.MachineOSBuild, jobStatus JobStatus) error {
+	jobName := fmt.Sprintf("build-%s", mosb.Name)
 
-	p, err := kubeclient.CoreV1().Pods(ctrlcommon.MCONamespace).Get(ctx, podName, metav1.GetOptions{})
+	j, err := kubeclient.BatchV1().Jobs(ctrlcommon.MCONamespace).Get(ctx, jobName, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
 
-	p.Status.Phase = phase
-	_, err = kubeclient.CoreV1().Pods(ctrlcommon.MCONamespace).UpdateStatus(ctx, p, metav1.UpdateOptions{})
+	j.Status.Active = jobStatus.Active
+	j.Status.Succeeded = jobStatus.Succeeded
+	j.Status.Failed = jobStatus.Failed
+	if jobStatus.UncountedTerminatedPodsFailed != "" {
+		j.Status.UncountedTerminatedPods = &batchv1.UncountedTerminatedPods{Failed: []apitypes.UID{apitypes.UID(jobStatus.UncountedTerminatedPodsFailed)}}
+	}
+	_, err = kubeclient.BatchV1().Jobs(ctrlcommon.MCONamespace).UpdateStatus(ctx, j, metav1.UpdateOptions{})
 	return err
 }
 

pkg/controller/build/imagebuilder/base.go

@@ -379,12 +379,12 @@ func (b *baseImageBuilder) getMachineOSConfigName() (string, error) {
 	return b.builder.MachineOSBuild()
 }
 
-// Gets the name of the builder execution unit (could be a Pod or Job) by
+// Gets the name of the builder execution unit by
 // either looking for the MachineOSBuild name and computing it or by getting it
 // directly from the Builder object.
 func (b *baseImageBuilder) getBuilderName() string {
 	if b.mosb != nil {
-		return utils.GetBuildPodName(b.mosb)
+		return utils.GetBuildJobName(b.mosb)
 	}
 
 	return b.builder.GetObject().GetName()