add ScaledJob's label to its job

[AIchovy]

Signed-off-by: mosesyou [email protected]

This PR is about to add ScaledJob's label to its job.

Checklist

• Commits are signed with Developer Certificate of Origin (DCO)
• Tests have been added
• A PR is opened to update the documentation on https://github.com/kedacore/keda-docs
• Changelog has been updated

Fixes #1311

[zroubalik]

LGTM

[zroubalik]

@mosesyou could you please add this feature to the Changelog? to the Improvements section in Unreleased, thanks!

[AIchovy]

@mosesyou could you please add this feature to the Changelog? to the Improvements section in Unreleased, thanks!

Sure

[erichorwath]

@zroubalik I have big interest in this PR and tested it, here are my findings:

1. With this PR labels can be added to the "jobs" but not to the "pods" created by the job (see pictures). Was this intended?
2. Annotations are still not working :( This would be needed to disable istio, see here: Using Istio with CronJobs istio/istio#11659 (comment) (this annotation is needed in the pod spec but not in the job spec)

[zroubalik]

@ericbottard sad to hear that. Thanks for the confirmation! We should fix this. Could you please open an issue with the details. And are you willing to contribute this? Or @mosesyou do you want to take a look?

[erichorwath]

@zroubalik Can you maybe just reopen #1311 since the issue described there still exists?

[ycabrer]

Hi, I would like to help if I can.

I took a look at the code and it seems to do a deep copy of the scaledjob spec.template to job spec.template on creation.

If we want a label or annotation to appear in the pod, wouldn't we would add it in the scaledjob spec.template metadata?

@erichorwath would you provide a snip of your scaledjob? To help me better understand the issue.

[erichorwath]

@ycabrer sure, the interesting part is here spec.template.metadata part. It would be great to be able to set this in an scaledJob yaml (-> spec.jobTargetRef.template.spec.metadata).

apiVersion: batch/v1
kind: Job
metadata:
  name: redis-job
spec:
  activeDeadlineSeconds: 96000
  backoffLimit: 0
  completions: 1
  parallelism: 1
  template:
    metadata:
      labels:
        my-workload: app
      annotations:
        sidecar.istio.io/inject: "false"
    spec:
      containers:
      - image: busybox
        imagePullPolicy: IfNotPresent
        name: redis-worker
        env: []
        ports:
        - containerPort: 8080
          name: http
          protocol: TCP
        resources: {}
        volumeMounts:
        - mountPath: /data
          name: worker-data
      imagePullSecrets:
      - name: regcred
      restartPolicy: Never
      terminationGracePeriodSeconds: 259200
      volumes:
      - emptyDir: {}
        name: worker-data

[AIchovy]

keda/pkg/scaling/executor/scale_jobs.go

Lines 83 to 90 in af5254d

	job := &batchv1.Job{
	ObjectMeta: metav1.ObjectMeta{
	GenerateName: scaledJob.GetName() + "-",
	Namespace: scaledJob.GetNamespace(),
	Labels: labels,
	},
	Spec: *scaledJob.Spec.JobTargetRef.DeepCopy(),
	}

I'm not sure, Spec: *scaledJob.Spec.JobTargetRef.DeepCopy(), Doesn't this line of code mean copy ScaledJob's Job Spec including PodTemplate to Job? So why we can't pass labels or annotations from ScaledJob's JobSpec{PodTemplate{ObjectMeta}} to Job{PodTemplate{ObjectMeta}}?

[julienperignon]

Hi there

I know this has been merged in and apparently works for labels but we still cannot add annotations on the PodSpec (to disable istio injection for example). Do we have a rough idea of when this feature will be brought back in Keda ?

Cheers

[tomkerkhove]

@julienperignon I've opened #1627 for it.