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

fix parenthesis preventing keda ScaledObject creation #13183

Merged
merged 1 commit into from
Dec 21, 2020

Conversation

dstandish
Copy link
Contributor

@dstandish dstandish commented Dec 19, 2020

fix placement of parentheses in keda autoscaler template so that keda scaledobject is created when enabled

adds simple test to verify that keda object is created under right circumstances (though not all permutations covered)

to do so, had to make schema validation optional cus the existing validator could not find schema. maybe this is cus it's a custom resource def? @dimberman maybe you know?

closes: #13181

@boring-cyborg boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Dec 19, 2020
@mik-laj
Copy link
Member

mik-laj commented Dec 20, 2020

to do so, had to make schema validation optional cus the existing validator could not find schema. maybe this is cus it's a custom resource def? @dimberman maybe you know?

Disabling schema validation only for a few exceptional cases, I think is a good solution. We will have to be more careful in our reviews, but if it's not frequent, that's not a problem.

If this is a problem then we will have to implement support for CRDs that have JSON Schema specifications, but this is an extra effort. We can do it later if needed.
https://github.com/kedacore/keda/blob/main/config/crd/bases/keda.sh_scaledobjects.yaml

@dstandish dstandish force-pushed the fix-keda-scaledobject-template branch from fd7c03f to ed3e36d Compare December 20, 2020 06:10
@dstandish
Copy link
Contributor Author

dstandish commented Dec 20, 2020

@mik-laj thanks for taking a look

i realized i had forgotten to commit the tests I added! (just pushed)

previously there were no tests related to keda (which is how this bug was able to stay hidden)

so with this change at least we test the "should we create keda objects" condition.

@mik-laj mik-laj merged commit a9d562e into apache:master Dec 21, 2020
potiuk pushed a commit that referenced this pull request Dec 23, 2020
kaxil pushed a commit that referenced this pull request Jan 21, 2021
kaxil pushed a commit that referenced this pull request Jan 21, 2021
kaxil pushed a commit that referenced this pull request Jan 22, 2021
kaxil pushed a commit that referenced this pull request Jan 22, 2021
cognifloyd pushed a commit to cognifloyd/stackstorm-k8s that referenced this pull request Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:helm-chart Airflow Helm Chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

keda scaledobject not created even though keda enabled in helm config
2 participants