From 2549aebab3e928b37555f25e88670c1cfbfd6831 Mon Sep 17 00:00:00 2001 From: Alex Vulaj Date: Tue, 4 Jun 2024 12:08:40 -0400 Subject: [PATCH] Remove yaml template entirely --- README.md | 1 - build/Dockerfile | 6 +- build/templates/job.template.yaml | 74 ------- .../mustgather/mustgather_controller.go | 113 +--------- .../mustgather/mustgather_controller_test.go | 3 - controllers/mustgather/template.go | 199 ++++++++++++++++++ 6 files changed, 209 insertions(+), 187 deletions(-) delete mode 100644 build/templates/job.template.yaml create mode 100644 controllers/mustgather/template.go diff --git a/README.md b/README.md index d21bcab9..c03f0e05 100644 --- a/README.md +++ b/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 '' ``` diff --git a/build/Dockerfile b/build/Dockerfile index 21abb112..952c8e70 100644 --- a/build/Dockerfile +++ b/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 && \ diff --git a/build/templates/job.template.yaml b/build/templates/job.template.yaml deleted file mode 100644 index fd98ced3..00000000 --- a/build/templates/job.template.yaml +++ /dev/null @@ -1,74 +0,0 @@ -apiVersion: batch/v1 -kind: Job -spec: - template: - spec: - affinity: - nodeAffinity: - preferredDuringSchedulingIgnoredDuringExecution: - - preference: - matchExpressions: - - key: node-role.kubernetes.io/infra - operator: Exists - weight: 1 - tolerations: - - effect: NoSchedule - key: node-role.kubernetes.io/infra - operator: Exists - restartPolicy: OnFailure - shareProcessNamespace: true - containers: - - command: - - /bin/bash - - -c - image: quay.io/openshift/origin-must-gather:MUST_GATHER_IMAGE_DONT_CHANGE - name: gather - volumeMounts: - - mountPath: /must-gather - name: must-gather-output - - command: - - /bin/bash - - -c - - | - count=0 - until [ $count -gt 4 ] - do - while `pgrep -a gather > /dev/null` - do - echo "waiting for gathers to complete ..." - sleep 120 - count=0 - done - echo "no gather is running ($count / 4)" - ((count++)) - sleep 30 - done - /usr/local/bin/upload - image: THIS_STRING_WILL_BE_REPLACED_BUT_DONT_CHANGE_IT - name: upload - volumeMounts: - - mountPath: /must-gather - name: must-gather-output - - mountPath: /must-gather-upload - name: must-gather-upload - env: - - name: username - valueFrom: - secretKeyRef: - key: username - - name: password - valueFrom: - secretKeyRef: - key: password - - name: caseid - - name: must_gather_output - value: /must-gather - - name: must_gather_upload - value: /must-gather-upload - - name: internal_user - volumes: - - emptyDir: {} - name: must-gather-output - - emptyDir: {} - name: must-gather-upload - diff --git a/controllers/mustgather/mustgather_controller.go b/controllers/mustgather/mustgather_controller.go index 3ce38a8d..d0f32325 100644 --- a/controllers/mustgather/mustgather_controller.go +++ b/controllers/mustgather/mustgather_controller.go @@ -31,7 +31,6 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/yaml" "os" "reflect" ctrl "sigs.k8s.io/controller-runtime" @@ -39,55 +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" - "strconv" - "strings" ) const ( ControllerName = "mustgather-controller" - templateFileNameEnv = "JOB_TEMPLATE_FILE_NAME" defaultMustGatherNamespace = "openshift-must-gather-operator" - - gatherCommandBinaryAudit = "gather_audit_logs" - gatherCommandBinaryNoAudit = "gather" - gatherCommand = "\ntimeout %v bash -x -c -- '/usr/bin/%v'\n\nstatus=$?\nif [[ $status -eq 124 || $status -eq 137 ]]; then\n echo \"Gather timed out.\"\n exit 0\nfi" - - containerEnvNameUsername = "username" - containerEnvNamePassword = "password" - containerEnvNameCaseID = "caseid" - containerEnvNameInternalUser = "internal_user" - containerEnvNameHttpProxy = "http_proxy" - containerEnvNameHttpsProxy = "https_proxy" - containerEnvNameNoProxy = "no_proxy" ) var log = logf.Log.WithName(ControllerName) -func initializeTemplate(clusterVersion string) (string, 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 "", 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 "", 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) - return str, err -} - // blank assignment to verify that MustGatherReconciler implements reconcile.Reconciler var _ reconcile.Reconciler = &MustGatherReconciler{} @@ -375,77 +335,20 @@ func (r *MustGatherReconciler) addFinalizer(reqLogger logr.Logger, m *mustgather } func (r *MustGatherReconciler) getJobFromInstance(instance *mustgatherv1alpha1.MustGather) (*batchv1.Job, error) { - 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 nil, err - } - - job, err := processJobTemplate(jobTemplate, instance) - if err != nil { - log.Error(err, "unable to process job template") + // 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 } - return job, nil -} - -func processJobTemplate(jobTemplate string, instance *mustgatherv1alpha1.MustGather) (*batchv1.Job, error) { - job := &batchv1.Job{} - err := yaml.Unmarshal([]byte(jobTemplate), job) + version, err := r.getClusterVersionForJobTemplate("version") if err != nil { - return nil, fmt.Errorf("failed to umarshal job template: %w", err) - } - job.ObjectMeta.Name = instance.ObjectMeta.Name - job.ObjectMeta.Namespace = instance.ObjectMeta.Namespace - - var gatherCommandBinary string - if audit := instance.Spec.Audit; audit { - gatherCommandBinary = gatherCommandBinaryAudit - } else { - gatherCommandBinary = gatherCommandBinaryNoAudit - } - - job.Spec.Template.Spec.Containers[0].Command = append( - job.Spec.Template.Spec.Containers[0].Command, - fmt.Sprintf(gatherCommand, instance.Spec.MustGatherTimeout.Duration, gatherCommandBinary), - ) - - for i, env := range job.Spec.Template.Spec.Containers[1].Env { - switch env.Name { - case containerEnvNameUsername, containerEnvNamePassword: - env.ValueFrom.SecretKeyRef.Name = instance.Spec.CaseManagementAccountSecretRef.Name - case containerEnvNameCaseID: - env.Value = instance.Spec.CaseID - case containerEnvNameInternalUser: - env.Value = strconv.FormatBool(instance.Spec.InternalUser) - } - job.Spec.Template.Spec.Containers[1].Env[i] = env - } - - if httpProxy := instance.Spec.ProxyConfig.HTTPProxy; httpProxy != "" { - httpProxyEnvar := corev1.EnvVar{Name: containerEnvNameHttpProxy, Value: httpProxy} - job.Spec.Template.Spec.Containers[1].Env = append(job.Spec.Template.Spec.Containers[1].Env, httpProxyEnvar) - } - - if httpsProxy := instance.Spec.ProxyConfig.HTTPSProxy; httpsProxy != "" { - httpProxyEnvar := corev1.EnvVar{Name: containerEnvNameHttpsProxy, Value: httpsProxy} - job.Spec.Template.Spec.Containers[1].Env = append(job.Spec.Template.Spec.Containers[1].Env, httpProxyEnvar) - } - - if noProxy := instance.Spec.ProxyConfig.NoProxy; noProxy != "" { - noProxyEnvar := corev1.EnvVar{Name: containerEnvNameNoProxy, Value: noProxy} - job.Spec.Template.Spec.Containers[1].Env = append(job.Spec.Template.Spec.Containers[1].Env, noProxyEnvar) + return nil, fmt.Errorf("failed to get cluster version for job template: %w", err) } - job.Spec.Template.Spec.ServiceAccountName = instance.Spec.ServiceAccountRef.Name - - return job, nil + return getJobTemplate(operatorImage, version, *instance), nil } func (r *MustGatherReconciler) getClusterVersionForJobTemplate(clusterVersionName string) (string, error) { diff --git a/controllers/mustgather/mustgather_controller_test.go b/controllers/mustgather/mustgather_controller_test.go index 213b7b5e..2dd1ea15 100644 --- a/controllers/mustgather/mustgather_controller_test.go +++ b/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() diff --git a/controllers/mustgather/template.go b/controllers/mustgather/template.go new file mode 100644 index 00000000..e473944a --- /dev/null +++ b/controllers/mustgather/template.go @@ -0,0 +1,199 @@ +package mustgather + +import ( + "fmt" + "github.com/openshift/must-gather-operator/api/v1alpha1" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "strconv" + "time" +) + +const ( + infraNodeLabelKey = "node-role.kubernetes.io/infra" + outputVolumeName = "must-gather-output" + uploadVolumeName = "must-gather-upload" + volumeMountPath = "/must-gather" + volumeUploadMountPath = "/must-gather-upload" + + gatherCommandBinaryAudit = "gather_audit_logs" + gatherCommandBinaryNoAudit = "gather" + gatherCommand = "\ntimeout %v bash -x -c -- '/usr/bin/%v'\n\nstatus=$?\nif [[ $status -eq 124 || $status -eq 137 ]]; then\n echo \"Gather timed out.\"\n exit 0\nfi" + mustGatherImage = "quay.io/openshift/origin-must-gather" + gatherContainerName = "gather" + + uploadContainerName = "upload" + uploadEnvUsername = "username" + uploadEnvPassword = "password" + uploadEnvCaseId = "caseid" + uploadEnvInternalUser = "internal_user" + uploadEnvHttpProxy = "http_proxy" + uploadEnvHttpsProxy = "https_proxy" + uploadEnvNoProxy = "no_proxy" + uploadCommand = "count=0\nuntil [ $count -gt 4 ]\ndo\n while `pgrep -a gather > /dev/null`\n do\n echo \"waiting for gathers to complete ...\" \n sleep 120\n count=0\n done\n echo \"no gather is running ($count / 4)\"\n ((count++))\n sleep 30\ndone\n/usr/local/bin/upload\n" +) + +func getJobTemplate(operatorImage string, clusterVersion string, mustGather v1alpha1.MustGather) *batchv1.Job { + job := initializeJobTemplate(mustGather.Name, mustGather.Namespace, mustGather.Spec.ServiceAccountRef.Name) + + job.Spec.Template.Spec.Containers = append( + job.Spec.Template.Spec.Containers, + getGatherContainer(mustGather.Spec.Audit, mustGather.Spec.MustGatherTimeout.Duration, clusterVersion), + getUploadContainer( + operatorImage, + mustGather.Spec.CaseID, + mustGather.Spec.InternalUser, + mustGather.Spec.ProxyConfig.HTTPProxy, + mustGather.Spec.ProxyConfig.HTTPSProxy, + mustGather.Spec.ProxyConfig.NoProxy, + ), + ) + return job +} + +func initializeJobTemplate(name string, namespace string, serviceAccountRef string) *batchv1.Job { + return &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Affinity: &corev1.Affinity{ + NodeAffinity: &corev1.NodeAffinity{ + PreferredDuringSchedulingIgnoredDuringExecution: []corev1.PreferredSchedulingTerm{ + { + Preference: corev1.NodeSelectorTerm{ + MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: infraNodeLabelKey, + Operator: corev1.NodeSelectorOpExists, + }, + }, + }, + Weight: 1, + }, + }, + }, + }, + Tolerations: []corev1.Toleration{ + { + Effect: corev1.TaintEffectNoSchedule, + Key: infraNodeLabelKey, + Operator: corev1.TolerationOpExists, + }, + }, + RestartPolicy: corev1.RestartPolicyOnFailure, + ShareProcessNamespace: ToPtr(true), + Volumes: []corev1.Volume{ + { + Name: outputVolumeName, + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + }, + { + Name: uploadVolumeName, + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}}, + }, + }, + ServiceAccountName: serviceAccountRef, + }, + }, + }, + } +} + +func getGatherContainer(audit bool, timeout time.Duration, mustGatherImageVersion string) corev1.Container { + var commandBinary string + if audit { + commandBinary = gatherCommandBinaryAudit + } else { + commandBinary = gatherCommandBinaryNoAudit + } + + return corev1.Container{ + Command: []string{ + "/bin/bash", + "-c", + fmt.Sprintf(gatherCommand, timeout, commandBinary), + }, + Image: fmt.Sprintf("%v:%v", mustGatherImage, mustGatherImageVersion), + Name: gatherContainerName, + VolumeMounts: []corev1.VolumeMount{ + { + MountPath: volumeMountPath, + Name: outputVolumeName, + }, + }, + } +} + +func getUploadContainer( + operatorImage string, + caseId string, + internalUser bool, + httpProxy string, + httpsProxy string, + noProxy string, +) corev1.Container { + container := corev1.Container{ + Command: []string{ + "/bin/bash", + "-c", + uploadCommand, + }, + Image: operatorImage, + Name: uploadContainerName, + VolumeMounts: []corev1.VolumeMount{ + { + MountPath: volumeMountPath, + Name: outputVolumeName, + }, + { + MountPath: volumeUploadMountPath, + Name: uploadVolumeName, + }, + }, + Env: []corev1.EnvVar{ + { + Name: uploadEnvUsername, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + Key: uploadEnvUsername, + }, + }, + }, + { + Name: uploadEnvPassword, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + Key: uploadEnvPassword, + }, + }, + }, + { + Name: uploadEnvCaseId, + Value: caseId, + }, + { + Name: uploadEnvInternalUser, + Value: strconv.FormatBool(internalUser), + }, + }, + } + + if httpProxy != "" { + container.Env = append(container.Env, corev1.EnvVar{Name: uploadEnvHttpProxy, Value: httpProxy}) + } + if httpsProxy != "" { + container.Env = append(container.Env, corev1.EnvVar{Name: uploadEnvHttpsProxy, Value: httpsProxy}) + } + if noProxy != "" { + container.Env = append(container.Env, corev1.EnvVar{Name: uploadEnvNoProxy, Value: noProxy}) + } + + return container +} + +func ToPtr[T any](t T) *T { return &t }