Fix providerID handling + label sanitizing #46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- convert providerID to instanceID where needed - rename provider from external-cloudstack to cloudstack - implement InstanceShutdown and InstanceShutdownByProviderID
|
I realize that these changes should probably be reflected in the README. Will address that shortly. |
|
@hrak @vishesh92 |
|
I am seeing the below errors after applying the changes. I am not sure about the root cause. E0606 13:02:31.408425 1 node_lifecycle_controller.go:149] error checking if node k8s-1273-node-18f3588c91c exists: ProviderID "cloudstack://2d35066b-a8e4-4c2a-b400-256db0a7d2cd" didn't match expected format "cloudstack://region/InstanceID"
E0606 13:02:36.515139 1 node_lifecycle_controller.go:149] error checking if node k8s-1273-node-18f3588c91c exists: ProviderID "cloudstack://2d35066b-a8e4-4c2a-b400-256db0a7d2cd" didn't match expected format "cloudstack://region/InstanceID"
E0606 13:02:37.025198 1 node_controller.go:249] Error getting instance metadata for node addresses: ProviderID "" didn't match expected format "cloudstack://region/InstanceID"
E0606 13:02:37.025248 1 node_controller.go:249] Error getting instance metadata for node addresses: ProviderID "" didn't match expected format "cloudstack://region/InstanceID"
E0606 13:02:37.025255 1 node_controller.go:249] Error getting instance metadata for node addresses: ProviderID "" didn't match expected format "cloudstack://region/InstanceID"
E0606 13:02:37.025261 1 node_controller.go:249] Error getting instance metadata for node addresses: ProviderID "" didn't match expected format "cloudstack://region/InstanceID"
E0606 13:02:37.025267 1 node_controller.go:249] Error getting instance metadata for node addresses: ProviderID "" didn't match expected format "cloudstack://region/InstanceID"
E0606 13:02:41.584449 1 node_lifecycle_controller.go:149] error checking if node k8s-1273-node-18f3588c91c exists: ProviderID "cloudstack://2d35066b-a8e4-4c2a-b400-256db0a7d2cd" didn't match expected format "cloudstack://region/InstanceID"
E0606 13:02:46.648644 1 node_lifecycle_controller.go:149] error checking if node k8s-1273-node-18f3588c91c exists: ProviderID "cloudstack://2d35066b-a8e4-4c2a-b400-256db0a7d2cd" didn't match expected format "cloudstack://region/InstanceID" |
@vishesh92 did you change the provider name from external-cloudstack to cloudstack? |
|
Yes. I did change that. Before that pod was crashing due to wrong provider name. |
According to the pr, the providerID should have 3 slashes instead of 2. cloudstack://2d35066b should be cloudstack:///2d35066b |
vishesh92
reviewed
Jun 6, 2024
Co-authored-by: Vishesh <[email protected]>
|
I tested the above PR with kuberentes 1.28. It doesn't seem to be adding the zone or region labels. I assume it's because this needs some more changes in the implementation of |
weizhouapache
force-pushed
the
main
branch
2 times, most recently
from
97003ba to
0dc33dd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes the way the CloudStack Cloud Controller Manager handles kubelet providerID into a more standardized way that is more common around several other CCM's like the Openstack or vSphere one. The changes were needed to make node labels work again.
The providerID is configured by either setting the kubelet command line flag
--provider-id(deprecated) or using theproviderIDsetting in kubelet config yaml. The format of the value is<providername>://region/instance-idso in case of CloudStack for a platform without region f.e. :cloudstack:///4e7689bc-99ea-43d8-8c37-5ff511c01665. With the region being parsed from the providerID, this PR also addresses #39It also implements the two interface methods
InstanceShutdownByProviderIDandInstanceShutdown.And it fixes the way node labels are sanitized. The old regex approach would in some cases strip off allowed characters, for example zone name
Development-Internalwould turn intoDevelopmentInternal, although the-is allowed in label values. The new approach converts all chars that are not allowed in a label value to underscores:Development-Internal->Development-InternalSmall Instance (4 GB / 2 CPU)->Small_Instance__4_GB___2_CPU