Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.14.0 release cannot use jobTargetRef template metadata labels #5752

Closed
somehowadev opened this issue Apr 26, 2024 · 9 comments · Fixed by kedacore/charts#632
Closed

2.14.0 release cannot use jobTargetRef template metadata labels #5752

somehowadev opened this issue Apr 26, 2024 · 9 comments · Fixed by kedacore/charts#632
Labels
bug Something isn't working

Comments

@somehowadev
Copy link

somehowadev commented Apr 26, 2024

Report

In 2.13.1, we were using a scaledJob as so:

spec: jobTargetRef: backoffLimit: 1 template: metadata: labels: azure.workload.identity/use: "true"

Which worked fine. However since 2.14.0, the same configuration throws the following error:
Error when applying patch: Strict decoding error: unknown field "spec.jobTargetRef.template.metadata.labels"

Downgrading to 2.13.1 resolves the error.

Expected Behavior

Patch is applied as expected, pod controlled by the job has the relevant Azure workload identity label.

Actual Behavior

Patch isn't applied, field is ignored.

Steps to Reproduce the Problem

  1. Create scaled job with a job template containing labels.
  2. Try to patch/apply

Logs from KEDA operator

No response

KEDA Version

2.14.0

Kubernetes Version

1.28

Platform

Microsoft Azure

Scaler Details

Azure Service Bus

Anything else?

No response

@somehowadev somehowadev added the bug Something isn't working label Apr 26, 2024
@somehowadev
Copy link
Author

Perhaps related to #5739?

@JorTurFer
Copy link
Member

Thanks for reporting, are you using helm to install KEDA or the raw manifests? I assume that you see the error during the manifest apply and not in the operator, right?

@jan-mrm
Copy link

jan-mrm commented Apr 27, 2024

We are seeing that behaviour as well. We install KEDA using helm. When applying a ScaledJob with
spec.jobTargetRef.template.metadata.labels: azure.workload.identity/use: "true" and all other labels, the labels are not visible in the live manifest of the ScaledJob and also not set on the workload created by the ScaledJob.

Diffing the ScaledJob definition (CRD) between 2.13.2 (KEDA chart version) and 2.14.0 (KEDA chart version) it looks like the x-kubernetes-preserve-unknown-fields: true gets removed from spec.properties.jobTargetRef.template.properties.metadata. Corresponding pull request would be the release of the chart kedacore/charts#630 line 366 (https://github.com/kedacore/charts/pull/630/files#diff-fb31a1b26a9bfa7250f0fe722048b4ec227dc17805eae2cf492793367398de4bL366). Looks like that introduces the issue, since labels are not defined?

Should we open another issue in the charts repository, I'm guessing?

@JorTurFer
Copy link
Member

Interesting... We don't generate the CRDs manually but self generating it, so probably some behaviour has changed in upstream deps and we will need to cover it also by code (chart CRDs are just a port from this repo). Are you willing to take a look?

@jan-mrm
Copy link

jan-mrm commented Apr 28, 2024

I can try to figure something out today. I'll take a further look at it in a bit.

Just quickly saw that this seems to be patched in

## needs to be patched because of an issue with controller-gen, which is not including anything in metadata from nested contructs

@JorTurFer
Copy link
Member

You are right! I have just noticed that I could copy the CRD from the wrong place during the chart upgrade.
Probably I used the CRDs folder instead of the release process generated yaml.
Are the raw yamls correct? They can be downloaded from releases page in this repo

@jan-mrm
Copy link

jan-mrm commented Apr 28, 2024

Yes, I think, the ScaledJob looks right downloaded from the release.

@JorTurFer
Copy link
Member

Yeah, I was to post here the fix xD
kedacore/charts#632

Thanks for the review! I used the wrong manifests when I was upgrading the helm chart, sorry

@jan-mrm
Copy link

jan-mrm commented Apr 28, 2024

Happens 🙂
Great, thanks for fixing it 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants