Don't inject must-gather fields directly into job yaml (#138)

* Don't inject must-gather fields directly into job yaml

* Remove yaml template entirely

README.md

@@ -105,6 +105,5 @@ Using the [operator-sdk](https://github.com/operator-framework/operator-sdk), ru
 oc apply -f deploy/crds/managed.openshift.io_mustgathers_crd.yaml
 oc new-project must-gather-operator
 export DEFAULT_MUST_GATHER_IMAGE='quay.io/openshift/origin-must-gather:latest'
-export JOB_TEMPLATE_FILE_NAME=./build/templates/job.template.yaml
 OPERATOR_NAME=must-gather-operator operator-sdk run --verbose --local --namespace ''
 ```

build/Dockerfile

@@ -2,8 +2,7 @@ FROM quay.io/app-sre/boilerplate:image-v5.0.0 AS builder
 ENV OPERATOR=/usr/local/bin/must-gather-operator \
     OPERATOR_BIN=must-gather-operator \
     USER_UID=1001 \
-    USER_NAME=must-gather-operator \
-    JOB_TEMPLATE_FILE_NAME=/etc/templates/job.template.yaml
+    USER_NAME=must-gather-operator
 
 RUN mkdir /src
 
@@ -21,8 +20,7 @@ FROM registry.access.redhat.com/ubi8/ubi-minimal:8.10-896.1716497715
 ENV OPERATOR=/usr/local/bin/must-gather-operator \
     OPERATOR_BIN=must-gather-operator \
     USER_UID=1001 \
-    USER_NAME=must-gather-operator \
-    JOB_TEMPLATE_FILE_NAME=/etc/templates/job.template.yaml
+    USER_NAME=must-gather-operator
 
 RUN microdnf install tar gzip openssh-clients wget shadow-utils procps && \
     wget https://kojipkgs.fedoraproject.org/packages/sshpass/1.06/9.el8/x86_64/sshpass-1.06-9.el8.x86_64.rpm && \
@@ -32,7 +30,6 @@ RUN microdnf install tar gzip openssh-clients wget shadow-utils procps && \
 
 COPY --from=builder /src/build/_output/bin/${OPERATOR_BIN} /usr/local/bin/${OPERATOR_BIN}
 COPY --from=builder /src/build/bin /usr/local/bin
-COPY --from=builder /src/build/templates /etc/templates
 
 RUN /usr/local/bin/user_setup
 

controllers/mustgather/mustgather_controller.go

@@ -27,11 +27,9 @@ import (
 	"github.com/openshift/must-gather-operator/pkg/k8sutil"
 	"github.com/openshift/must-gather-operator/pkg/localmetrics"
 	"github.com/redhat-cop/operator-utils/pkg/util"
-	"github.com/redhat-cop/operator-utils/pkg/util/templates"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/types"
 	"os"
 	"reflect"
@@ -40,47 +38,16 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
-	"strings"
-	"text/template"
 )
 
-const ControllerName = "mustgather-controller"
+const (
+	ControllerName = "mustgather-controller"
 
-const templateFileNameEnv = "JOB_TEMPLATE_FILE_NAME"
-const defaultMustGatherNamespace = "openshift-must-gather-operator"
+	defaultMustGatherNamespace = "openshift-must-gather-operator"
+)
 
 var log = logf.Log.WithName(ControllerName)
 
-var jobTemplate *template.Template
-
-func initializeTemplate(clusterVersion string) (*template.Template, error) {
-	templateFileName, ok := os.LookupEnv(templateFileNameEnv)
-	if !ok {
-		templateFileName = "/etc/templates/job.template.yaml"
-	}
-	text, err := os.ReadFile(templateFileName)
-	if err != nil {
-		log.Error(err, "Error reading job template file", "filename", templateFileName)
-		return &template.Template{}, err
-	}
-	// Inject the operator image URI from the pod's env variables
-	operatorImage, varPresent := os.LookupEnv("OPERATOR_IMAGE")
-	if !varPresent {
-		err := goerror.New("Operator image environment variable not found")
-		log.Error(err, "Error: no operator image found for job template")
-		return &template.Template{}, err
-	}
-	// TODO: make these normal template parameters instead. This is ugly but works
-	str := strings.Replace(string(text), "THIS_STRING_WILL_BE_REPLACED_BUT_DONT_CHANGE_IT", operatorImage, 1)
-	str = strings.Replace(str, "MUST_GATHER_IMAGE_DONT_CHANGE", clusterVersion, 1)
-	jobTemplate, err := template.New("MustGatherJob").Parse(str)
-	if err != nil {
-		log.Error(err, "Error parsing template", "template", str)
-		return &template.Template{}, err
-	}
-	return jobTemplate, err
-}
-
 // blank assignment to verify that MustGatherReconciler implements reconcile.Reconciler
 var _ reconcile.Reconciler = &MustGatherReconciler{}
 
@@ -367,23 +334,21 @@ func (r *MustGatherReconciler) addFinalizer(reqLogger logr.Logger, m *mustgather
 	return nil
 }
 
-func (r *MustGatherReconciler) getJobFromInstance(instance *mustgatherv1alpha1.MustGather) (*unstructured.Unstructured, error) {
+func (r *MustGatherReconciler) getJobFromInstance(instance *mustgatherv1alpha1.MustGather) (*batchv1.Job, error) {
+	// Inject the operator image URI from the pod's env variables
+	operatorImage, varPresent := os.LookupEnv("OPERATOR_IMAGE")
+	if !varPresent {
+		err := goerror.New("Operator image environment variable not found")
+		log.Error(err, "Error: no operator image found for job template")
+		return nil, err
+	}
+
 	version, err := r.getClusterVersionForJobTemplate("version")
 	if err != nil {
 		return nil, fmt.Errorf("failed to get cluster version for job template: %w", err)
 	}
 
-	jobTemplate, err = initializeTemplate(version)
-	if err != nil {
-		log.Error(err, "unable to initialize job template")
-		return &unstructured.Unstructured{}, err
-	}
-	unstructuredJob, err := templates.ProcessTemplate(context.TODO(), instance, jobTemplate)
-	if err != nil {
-		log.Error(err, "unable to process", "template", jobTemplate, "with parameter", instance)
-		return &unstructured.Unstructured{}, err
-	}
-	return unstructuredJob, nil
+	return getJobTemplate(operatorImage, version, *instance), nil
 }
 
 func (r *MustGatherReconciler) getClusterVersionForJobTemplate(clusterVersionName string) (string, error) {

controllers/mustgather/mustgather_controller_test.go

@@ -3,7 +3,6 @@ package mustgather
 import (
 	"context"
 	configv1 "github.com/openshift/api/config/v1"
-	"os"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"testing"
 
@@ -24,8 +23,6 @@ import (
 )
 
 func TestMustGatherController(t *testing.T) {
-	os.Setenv("JOB_TEMPLATE_FILE_NAME", "../../../build/templates/job.template.yaml")
-
 	mgObj := createMustGatherObject()
 	secObj := createMustGatherSecretObject()