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

Add relaxed names to nodeSelector property resolution. #464

Merged

Conversation

corneil
Copy link
Contributor

@corneil corneil commented Jul 9, 2024

Fixes #462

@corneil corneil requested a review from cppwfs July 9, 2024 10:25

boolean hasGlobalNodeSelector = StringUtils.hasText(properties.getNodeSelector());
String nodeSelectorDeploymentProperty = deploymentProperties.getOrDefault(KubernetesDeployerProperties.KUBERNETES_DEPLOYMENT_NODE_SELECTOR, "");
for (String name : RelaxedNames.forCamelCase(KubernetesDeployerProperties.KUBERNETES_DEPLOYMENT_NODE_SELECTOR)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution will not work because KUBERNETES_DEPLOYMENT_NODE_SELECTOR does not follow the KubernetesDeployment prefix format of spring.cloud.deployer.kubernetes. It is currently spring.cloud.deployer.kubernetes.deployment.
The addition of the RelaxedNames does support when users use camel case instead of kabob case, but with the correct prefix.

Copy link
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!
But we need to update the prefix.

@cppwfs
Copy link
Contributor

cppwfs commented Jul 9, 2024

Closing this PR in light of the PR 466 issued by @corneil

@cppwfs cppwfs closed this Jul 9, 2024
@cppwfs
Copy link
Contributor

cppwfs commented Jul 9, 2024

Whoops closed the wrong PR !

@cppwfs cppwfs reopened this Jul 9, 2024
@corneil corneil requested a review from cppwfs July 9, 2024 13:35
@cppwfs cppwfs changed the title Add relaxed names to nodeSelector property resolution. [DO NOT MERGE till 2.9.5] Add relaxed names to nodeSelector property resolution. Jul 9, 2024
@cppwfs cppwfs added this to the 2.9.5 milestone Jul 9, 2024
@corneil corneil changed the title [DO NOT MERGE till 2.9.5] Add relaxed names to nodeSelector property resolution. Add relaxed names to nodeSelector property resolution. Jul 30, 2024
@corneil corneil merged commit 93edfc3 into spring-cloud:main Jul 30, 2024
4 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.

Node Selector doesn't get applied on K8s platforms
2 participants