diff --git a/cloudstack.go b/cloudstack.go index 8d78a861..b7b019a5 100644 --- a/cloudstack.go +++ b/cloudstack.go @@ -34,7 +34,7 @@ import ( ) // ProviderName is the name of this cloud provider. -const ProviderName = "external-cloudstack" +const ProviderName = "cloudstack" // CSConfig wraps the config for the CloudStack cloud provider. type CSConfig struct { @@ -199,13 +199,18 @@ func (cs *CSCloud) GetZone(ctx context.Context) (cloudprovider.Zone, error) { func (cs *CSCloud) GetZoneByProviderID(ctx context.Context, providerID string) (cloudprovider.Zone, error) { zone := cloudprovider.Zone{} + id, _, err := instanceIDFromProviderID(providerID) + if err != nil { + return zone, err + } + instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByID( - providerID, + id, cloudstack.WithProject(cs.projectID), ) if err != nil { if count == 0 { - return zone, fmt.Errorf("could not find node by ID: %v", providerID) + return zone, fmt.Errorf("could not find node by ID: %v", id) } return zone, fmt.Errorf("error retrieving zone: %v", err) } diff --git a/cloudstack_instances.go b/cloudstack_instances.go index 91d65751..a1a73420 100644 --- a/cloudstack_instances.go +++ b/cloudstack_instances.go @@ -23,17 +23,13 @@ import ( "context" "errors" "fmt" - "regexp" "github.com/apache/cloudstack-go/v2/cloudstack" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" cloudprovider "k8s.io/cloud-provider" - "k8s.io/klog/v2" ) -var labelInvalidCharsRegex *regexp.Regexp = regexp.MustCompile(`([^A-Za-z0-9][^-A-Za-z0-9_.]*)?[^A-Za-z0-9]`) - // NodeAddresses returns the addresses of the specified instance. func (cs *CSCloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]corev1.NodeAddress, error) { instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByName( @@ -52,8 +48,13 @@ func (cs *CSCloud) NodeAddresses(ctx context.Context, name types.NodeName) ([]co // NodeAddressesByProviderID returns the addresses of the specified instance. func (cs *CSCloud) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]corev1.NodeAddress, error) { + id, _, err := instanceIDFromProviderID(providerID) + if err != nil { + return nil, err + } + instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByID( - providerID, + id, cloudstack.WithProject(cs.projectID), ) if err != nil { @@ -81,10 +82,6 @@ func (cs *CSCloud) nodeAddresses(instance *cloudstack.VirtualMachine) ([]corev1. if instance.Publicip != "" { addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeExternalIP, Address: instance.Publicip}) - } else { - // Since there is no sane way to determine the external IP if the host isn't - // using static NAT, we will just fire a log message and omit the external IP. - klog.V(4).Infof("Could not determine the public IP of host %v (%v)", instance.Name, instance.Id) } return addresses, nil @@ -119,13 +116,18 @@ func (cs *CSCloud) InstanceType(ctx context.Context, name types.NodeName) (strin return "", fmt.Errorf("error retrieving instance type: %v", err) } - return labelInvalidCharsRegex.ReplaceAllString(instance.Serviceofferingname, ``), nil + return sanitizeLabel(instance.Serviceofferingname), nil } // InstanceTypeByProviderID returns the type of the specified instance. func (cs *CSCloud) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) { + id, _, err := instanceIDFromProviderID(providerID) + if err != nil { + return "", err + } + instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByID( - providerID, + id, cloudstack.WithProject(cs.projectID), ) if err != nil { @@ -135,7 +137,7 @@ func (cs *CSCloud) InstanceTypeByProviderID(ctx context.Context, providerID stri return "", fmt.Errorf("error retrieving instance type: %v", err) } - return labelInvalidCharsRegex.ReplaceAllString(instance.Serviceofferingname, ``), nil + return sanitizeLabel(instance.Serviceofferingname), nil } // AddSSHKeyToAllInstances is currently not implemented. @@ -150,8 +152,13 @@ func (cs *CSCloud) CurrentNodeName(ctx context.Context, hostname string) (types. // InstanceExistsByProviderID returns if the instance still exists. func (cs *CSCloud) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) { + id, _, err := instanceIDFromProviderID(providerID) + if err != nil { + return false, err + } + _, count, err := cs.client.VirtualMachine.GetVirtualMachineByID( - providerID, + id, cloudstack.WithProject(cs.projectID), ) if err != nil { @@ -166,7 +173,22 @@ func (cs *CSCloud) InstanceExistsByProviderID(ctx context.Context, providerID st // InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes func (cs *CSCloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { - return false, cloudprovider.NotImplemented + id, _, err := instanceIDFromProviderID(providerID) + if err != nil { + return false, err + } + + instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByID( + id, + cloudstack.WithProject(cs.projectID), + ) + if err != nil { + if count == 0 { + return false, cloudprovider.InstanceNotFound + } + return false, fmt.Errorf("error retrieving instance state: %v", err) + } + return instance != nil && instance.State == "Stopped", nil } func (cs *CSCloud) InstanceExists(ctx context.Context, node *corev1.Node) (bool, error) { @@ -180,31 +202,36 @@ func (cs *CSCloud) InstanceExists(ctx context.Context, node *corev1.Node) (bool, } func (cs *CSCloud) InstanceShutdown(ctx context.Context, node *corev1.Node) (bool, error) { - return false, cloudprovider.NotImplemented + return cs.InstanceShutdownByProviderID(ctx, node.Spec.ProviderID) } func (cs *CSCloud) InstanceMetadata(ctx context.Context, node *corev1.Node) (*cloudprovider.InstanceMetadata, error) { - - instanceType, err := cs.InstanceType(ctx, types.NodeName(node.Name)) + id, region, err := instanceIDFromProviderID(node.Spec.ProviderID) if err != nil { return nil, err } - addresses, err := cs.NodeAddresses(ctx, types.NodeName(node.Name)) + instance, count, err := cs.client.VirtualMachine.GetVirtualMachineByID( + id, + cloudstack.WithProject(cs.projectID), + ) if err != nil { - return nil, err + if count == 0 { + return nil, cloudprovider.InstanceNotFound + } + return nil, fmt.Errorf("error retrieving instance: %v", err) } - zone, err := cs.GetZone(ctx) + addresses, err := cs.nodeAddresses(instance) if err != nil { return nil, err } return &cloudprovider.InstanceMetadata{ - ProviderID: cs.ProviderName(), - InstanceType: instanceType, + ProviderID: node.Spec.ProviderID, + InstanceType: sanitizeLabel(instance.Serviceofferingname), NodeAddresses: addresses, - Zone: cs.zone, - Region: zone.Region, + Zone: sanitizeLabel(instance.Zonename), + Region: region, }, nil } diff --git a/util.go b/util.go new file mode 100644 index 00000000..a9adc855 --- /dev/null +++ b/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 + `://?([^/]*)/([^/]+)$`) + +// instanceIDFromProviderID splits a provider's id and return instanceID. +// A providerID is build out of '${ProviderName}:///${instance-id}' which contains ':///'. +// or '${ProviderName}://${region}/${instance-id}' which contains '://'. +// See cloudprovider.GetInstanceProviderID and Instances.InstanceID. +func instanceIDFromProviderID(providerID string) (instanceID string, region string, err error) { + + // https://github.com/kubernetes/kubernetes/issues/85731 + if providerID != "" && !strings.Contains(providerID, "://") { + providerID = ProviderName + "://" + providerID + } + + matches := providerIDRegexp.FindStringSubmatch(providerID) + if len(matches) != 3 { + return "", "", fmt.Errorf("ProviderID \"%s\" didn't match expected format \"cloudstack://region/InstanceID\"", providerID) + } + return matches[2], matches[1], nil +} + +// Sanitize label value so it complies with https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set +// Anything but [-A-Za-z0-9_.] will get converted to '_' +func sanitizeLabel(value string) string { + fn := func(r rune) rune { + if r >= 'a' && r <= 'z' || + r >= 'A' && r <= 'Z' || + r >= '0' && r <= '9' || + r == '-' || r == '_' || r == '.' { + return r + } + return '_' + } + value = strings.Map(fn, value) + + // Must start & end with alphanumeric char + value = strings.Trim(value, "-_.") + + // Strip anything over 63 chars + if len(value) > 63 { + value = value[:63] + value = strings.Trim(value, "-_.") + } + + return value +}