You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by "hrak (via GitHub)" <gi...@apache.org> on 2023/05/05 06:23:42 UTC
[GitHub] [cloudstack-kubernetes-provider] hrak opened a new pull request, #46: Fix providerID handling + label sanitizing
hrak opened a new pull request, #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46
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 the `providerID` setting in [kubelet config yaml](https://kubernetes.io/docs/reference/config-api/kubelet-config.v1beta1/). The format of the value is `<providername>://region/instance-id` so in case of CloudStack for a platform without region f.e. : `cloudstack:///4e7689bc-99ea-43d8-8c37-5ff511c01665`.
It also implements the two interface methods `InstanceShutdownByProviderID` and `InstanceShutdown`.
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-Internal` would turn into `DevelopmentInternal`, 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-Internal`
`Small Instance (4 GB / 2 CPU)` -> `Small_Instance__4_GB___2_CPU`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix providerID handling + label sanitizing [cloudstack-kubernetes-provider]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46#discussion_r1629766415
##########
util.go:
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package cloudstack
+
+import (
+ "fmt"
+ "regexp"
+ "strings"
+)
+
+// If Instances.InstanceID or cloudprovider.GetInstanceProviderID is changed, the regexp should be changed too.
+var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `://([^/]*)/([^/]+)$`)
Review Comment:
```suggestion
var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `://?([^/]*)/([^/]+)$`)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack-kubernetes-provider] hrak commented on pull request #46: Fix providerID handling + label sanitizing
Posted by "hrak (via GitHub)" <gi...@apache.org>.
hrak commented on PR #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46#issuecomment-1535771035
I realize that these changes should probably be reflected in the README. Will address that shortly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix providerID handling + label sanitizing [cloudstack-kubernetes-provider]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46#issuecomment-2135116093
@hrak @vishesh92
it looks this requires some other changes like
https://github.com/Leaseweb/cloudstack-kubernetes-provider/commit/323b493f9d35dc0b6c0f6c0910e73043aac1ded6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix providerID handling + label sanitizing [cloudstack-kubernetes-provider]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46#issuecomment-2152641282
> I am seeing the below errors after applying the changes. I am not sure about the root cause.
> I launched a CKS cluster and updated the image.
> ```go
> 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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix providerID handling + label sanitizing [cloudstack-kubernetes-provider]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46#issuecomment-2152411619
I am seeing the below errors after applying the changes. I am not sure about the root cause.
I launched a CKS cluster and updated the image.
```go
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"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix providerID handling + label sanitizing [cloudstack-kubernetes-provider]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46#issuecomment-2154597439
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 `InstancesV2` interface. Moving this PR to milestone for next release for now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix providerID handling + label sanitizing [cloudstack-kubernetes-provider]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46#issuecomment-2152665390
> 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix providerID handling + label sanitizing [cloudstack-kubernetes-provider]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #46:
URL: https://github.com/apache/cloudstack-kubernetes-provider/pull/46#issuecomment-2152646446
Yes. I did change that. Before that pod was crashing due to wrong provider name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org