feat: reject ScaledObject creation and update when the name exceeds 63 characters#7762
Conversation
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Thank you for your contribution! 🙏 Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected. While you are waiting, make sure to:
Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient. Learn more about our contribution guide. |
|
/run-e2e passed tests: 174Execution of tests/secret-providers/azure_keyvault_workload_identity/azure_keyvault_workload_identity_test.go, has passed after "one" attempts failed tests: 18Execution of tests/scalers/redis/redis_cluster_lists/redis_cluster_lists_test.go, has failed after "three" attempts |
|
/run-e2e redis* passed tests: 1failed tests: 12 |
|
/run-e2e cassandra|arangodb|solr|nsq|nats_jetstream*|github* passed tests: 7failed tests: 0 |
|
The tests suddenly seem quite flaky. I am curious if we merged something that apparently requires more time somewhere, causing it to just miss completing its actions within the set time. |
JorTurFer
left a comment
JorTurFer
left a comment
There was a problem hiding this comment.
I have created this issue for a potential removal of keda-hpa in the future .
In the meantime, I think that this is better than silently failing, but I'm not sure about enabling it as default, this can be a potential breaking change, can't?
I'm not sure. @wozniakjan mentioned in #6998 (comment)
Having seen that, perhaps a piece of documentation would be good? |
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
rickbrouwer
added
the
Awaiting/2nd-approval
I still hold this opinion, willing to discuss alternatives though :)
very frequently those flakes have similar patterns - that some bigger "infra" Pods (mysql, redis, etc.) are unable to schedule on the three cluster node due to insufficient space. I'm passively looking into this and I would like to add better visibility into the cluster failures. |
|
/run-e2e internal passed tests: 34failed tests: 0 |
in theory, we should NOT set resources for workloads during e2e test to avoid scheduling issues. Even if SQL Server (as example) uses X CPU and Y RAM for operating, our e2e are light enough to not set them. Some time ago I visited all the e2e tests and removed all the requests, but maybe there are new charts setting them. We can spin up more nodes for e2e tests, but I'd say that just removing the pod request should be enough |
|
Docs: kedacore/keda-docs#1774 |
rickbrouwer
changed the title
…3 characters Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
|
/run-e2e internal passed tests: 34failed tests: 0 |
rickbrouwer
added
ok-to-merge
|
/run-e2e scaled_object_validation passed tests: 1failed tests: 0 |
3 participants
Adds admission-webhook validation that rejects ScaledObject creation when the name is longer than 54 characters. The generated HPA name is
keda-hpa-<so.Name>, which must itself be https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-label-names and is also used as ahttps://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set. Both capped at 63 characters by Kubernetes. The ScaledObject name therefore has to stay within 63 (minus "keda-hpa-" = 54).
Kubernetes itself enforces this kind of static name validation purely at admission and not in controllers, so, that's why i want to follow the same pattern.
Checklist
Fixes #6998
Docs: kedacore/keda-docs#1774