#21 Use OwnerReferences and labels to find jobs

Names are not totally reliable to find the jobs anymore when we truncate them.

Author: J12934

operator/controllers/execution/scan_controller.go

@@ -116,7 +116,6 @@ func (r *ScanReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 		err = r.setHookStatus(&scan)
 	case "ReadAndWriteHookProcessing":
 		err = r.executeReadAndWriteHooks(&scan)
-
 	case "ReadAndWriteHookCompleted":
 		err = r.startReadOnlyHooks(&scan)
 	case "ReadOnlyHookProcessing":
@@ -129,21 +128,6 @@ func (r *ScanReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	return ctrl.Result{}, nil
 }
 
-func (r *ScanReconciler) getJob(name, namespace string) (*batch.Job, error) {
-	ctx := context.Background()
-
-	var job batch.Job
-	err := r.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, &job)
-	if apierrors.IsNotFound(err) {
-		return nil, nil
-	} else if err != nil {
-		r.Log.Error(err, "unable to get job")
-		return nil, err
-	}
-
-	return &job, nil
-}
-
 type jobCompletionType string
 
 const (
@@ -153,22 +137,51 @@ const (
 	unknown    jobCompletionType = "Unknown"
 )
 
-func (r *ScanReconciler) checkIfJobIsCompleted(name, namespace string) (jobCompletionType, error) {
-	job, err := r.getJob(name, namespace)
-	if err != nil {
-		return unknown, err
+func allJobsCompleted(jobs *batch.JobList) jobCompletionType {
+	hasCompleted := true
+
+	for _, job := range jobs.Items {
+		if job.Status.Failed > 0 {
+			return failed
+		} else if job.Status.Succeeded == 0 {
+			hasCompleted = false
+		}
 	}
-	if job == nil {
-		return unknown, errors.New("Both Job and error were nil. This isn't really expected")
+
+	if hasCompleted {
+		return completed
 	}
+	return incomplete
+}
+
+func (r *ScanReconciler) getJobsForScan(scan *executionv1.Scan, labels client.MatchingLabels) (*batch.JobList, error) {
+	ctx := context.Background()
 
-	if job.Status.Succeeded != 0 {
-		return completed, nil
+	// check if k8s job for scan was already created
+	var jobs batch.JobList
+	if err := r.List(
+		ctx,
+		&jobs,
+		client.InNamespace(scan.Namespace),
+		client.MatchingField(ownerKey, scan.Name),
+		labels,
+	); err != nil {
+		r.Log.Error(err, "Unable to list child jobs")
+		return nil, err
 	}
-	if job.Status.Failed != 0 {
-		return failed, nil
+
+	return &jobs, nil
+}
+
+func (r *ScanReconciler) checkIfJobIsCompleted(scan *executionv1.Scan, labels client.MatchingLabels) (jobCompletionType, error) {
+	jobs, err := r.getJobsForScan(scan, labels)
+	if err != nil {
+		return unknown, err
 	}
-	return unknown, nil
+
+	r.Log.V(9).Info("Got related jobs", "count", len(jobs.Items))
+
+	return allJobsCompleted(jobs), nil
 }
 
 // Helper functions to check and remove string from a slice of strings.
@@ -220,11 +233,11 @@ func (r *ScanReconciler) startScan(scan *executionv1.Scan) error {
 	namespacedName := fmt.Sprintf("%s/%s", scan.Namespace, scan.Name)
 	log := r.Log.WithValues("scan_init", namespacedName)
 
-	job, err := r.getJob(fmt.Sprintf("scan-%s", scan.Name), scan.Namespace)
+	jobs, err := r.getJobsForScan(scan, client.MatchingLabels{"experimental.securecodebox.io/job-type": "scanner"})
 	if err != nil {
 		return err
 	}
-	if job != nil {
+	if len(jobs.Items) > 0 {
 		log.V(8).Info("Job already exists. Doesn't need to be created.")
 		return nil
 	}
@@ -267,7 +280,7 @@ func (r *ScanReconciler) startScan(scan *executionv1.Scan) error {
 		rules,
 	)
 
-	job, err = r.constructJobForScan(scan, &scanType)
+	job, err := r.constructJobForScan(scan, &scanType)
 	if err != nil {
 		log.Error(err, "unable to create job object ScanType")
 		return err
@@ -296,7 +309,7 @@ func (r *ScanReconciler) startScan(scan *executionv1.Scan) error {
 func (r *ScanReconciler) checkIfScanIsCompleted(scan *executionv1.Scan) error {
 	ctx := context.Background()
 
-	status, err := r.checkIfJobIsCompleted(fmt.Sprintf("scan-%s", scan.Name), scan.Namespace)
+	status, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{"experimental.securecodebox.io/job-type": "scanner"})
 	if err != nil {
 		return err
 	}
@@ -326,11 +339,11 @@ func (r *ScanReconciler) startParser(scan *executionv1.Scan) error {
 	namespacedName := fmt.Sprintf("%s/%s", scan.Namespace, scan.Name)
 	log := r.Log.WithValues("scan_parse", namespacedName)
 
-	job, err := r.getJob(fmt.Sprintf("parse-%s", scan.Name), scan.Namespace)
+	jobs, err := r.getJobsForScan(scan, client.MatchingLabels{"experimental.securecodebox.io/job-type": "parser"})
 	if err != nil {
 		return err
 	}
-	if job != nil {
+	if len(jobs.Items) > 0 {
 		log.V(8).Info("Job already exists. Doesn't need to be created.")
 		return nil
 	}
@@ -384,7 +397,7 @@ func (r *ScanReconciler) startParser(scan *executionv1.Scan) error {
 	labels["experimental.securecodebox.io/job-type"] = "parser"
 	automountServiceAccountToken := true
 	var backOffLimit int32 = 3
-	job = &batch.Job{
+	job := &batch.Job{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: make(map[string]string),
 			Name:        fmt.Sprintf("parse-%s", scan.Name),
@@ -459,7 +472,7 @@ func (r *ScanReconciler) startParser(scan *executionv1.Scan) error {
 func (r *ScanReconciler) checkIfParsingIsCompleted(scan *executionv1.Scan) error {
 	ctx := context.Background()
 
-	status, err := r.checkIfJobIsCompleted(fmt.Sprintf("parse-%s", scan.Name), scan.Namespace)
+	status, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{"experimental.securecodebox.io/job-type": "parser"})
 	if err != nil {
 		return err
 	}
@@ -729,44 +742,13 @@ func (r *ScanReconciler) startReadOnlyHooks(scan *executionv1.Scan) error {
 	return nil
 }
 
-func allJobsCompleted(jobs *batch.JobList) jobCompletionType {
-	hasCompleted := true
-
-	for _, job := range jobs.Items {
-		if job.Status.Failed > 0 {
-			return failed
-		} else if job.Status.Succeeded == 0 {
-			hasCompleted = false
-		}
-	}
-
-	if hasCompleted {
-		return completed
-	}
-	return incomplete
-}
-
 func (r *ScanReconciler) checkIfReadOnlyHookIsCompleted(scan *executionv1.Scan) error {
 	ctx := context.Background()
-
-	// check if k8s job for scan was already created
-	var readOnlyHookJobs batch.JobList
-	if err := r.List(
-		ctx,
-		&readOnlyHookJobs,
-		client.InNamespace(scan.Namespace),
-		client.MatchingField(ownerKey, scan.Name),
-		client.MatchingLabels{
-			"experimental.securecodebox.io/job-type": "read-only-hook",
-		},
-	); err != nil {
-		r.Log.Error(err, "Unable to list child jobs")
+	readOnlyHookCompletion, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{"experimental.securecodebox.io/job-type": "read-only-hook"})
+	if err != nil {
 		return err
 	}
 
-	r.Log.V(9).Info("Got related jobs", "count", len(readOnlyHookJobs.Items))
-
-	readOnlyHookCompletion := allJobsCompleted(&readOnlyHookJobs)
 	if readOnlyHookCompletion == completed {
 		r.Log.V(7).Info("All ReadOnlyHooks have completed")
 		scan.Status.State = "Done"
@@ -1143,7 +1125,10 @@ func (r *ScanReconciler) executeReadAndWriteHooks(scan *executionv1.Scan) error
 		})
 		return err
 	case executionv1.InProgress:
-		jobStatus, err := r.checkIfJobIsCompleted(nonCompletedHook.JobName, scan.Namespace)
+		jobStatus, err := r.checkIfJobIsCompleted(scan, client.MatchingLabels{
+			"experimental.securecodebox.io/job-type":  "read-and-write-hook",
+			"experimental.securecodebox.io/hook-name": nonCompletedHook.HookName,
+		})
 		if err != nil {
 			r.Log.Error(err, "Failed to check job status for ReadAndWrite Hook")
 			return err