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

[tempo-distributed] Removing Job Selector Config #3119

Merged
merged 1 commit into from
May 7, 2024

Conversation

jordan-simonovski
Copy link
Contributor

@jordan-simonovski jordan-simonovski commented May 6, 2024

Many apologies! This is on me for not properly verifying the Jobs prior to raising my previous PR.

Templating with a config like the following

  selector:
  template:
    metadata:
      labels:
        app.kubernetes.io/component: tokengen-job-2

will throw the error:

selector` does not match template `labels`, spec.selector: Invalid value: "null": field is immutable

On the other hand, adding in the selector labels will also fail as Kubernetes will throw the following error:

 `selector` not auto-generated

I've given this a much more rigorous test, and apologies for my previous bad change 🙏

The Selector should not be manually specified unless otherwise configured in spec. Removing the selector altogether ensures the job configuration is valid, while also allowing for templating without null values causing errors.

@kvrhdn
Copy link
Member

kvrhdn commented May 7, 2024

This is a revert of #3111 right?

@zalegrala
Copy link
Contributor

The chart version will need a bump and the docs need an update.

@jordan-simonovski
Copy link
Contributor Author

This is a revert of #3111 right?

It's not so much a revert as much as it is just removing the whole selector key from the config.
We could revert and keep selector: but this still seems to trigger the issue when templating which throws Invalid value: "null"

I have verified that upon removing this from the template that Kube will still add the selector config with the auto-generated selector value.

…ured in spec. Removing the selector altogether ensures the job configuration is valid, while also allowing for templating without null values causing errors

Signed-off-by: Jordan Simonovski <jsimonovski@atlassian.com>
@zalegrala
Copy link
Contributor

Thanks for coming back. I've rebased bumped the chart version for the PR.

@zalegrala zalegrala merged commit 9dd80e5 into grafana:main May 7, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants