diff --git a/go.mod b/go.mod index 21c9149fae..2bb58d8c49 100644 --- a/go.mod +++ b/go.mod @@ -46,6 +46,7 @@ require ( // the below fixes the "go list -m all" execution replace ( k8s.io/cluster-bootstrap => k8s.io/cluster-bootstrap v0.31.1 + k8s.io/cri-client => k8s.io/cri-client v0.31.1 k8s.io/dynamic-resource-allocation => k8s.io/dynamic-resource-allocation v0.31.1 k8s.io/endpointslice => k8s.io/endpointslice v0.31.1 k8s.io/kube-aggregator => k8s.io/kube-aggregator v0.31.1 diff --git a/pkg/openstack/instances.go b/pkg/openstack/instances.go index 8ccb3e5005..f9b617736f 100644 --- a/pkg/openstack/instances.go +++ b/pkg/openstack/instances.go @@ -1,5 +1,5 @@ /* -Copyright 2016 The Kubernetes Authors. +Copyright 2024 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,132 +17,45 @@ limitations under the License. package openstack import ( - "bytes" "context" "fmt" - "net" sysos "os" "regexp" - "slices" - "sort" "strings" "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/flavors" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" - "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/networks" - neutronports "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports" + "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports" "github.com/gophercloud/gophercloud/v2/pagination" - "github.com/mitchellh/mapstructure" - v1 "k8s.io/api/core/v1" - "k8s.io/klog/v2" - "k8s.io/apimachinery/pkg/types" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/validation" cloudprovider "k8s.io/cloud-provider" "k8s.io/cloud-provider-openstack/pkg/client" "k8s.io/cloud-provider-openstack/pkg/metrics" + "k8s.io/cloud-provider-openstack/pkg/util" "k8s.io/cloud-provider-openstack/pkg/util/errors" - "k8s.io/cloud-provider-openstack/pkg/util/metadata" + "k8s.io/klog/v2" +) + +const ( + RegionalProviderIDEnv = "OS_CCM_REGIONAL" + instanceShutoff = "SHUTOFF" ) -// Instances encapsulates an implementation of Instances for OpenStack. -type Instances struct { +// InstancesV2 encapsulates an implementation of InstancesV2 for OpenStack. +type InstancesV2 struct { compute *gophercloud.ServiceClient network *gophercloud.ServiceClient region string regionProviderID bool - opts metadata.Opts networkingOpts NetworkingOpts } -const ( - instanceShutoff = "SHUTOFF" - RegionalProviderIDEnv = "OS_CCM_REGIONAL" - noSortPriority = 0 -) - -var _ cloudprovider.Instances = &Instances{} - -// buildAddressSortOrderList builds a list containing only valid CIDRs based on the content of addressSortOrder. -// -// It will ignore and warn about invalid sort order items. -func buildAddressSortOrderList(addressSortOrder string) []*net.IPNet { - var list []*net.IPNet - for _, item := range strings.Split(addressSortOrder, ",") { - item = strings.TrimSpace(item) - - _, cidr, err := net.ParseCIDR(item) - if err != nil { - klog.Warningf("Ignoring invalid sort order item '%s': %v.", item, err) - continue - } - - list = append(list, cidr) - } - - return list -} - -// getSortPriority returns the priority as int of an address. -// -// The priority depends on the index of the CIDR in the list the address is matching, -// where the first item of the list has higher priority than the last. -// -// If the address does not match any CIDR or is not an IP address the function returns noSortPriority. -func getSortPriority(list []*net.IPNet, address string) int { - parsedAddress := net.ParseIP(address) - if parsedAddress == nil { - return noSortPriority - } - - for i, cidr := range list { - if cidr.Contains(parsedAddress) { - fmt.Println(i, cidr, len(list)-i) - return len(list) - i - } - } - - return noSortPriority -} - -// sortNodeAddresses sorts node addresses based on comma separated list of CIDRs represented by addressSortOrder. -// -// The function only sorts addresses which match the CIDR and leaves the other addresses in the same order they are in. -// Essentially, it will also group the addresses matching a CIDR together and sort them ascending in this group, -// whereas the inter-group sorting depends on the priority. -// -// The priority depends on the order of the item in addressSortOrder, where the first item has higher priority than the last. -func sortNodeAddresses(addresses []v1.NodeAddress, addressSortOrder string) { - list := buildAddressSortOrderList(addressSortOrder) - - sort.SliceStable(addresses, func(i int, j int) bool { - addressLeft := addresses[i] - addressRight := addresses[j] - - priorityLeft := getSortPriority(list, addressLeft.Address) - priorityRight := getSortPriority(list, addressRight.Address) - - // ignore priorities of value 0 since this means the address has noSortPriority and we need to sort by priority - if priorityLeft > noSortPriority && priorityLeft == priorityRight { - return bytes.Compare(net.ParseIP(addressLeft.Address), net.ParseIP(addressRight.Address)) < 0 - } - - return priorityLeft > priorityRight - }) -} - -// Instances returns an implementation of Instances for OpenStack. -// TODO: v1 instance apis can be deleted after the v2 is verified enough -func (os *OpenStack) Instances() (cloudprovider.Instances, bool) { - if os.useV1Instances { - return os.instances() - } - return nil, false -} - -func (os *OpenStack) instances() (*Instances, bool) { - klog.V(4).Info("openstack.Instances() called") +// InstancesV2 returns an implementation of InstancesV2 for OpenStack. +func (os *OpenStack) InstancesV2() (cloudprovider.InstancesV2, bool) { + klog.V(4).Info("openstack.Instancesv2() called") compute, err := client.NewComputeV2(os.provider, os.epOpts) if err != nil { @@ -161,384 +74,117 @@ func (os *OpenStack) instances() (*Instances, bool) { regionalProviderID = true } - return &Instances{ + return &InstancesV2{ compute: compute, network: network, region: os.epOpts.Region, regionProviderID: regionalProviderID, - opts: os.metadataOpts, networkingOpts: os.networkingOpts, }, true } -// InstanceID returns the kubelet's cloud provider ID. -func (os *OpenStack) InstanceID() (string, error) { - if len(os.localInstanceID) == 0 { - id, err := readInstanceID(os.metadataOpts.SearchOrder) - if err != nil { - return "", err - } - os.localInstanceID = id - } - return os.localInstanceID, nil -} - -// CurrentNodeName implements Instances.CurrentNodeName -// Note this is *not* necessarily the same as hostname. -func (i *Instances) CurrentNodeName(ctx context.Context, hostname string) (types.NodeName, error) { - md, err := metadata.Get(i.opts.SearchOrder) - if err != nil { - return "", err - } - return types.NodeName(md.Name), nil -} - -// AddSSHKeyToAllInstances is not implemented for OpenStack -func (i *Instances) AddSSHKeyToAllInstances(ctx context.Context, user string, keyData []byte) error { - return cloudprovider.NotImplemented -} - -// NodeAddresses implements Instances.NodeAddresses -func (i *Instances) NodeAddresses(ctx context.Context, name types.NodeName) ([]v1.NodeAddress, error) { - klog.V(4).Infof("NodeAddresses(%v) called", name) - - addrs, err := getAddressesByName(i.compute, name, i.networkingOpts) - if err != nil { - return nil, err - } - - klog.V(4).Infof("NodeAddresses(%v) => %v", name, addrs) - return addrs, nil -} - -// NodeAddressesByProviderID returns the node addresses of an instances with the specified unique providerID -// This method will not be called from the node that is requesting this ID. i.e. metadata service -// and other local methods cannot be used here -func (i *Instances) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]v1.NodeAddress, error) { - klog.V(4).Infof("NodeAddressesByProviderID(%v) called", providerID) - - instanceID, instanceRegion, err := instanceIDFromProviderID(providerID) - if err != nil { - return []v1.NodeAddress{}, err - } - - if instanceRegion != "" && instanceRegion != i.region { - klog.V(4).Infof("NodeAddressesByProviderID(%v) has foreign region %v, skipped", providerID, instanceRegion) - - return []v1.NodeAddress{}, nil - } - - mc := metrics.NewMetricContext("server", "get") - server, err := servers.Get(ctx, i.compute, instanceID).Extract() - - if mc.ObserveRequest(err) != nil { - return []v1.NodeAddress{}, err - } - - ports, err := getAttachedPorts(i.network, server.ID) - if err != nil { - return []v1.NodeAddress{}, err - } - - addresses, err := nodeAddresses(server, ports, i.network, i.networkingOpts) - if err != nil { - return []v1.NodeAddress{}, err +// InstanceExists indicates whether a given node exists according to the cloud provider +func (i *InstancesV2) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) { + _, err := i.getInstance(ctx, node) + if err == cloudprovider.InstanceNotFound { + klog.V(6).Infof("instance not found for node: %s", node.Name) + return false, nil } - klog.V(4).Infof("NodeAddressesByProviderID(%v) => %v", providerID, addresses) - return addresses, nil -} - -// InstanceExists returns true if the instance for the given node exists. -func (i *Instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) { - return i.InstanceExistsByProviderID(ctx, node.Spec.ProviderID) -} - -func instanceExistsByProviderID(ctx context.Context, compute *gophercloud.ServiceClient, providerID string, region string) (bool, error) { - instanceID, instanceRegion, err := instanceIDFromProviderID(providerID) if err != nil { return false, err } - if instanceRegion != "" && instanceRegion != region { - klog.V(4).Infof("instanceExistsByProviderID(%v) has foreign region %v, skipped", providerID, instanceRegion) - - return true, nil - } - - mc := metrics.NewMetricContext("server", "get") - _, err = servers.Get(ctx, compute, instanceID).Extract() - if mc.ObserveRequest(err) != nil { - if errors.IsNotFound(err) { - return false, nil - } - return false, err - } - return true, nil } -// InstanceExistsByProviderID returns true if the instance with the given provider id still exists. -// If false is returned with no error, the instance will be immediately deleted by the cloud controller manager. -func (i *Instances) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) { - return instanceExistsByProviderID(ctx, i.compute, providerID, i.region) -} - -// InstanceShutdown returns true if the instances is in safe state to detach volumes. -// It is the only state, where volumes can be detached immediately. -func (i *Instances) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) { - return i.InstanceShutdownByProviderID(ctx, node.Spec.ProviderID) -} - -func instanceShutdownByProviderID(ctx context.Context, compute *gophercloud.ServiceClient, providerID string, region string) (bool, error) { - instanceID, instanceRegion, err := instanceIDFromProviderID(providerID) +// InstanceShutdown returns true if the instance is shutdown according to the cloud provider. +func (i *InstancesV2) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) { + server, err := i.getInstance(ctx, node) if err != nil { return false, err } - if instanceRegion != "" && instanceRegion != region { - return false, fmt.Errorf("ProviderID \"%s\" didn't match supported region \"%s\"", providerID, region) - } - - mc := metrics.NewMetricContext("server", "get") - server, err := servers.Get(ctx, compute, instanceID).Extract() - if mc.ObserveRequest(err) != nil { - return false, err - } - // SHUTOFF is the only state where we can detach volumes immediately if server.Status == instanceShutoff { return true, nil } - return false, nil -} -// InstanceShutdownByProviderID returns true if the instances is in safe state to detach volumes. -// It is the only state, where volumes can be detached immediately. -func (i *Instances) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) { - return instanceShutdownByProviderID(ctx, i.compute, providerID, i.region) + return false, nil } -// InstanceMetadata returns metadata of the specified instance. -func (i *Instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) { - instanceID, instanceRegion, err := instanceIDFromProviderID(node.Spec.ProviderID) +// InstanceMetadata returns the instance's metadata. +func (i *InstancesV2) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) { + srv, err := i.getInstance(ctx, node) if err != nil { return nil, err } - - if instanceRegion != "" && instanceRegion != i.region { - return nil, fmt.Errorf("ProviderID \"%s\" didn't match supported region \"%s\"", node.Spec.ProviderID, i.region) + var server servers.Server + if srv != nil { + server = *srv } - mc := metrics.NewMetricContext("server", "get") - srv, err := servers.Get(ctx, i.compute, instanceID).Extract() - if mc.ObserveRequest(err) != nil { - return nil, err - } - - instanceType, err := srvInstanceType(i.compute, srv) + instanceType, err := srvInstanceType(ctx, i.compute, &server) if err != nil { return nil, err } - ports, err := getAttachedPorts(i.network, srv.ID) + ports, err := getAttachedPorts(ctx, i.network, server.ID) if err != nil { return nil, err } - addresses, err := nodeAddresses(srv, ports, i.network, i.networkingOpts) + + addresses, err := nodeAddresses(ctx, &server, ports, i.network, i.networkingOpts) if err != nil { return nil, err } + availabilityZone := util.SanitizeLabel(server.AvailabilityZone) + return &cloudprovider.InstanceMetadata{ - ProviderID: node.Spec.ProviderID, + ProviderID: i.makeInstanceID(&server), InstanceType: instanceType, NodeAddresses: addresses, + Zone: availabilityZone, + Region: i.region, }, nil } -// InstanceID returns the cloud provider ID of the specified instance. -func (i *Instances) InstanceID(ctx context.Context, name types.NodeName) (string, error) { - srv, err := getServerByName(i.compute, name) - if err != nil { - if err == errors.ErrNotFound { - return "", cloudprovider.InstanceNotFound - } - return "", err - } - +func (i *InstancesV2) makeInstanceID(srv *servers.Server) string { if i.regionProviderID { - return i.region + "/" + srv.ID, nil + return fmt.Sprintf("%s://%s/%s", ProviderName, i.region, srv.ID) } - - // In the future it is possible to also return an endpoint as: - // / - return "/" + srv.ID, nil + return fmt.Sprintf("%s:///%s", ProviderName, srv.ID) } -// InstanceTypeByProviderID returns the cloudprovider instance type of the node with the specified unique providerID -// This method will not be called from the node that is requesting this ID. i.e. metadata service -// and other local methods cannot be used here -func (i *Instances) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) { - instanceID, instanceRegion, err := instanceIDFromProviderID(providerID) +func (i *InstancesV2) getInstance(ctx context.Context, node *v1.Node) (*servers.Server, error) { + if node.Spec.ProviderID == "" { + return getServerByName(ctx, i.compute, node.Name) + } + + instanceID, instanceRegion, err := instanceIDFromProviderID(node.Spec.ProviderID) if err != nil { - return "", err + return nil, err } if instanceRegion != "" && instanceRegion != i.region { - return "", fmt.Errorf("ProviderID \"%s\" didn't match supported region \"%s\"", providerID, i.region) + return nil, fmt.Errorf("ProviderID \"%s\" didn't match supported region \"%s\"", node.Spec.ProviderID, i.region) } mc := metrics.NewMetricContext("server", "get") server, err := servers.Get(ctx, i.compute, instanceID).Extract() - if mc.ObserveRequest(err) != nil { - return "", err - } - - return srvInstanceType(i.compute, server) -} - -// InstanceType returns the type of the specified instance. -func (i *Instances) InstanceType(ctx context.Context, name types.NodeName) (string, error) { - srv, err := getServerByName(i.compute, name) - - if err != nil { - return "", err - } - - return srvInstanceType(i.compute, srv) -} - -func srvInstanceType(client *gophercloud.ServiceClient, srv *servers.Server) (string, error) { - keys := []string{"original_name", "id"} - for _, key := range keys { - val, found := srv.Flavor[key] - if !found { - continue - } - - flavor, ok := val.(string) - if !ok { - continue - } - - if key == "original_name" && isValidLabelValue(flavor) { - return flavor, nil - } - - // get flavor name by id - mc := metrics.NewMetricContext("flavor", "get") - f, err := flavors.Get(context.TODO(), client, flavor).Extract() - if mc.ObserveRequest(err) == nil { - if isValidLabelValue(f.Name) { - return f.Name, nil - } - // fallback on flavor id - return f.ID, nil - } - } - return "", fmt.Errorf("flavor original_name/id not found") -} - -func isValidLabelValue(v string) bool { - if errs := validation.IsValidLabelValue(v); len(errs) != 0 { - return false - } - return true -} - -// 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 \"openstack://region/InstanceID\"", providerID) - } - return matches[2], matches[1], nil -} - -// AddToNodeAddresses appends the NodeAddresses to the passed-by-pointer slice, -// only if they do not already exist -func AddToNodeAddresses(addresses *[]v1.NodeAddress, addAddresses ...v1.NodeAddress) { - for _, add := range addAddresses { - exists := false - for _, existing := range *addresses { - if existing.Address == add.Address && existing.Type == add.Type { - exists = true - break - } - } - if !exists { - *addresses = append(*addresses, add) - } - } -} - -// RemoveFromNodeAddresses removes the NodeAddresses from the passed-by-pointer -// slice if they already exist. -func RemoveFromNodeAddresses(addresses *[]v1.NodeAddress, removeAddresses ...v1.NodeAddress) { - var indexesToRemove []int - for _, remove := range removeAddresses { - for i := len(*addresses) - 1; i >= 0; i-- { - existing := (*addresses)[i] - if existing.Address == remove.Address && (existing.Type == remove.Type || remove.Type == "") { - indexesToRemove = append(indexesToRemove, i) - } - } - } - for _, i := range indexesToRemove { - if i < len(*addresses) { - *addresses = append((*addresses)[:i], (*addresses)[i+1:]...) - } - } -} - -// mapNodeNameToServerName maps a k8s NodeName to an OpenStack Server Name -// This is a simple string cast. -func mapNodeNameToServerName(nodeName types.NodeName) string { - return string(nodeName) -} - -func readInstanceID(searchOrder string) (string, error) { - // First, try to get data from metadata service because local - // data might be changed by accident - md, err := metadata.Get(searchOrder) - if err == nil { - return md.UUID, nil - } - - // Try to find instance ID on the local filesystem (created by cloud-init) - const instanceIDFile = "/var/lib/cloud/data/instance-id" - idBytes, err := sysos.ReadFile(instanceIDFile) - if err == nil { - instanceID := string(idBytes) - instanceID = strings.TrimSpace(instanceID) - klog.V(3).Infof("Got instance id from %s: %s", instanceIDFile, instanceID) - if instanceID != "" && instanceID != "iid-datasource-none" { - return instanceID, nil + if errors.IsNotFound(err) { + return nil, cloudprovider.InstanceNotFound } + return nil, err } - - klog.V(3).Infof("Failed to obtain instanceID, this likely lead to potential error") - - return "", err + return server, nil } -func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*servers.Server, error) { +func getServerByName(ctx context.Context, client *gophercloud.ServiceClient, name string) (*servers.Server, error) { opts := servers.ListOpts{ - Name: fmt.Sprintf("^%s$", regexp.QuoteMeta(mapNodeNameToServerName(name))), + Name: fmt.Sprintf("^%s$", regexp.QuoteMeta(name)), } serverList := make([]servers.Server, 0, 1) @@ -546,7 +192,7 @@ func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*s mc := metrics.NewMetricContext("server", "list") pager := servers.List(client, opts) - err := pager.EachPage(context.TODO(), func(_ context.Context, page pagination.Page) (bool, error) { + err := pager.EachPage(ctx, func(_ context.Context, page pagination.Page) (bool, error) { s, err := servers.ExtractServers(page) if err != nil { return false, err @@ -568,188 +214,80 @@ func getServerByName(client *gophercloud.ServiceClient, name types.NodeName) (*s return &serverList[0], nil } -// IP addresses order: -// * interfaces private IPs -// * access IPs -// * metadata hostname -// * server object Addresses (floating type) -func nodeAddresses(srv *servers.Server, ports []PortWithTrunkDetails, client *gophercloud.ServiceClient, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) { - addrs := []v1.NodeAddress{} - - // parse private IP addresses first in an ordered manner - for _, port := range ports { - for _, fixedIP := range port.FixedIPs { - if port.Status == "ACTIVE" { - isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil - if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { - AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: v1.NodeInternalIP, - Address: fixedIP.IPAddress, - }, - ) - } - } - } - } +// If Instances.InstanceID or cloudprovider.GetInstanceProviderID is changed, the regexp should be changed too. +var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `://([^/]*)/([^/]+)$`) - // process public IP addresses - if srv.AccessIPv4 != "" { - AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: v1.NodeExternalIP, - Address: srv.AccessIPv4, - }, - ) +// 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 } - if srv.AccessIPv6 != "" && !networkingOpts.IPv6SupportDisabled { - AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: v1.NodeExternalIP, - Address: srv.AccessIPv6, - }, - ) + matches := providerIDRegexp.FindStringSubmatch(providerID) + if len(matches) != 3 { + return "", "", fmt.Errorf("ProviderID \"%s\" didn't match expected format \"openstack://region/InstanceID\"", providerID) } + return matches[2], matches[1], nil +} - if srv.Metadata[TypeHostName] != "" { - AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: v1.NodeHostName, - Address: srv.Metadata[TypeHostName], - }, - ) - } +// getAttachedPorts returns a list of ports attached to a server. +func getAttachedPorts(ctx context.Context, client *gophercloud.ServiceClient, serverID string) ([]PortWithTrunkDetails, error) { + var allPorts []PortWithTrunkDetails - // process the rest - type Address struct { - IPType string `mapstructure:"OS-EXT-IPS:type"` - Addr string + listOpts := ports.ListOpts{ + DeviceID: serverID, } - - var addresses map[string][]Address - err := mapstructure.Decode(srv.Addresses, &addresses) + allPages, err := ports.List(client, listOpts).AllPages(ctx) if err != nil { - return nil, err + return allPorts, err } - // Add the addresses assigned on subports via trunk - // This exposes the vlan networks to which subports are attached - for _, port := range ports { - for _, subport := range port.TrunkDetails.SubPorts { - p, err := neutronports.Get(context.TODO(), client, subport.PortID).Extract() - if err != nil { - klog.Errorf("Failed to get subport %s details: %v", subport.PortID, err) - continue - } - n, err := networks.Get(context.TODO(), client, p.NetworkID).Extract() - if err != nil { - klog.Errorf("Failed to get subport %s network details: %v", subport.PortID, err) - continue - } - for _, fixedIP := range p.FixedIPs { - klog.V(5).Infof("Node '%s' is found subport '%s' address '%s/%s'", srv.Name, p.Name, n.Name, fixedIP.IPAddress) - isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil - if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { - addr := Address{IPType: "fixed", Addr: fixedIP.IPAddress} - subportAddresses := map[string][]Address{n.Name: {addr}} - srvAddresses, ok := addresses[n.Name] - if !ok { - addresses[n.Name] = subportAddresses[n.Name] - } else { - // this is to take care the corner case - // where the same network is attached to the node both directly and via trunk - addresses[n.Name] = append(srvAddresses, subportAddresses[n.Name]...) - } - } - } - } + err = ports.ExtractPortsInto(allPages, &allPorts) + if err != nil { + return allPorts, err } - networks := make([]string, 0, len(addresses)) - for k := range addresses { - networks = append(networks, k) - } - sort.Strings(networks) - - for _, network := range networks { - for _, props := range addresses[network] { - var addressType v1.NodeAddressType - if props.IPType == "floating" { - addressType = v1.NodeExternalIP - } else if slices.Contains(networkingOpts.PublicNetworkName, network) { - addressType = v1.NodeExternalIP - // removing already added address to avoid listing it as both ExternalIP and InternalIP - // may happen due to listing "private" network as "public" in CCM's config - RemoveFromNodeAddresses(&addrs, - v1.NodeAddress{ - Address: props.Addr, - }, - ) - } else { - if len(networkingOpts.InternalNetworkName) == 0 || slices.Contains(networkingOpts.InternalNetworkName, network) { - addressType = v1.NodeInternalIP - } else { - klog.V(5).Infof("Node '%s' address '%s' ignored due to 'internal-network-name' option", srv.Name, props.Addr) - RemoveFromNodeAddresses(&addrs, - v1.NodeAddress{ - Address: props.Addr, - }, - ) - continue - } - } + return allPorts, nil +} - isIPv6 := net.ParseIP(props.Addr).To4() == nil - if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { - AddToNodeAddresses(&addrs, - v1.NodeAddress{ - Type: addressType, - Address: props.Addr, - }, - ) - } +func srvInstanceType(ctx context.Context, client *gophercloud.ServiceClient, srv *servers.Server) (string, error) { + keys := []string{"original_name", "id"} + for _, key := range keys { + val, found := srv.Flavor[key] + if !found { + continue } - } - - if networkingOpts.AddressSortOrder != "" { - sortNodeAddresses(addrs, networkingOpts.AddressSortOrder) - } - klog.V(5).Infof("Node '%s' returns addresses '%v'", srv.Name, addrs) - return addrs, nil -} + flavor, ok := val.(string) + if !ok { + continue + } -func getAddressesByName(client *gophercloud.ServiceClient, name types.NodeName, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) { - srv, err := getServerByName(client, name) - if err != nil { - return nil, err - } + if key == "original_name" && isValidLabelValue(flavor) { + return flavor, nil + } - ports, err := getAttachedPorts(client, srv.ID) - if err != nil { - return nil, err + // get flavor name by id + mc := metrics.NewMetricContext("flavor", "get") + f, err := flavors.Get(ctx, client, flavor).Extract() + if mc.ObserveRequest(err) == nil { + if isValidLabelValue(f.Name) { + return f.Name, nil + } + // fallback on flavor id + return f.ID, nil + } } - - return nodeAddresses(srv, ports, client, networkingOpts) + return "", fmt.Errorf("flavor original_name/id not found") } -// getAttachedPorts returns a list of ports attached to a server. -func getAttachedPorts(client *gophercloud.ServiceClient, serverID string) ([]PortWithTrunkDetails, error) { - listOpts := neutronports.ListOpts{ - DeviceID: serverID, - } - - var ports []PortWithTrunkDetails - - allPages, err := neutronports.List(client, listOpts).AllPages(context.TODO()) - if err != nil { - return ports, err - } - err = neutronports.ExtractPortsInto(allPages, &ports) - if err != nil { - return ports, err +func isValidLabelValue(v string) bool { + if errs := validation.IsValidLabelValue(v); len(errs) != 0 { + return false } - - return ports, nil + return true } diff --git a/pkg/openstack/instances_addresses.go b/pkg/openstack/instances_addresses.go new file mode 100644 index 0000000000..5d8e1d74dd --- /dev/null +++ b/pkg/openstack/instances_addresses.go @@ -0,0 +1,293 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed 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 openstack + +import ( + "bytes" + "context" + "net" + "slices" + "sort" + + "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" + "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/networks" + neutronports "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports" + "github.com/mitchellh/mapstructure" + + v1 "k8s.io/api/core/v1" + "k8s.io/cloud-provider-openstack/pkg/util" + "k8s.io/klog/v2" +) + +const ( + noSortPriority = 0 +) + +// buildAddressSortOrderList builds a list containing only valid CIDRs based on the content of addressSortOrder. +// +// It will ignore and warn about invalid sort order items. +func buildAddressSortOrderList(addressSortOrder string) []*net.IPNet { + var list []*net.IPNet + for _, item := range util.SplitTrim(addressSortOrder, ',') { + _, cidr, err := net.ParseCIDR(item) + if err != nil { + klog.Warningf("Ignoring invalid sort order item '%s': %v.", item, err) + continue + } + + list = append(list, cidr) + } + + return list +} + +// getSortPriority returns the priority as int of an address. +// +// The priority depends on the index of the CIDR in the list the address is matching, +// where the first item of the list has higher priority than the last. +// +// If the address does not match any CIDR or is not an IP address the function returns noSortPriority. +func getSortPriority(list []*net.IPNet, address string) int { + parsedAddress := net.ParseIP(address) + if parsedAddress == nil { + return noSortPriority + } + + for i, cidr := range list { + if cidr.Contains(parsedAddress) { + return len(list) - i + } + } + + return noSortPriority +} + +// sortNodeAddresses sorts node addresses based on comma separated list of CIDRs represented by addressSortOrder. +// +// The function only sorts addresses which match the CIDR and leaves the other addresses in the same order they are in. +// Essentially, it will also group the addresses matching a CIDR together and sort them ascending in this group, +// whereas the inter-group sorting depends on the priority. +// +// The priority depends on the order of the item in addressSortOrder, where the first item has higher priority than the last. +func sortNodeAddresses(addresses []v1.NodeAddress, addressSortOrder string) { + list := buildAddressSortOrderList(addressSortOrder) + + sort.SliceStable(addresses, func(i int, j int) bool { + addressLeft := addresses[i] + addressRight := addresses[j] + + priorityLeft := getSortPriority(list, addressLeft.Address) + priorityRight := getSortPriority(list, addressRight.Address) + + // ignore priorities of value 0 since this means the address has noSortPriority and we need to sort by priority + if priorityLeft > noSortPriority && priorityLeft == priorityRight { + return bytes.Compare(net.ParseIP(addressLeft.Address), net.ParseIP(addressRight.Address)) < 0 + } + + return priorityLeft > priorityRight + }) +} + +// addToNodeAddresses appends the NodeAddresses to the passed-by-pointer slice, +// only if they do not already exist +func addToNodeAddresses(addresses *[]v1.NodeAddress, addAddresses ...v1.NodeAddress) { + for _, add := range addAddresses { + exists := false + for _, existing := range *addresses { + if existing.Address == add.Address && existing.Type == add.Type { + exists = true + break + } + } + if !exists { + *addresses = append(*addresses, add) + } + } +} + +// removeFromNodeAddresses removes the NodeAddresses from the passed-by-pointer +// slice if they already exist. +func removeFromNodeAddresses(addresses *[]v1.NodeAddress, removeAddresses ...v1.NodeAddress) { + var indexesToRemove []int + for _, remove := range removeAddresses { + for i := len(*addresses) - 1; i >= 0; i-- { + existing := (*addresses)[i] + if existing.Address == remove.Address && (existing.Type == remove.Type || remove.Type == "") { + indexesToRemove = append(indexesToRemove, i) + } + } + } + for _, i := range indexesToRemove { + if i < len(*addresses) { + *addresses = append((*addresses)[:i], (*addresses)[i+1:]...) + } + } +} + +// IP addresses order: +// * interfaces private IPs +// * access IPs +// * metadata hostname +// * server object Addresses (floating type) +func nodeAddresses(ctx context.Context, srv *servers.Server, ports []PortWithTrunkDetails, client *gophercloud.ServiceClient, networkingOpts NetworkingOpts) ([]v1.NodeAddress, error) { + addrs := []v1.NodeAddress{} + + // parse private IP addresses first in an ordered manner + for _, port := range ports { + for _, fixedIP := range port.FixedIPs { + if port.Status != "ACTIVE" { + continue + } + isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil + if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { + addToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeInternalIP, + Address: fixedIP.IPAddress, + }, + ) + } + } + } + + // process public IP addresses + if srv.AccessIPv4 != "" { + addToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeExternalIP, + Address: srv.AccessIPv4, + }, + ) + } + + if srv.AccessIPv6 != "" && !networkingOpts.IPv6SupportDisabled { + addToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeExternalIP, + Address: srv.AccessIPv6, + }, + ) + } + + if srv.Metadata[TypeHostName] != "" { + addToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: v1.NodeHostName, + Address: srv.Metadata[TypeHostName], + }, + ) + } + + // process the rest + type Address struct { + IPType string `mapstructure:"OS-EXT-IPS:type"` + Addr string + } + + var addresses map[string][]Address + err := mapstructure.Decode(srv.Addresses, &addresses) + if err != nil { + return nil, err + } + + // Add the addresses assigned on subports via trunk + // This exposes the vlan networks to which subports are attached + for _, port := range ports { + for _, subport := range port.TrunkDetails.SubPorts { + p, err := neutronports.Get(ctx, client, subport.PortID).Extract() + if err != nil { + klog.Errorf("Failed to get subport %s details: %v", subport.PortID, err) + continue + } + n, err := networks.Get(ctx, client, p.NetworkID).Extract() + if err != nil { + klog.Errorf("Failed to get subport %s network details: %v", subport.PortID, err) + continue + } + for _, fixedIP := range p.FixedIPs { + klog.V(5).Infof("Node '%s' is found subport '%s' address '%s/%s'", srv.Name, p.Name, n.Name, fixedIP.IPAddress) + isIPv6 := net.ParseIP(fixedIP.IPAddress).To4() == nil + if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { + addr := Address{IPType: "fixed", Addr: fixedIP.IPAddress} + subportAddresses := map[string][]Address{n.Name: {addr}} + srvAddresses, ok := addresses[n.Name] + if !ok { + addresses[n.Name] = subportAddresses[n.Name] + } else { + // this is to take care the corner case + // where the same network is attached to the node both directly and via trunk + addresses[n.Name] = append(srvAddresses, subportAddresses[n.Name]...) + } + } + } + } + } + + networks := make([]string, 0, len(addresses)) + for k := range addresses { + networks = append(networks, k) + } + sort.Strings(networks) + + for _, network := range networks { + for _, props := range addresses[network] { + var addressType v1.NodeAddressType + if props.IPType == "floating" { + addressType = v1.NodeExternalIP + } else if slices.Contains(networkingOpts.PublicNetworkName, network) { + addressType = v1.NodeExternalIP + // removing already added address to avoid listing it as both ExternalIP and InternalIP + // may happen due to listing "private" network as "public" in CCM's config + removeFromNodeAddresses(&addrs, + v1.NodeAddress{ + Address: props.Addr, + }, + ) + } else { + if len(networkingOpts.InternalNetworkName) == 0 || slices.Contains(networkingOpts.InternalNetworkName, network) { + addressType = v1.NodeInternalIP + } else { + klog.V(5).Infof("Node '%s' address '%s' ignored due to 'internal-network-name' option", srv.Name, props.Addr) + removeFromNodeAddresses(&addrs, + v1.NodeAddress{ + Address: props.Addr, + }, + ) + continue + } + } + + isIPv6 := net.ParseIP(props.Addr).To4() == nil + if !(isIPv6 && networkingOpts.IPv6SupportDisabled) { + addToNodeAddresses(&addrs, + v1.NodeAddress{ + Type: addressType, + Address: props.Addr, + }, + ) + } + } + } + + if networkingOpts.AddressSortOrder != "" { + sortNodeAddresses(addrs, networkingOpts.AddressSortOrder) + } + + klog.V(5).Infof("Node '%s' returns addresses '%v'", srv.Name, addrs) + return addrs, nil +} diff --git a/pkg/openstack/instances_addresses_test.go b/pkg/openstack/instances_addresses_test.go new file mode 100644 index 0000000000..eb3955c891 --- /dev/null +++ b/pkg/openstack/instances_addresses_test.go @@ -0,0 +1,191 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed 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 openstack + +import ( + "fmt" + "net" + "reflect" + "testing" + + v1 "k8s.io/api/core/v1" +) + +func TestBuildAddressSortOrderList(t *testing.T) { + var emptyList []*net.IPNet + + _, cidrIPv4, _ := net.ParseCIDR("192.168.0.0/16") + _, cidrIPv6, _ := net.ParseCIDR("2001:4800:790e::/64") + + emptyOption := "" + multipleInvalidOptions := "InvalidOption, AnotherInvalidOption" + multipleOptionsWithInvalidOption := fmt.Sprintf("%s, %s, %s", cidrIPv4, multipleInvalidOptions, cidrIPv6) + + tests := map[string][]*net.IPNet{ + emptyOption: emptyList, + multipleInvalidOptions: emptyList, + multipleOptionsWithInvalidOption: {cidrIPv4, cidrIPv6}, + } + + for option, want := range tests { + actual := buildAddressSortOrderList(option) + if !reflect.DeepEqual(want, actual) { + t.Errorf("assignSortOrderPriorities returned incorrect value for '%v', want %+v but got %+v", option, want, actual) + } + } +} + +func TestGetSortPriority(t *testing.T) { + _, cidrIPv4, _ := net.ParseCIDR("192.168.100.0/24") + _, cidrIPv6, _ := net.ParseCIDR("2001:4800:790e::/64") + + list := []*net.IPNet{cidrIPv4, cidrIPv6} + t.Log(list) + tests := map[string]int{ + "": noSortPriority, + "some-host.exam.ple": noSortPriority, + "2001:4800:790e::82a8": 1, + "2001:cafe:babe::82a8": noSortPriority, + "192.168.100.200": 2, + "192.168.101.123": noSortPriority, + } + + for option, want := range tests { + actual := getSortPriority(list, option) + if !reflect.DeepEqual(want, actual) { + t.Errorf("assignSortOrderPriorities returned incorrect value for '%v', want %+v but got %+v", option, want, actual) + } + } +} + +func executeSortNodeAddressesTest(t *testing.T, addressSortOrder string, want []v1.NodeAddress) { + addresses := []v1.NodeAddress{ + {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, + {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, + {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, + {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, + } + + sortNodeAddresses(addresses, addressSortOrder) + + t.Logf("addresses are %v", addresses) + if !reflect.DeepEqual(want, addresses) { + t.Fatalf("sortNodeAddresses returned incorrect value, want %v", want) + } +} + +func TestSortNodeAddressesWithAnInvalidCIDR(t *testing.T) { + addressSortOrder := "10.0.0.0/244" + + want := []v1.NodeAddress{ + {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, + {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, + {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, + {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, + } + + executeSortNodeAddressesTest(t, addressSortOrder, want) +} + +func TestSortNodeAddressesWithOneIPv4CIDR(t *testing.T) { + addressSortOrder := "10.0.0.0/8" + + want := []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, + {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, + {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, + } + + executeSortNodeAddressesTest(t, addressSortOrder, want) +} + +func TestSortNodeAddressesWithOneIPv6CIDR(t *testing.T) { + addressSortOrder := "fd08:1374:fcee:916b::/64" + + want := []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, + {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, + {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, + {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, + {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, + } + + executeSortNodeAddressesTest(t, addressSortOrder, want) +} + +func TestSortNodeAddressesWithMultipleCIDRs(t *testing.T) { + addressSortOrder := "10.0.0.0/8, 172.16.0.0/16, 192.168.0.0/24, fd08:1374:fcee:916b::/64, 50.56.176.0/24, 2001:cafe:babe::/64" + + want := []v1.NodeAddress{ + {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, + {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, + {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, + {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, + {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, + {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, + {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, + {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, + {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, + } + + executeSortNodeAddressesTest(t, addressSortOrder, want) +} diff --git a/pkg/openstack/instances_test.go b/pkg/openstack/instances_test.go index 0d4b2e1c92..c8c21cdeea 100644 --- a/pkg/openstack/instances_test.go +++ b/pkg/openstack/instances_test.go @@ -17,180 +17,11 @@ limitations under the License. package openstack import ( - "fmt" - "net" - "reflect" "testing" "github.com/stretchr/testify/assert" - v1 "k8s.io/api/core/v1" ) -func TestBuildAddressSortOrderList(t *testing.T) { - var emptyList []*net.IPNet - - _, cidrIPv4, _ := net.ParseCIDR("192.168.0.0/16") - _, cidrIPv6, _ := net.ParseCIDR("2001:4800:790e::/64") - - emptyOption := "" - multipleInvalidOptions := "InvalidOption, AnotherInvalidOption" - multipleOptionsWithInvalidOption := fmt.Sprintf("%s, %s, %s", cidrIPv4, multipleInvalidOptions, cidrIPv6) - - tests := map[string][]*net.IPNet{ - emptyOption: emptyList, - multipleInvalidOptions: emptyList, - multipleOptionsWithInvalidOption: {cidrIPv4, cidrIPv6}, - } - - for option, want := range tests { - actual := buildAddressSortOrderList(option) - if !reflect.DeepEqual(want, actual) { - t.Errorf("assignSortOrderPriorities returned incorrect value for '%v', want %+v but got %+v", option, want, actual) - } - } -} - -func TestGetSortPriority(t *testing.T) { - _, cidrIPv4, _ := net.ParseCIDR("192.168.100.0/24") - _, cidrIPv6, _ := net.ParseCIDR("2001:4800:790e::/64") - - list := []*net.IPNet{cidrIPv4, cidrIPv6} - t.Log(list) - tests := map[string]int{ - "": noSortPriority, - "some-host.exam.ple": noSortPriority, - "2001:4800:790e::82a8": 1, - "2001:cafe:babe::82a8": noSortPriority, - "192.168.100.200": 2, - "192.168.101.123": noSortPriority, - } - - for option, want := range tests { - actual := getSortPriority(list, option) - if !reflect.DeepEqual(want, actual) { - t.Errorf("assignSortOrderPriorities returned incorrect value for '%v', want %+v but got %+v", option, want, actual) - } - } -} - -func executeSortNodeAddressesTest(t *testing.T, addressSortOrder string, want []v1.NodeAddress) { - addresses := []v1.NodeAddress{ - {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, - {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, - {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, - {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, - } - - sortNodeAddresses(addresses, addressSortOrder) - - t.Logf("addresses are %v", addresses) - if !reflect.DeepEqual(want, addresses) { - t.Fatalf("sortNodeAddresses returned incorrect value, want %v", want) - } -} - -func TestSortNodeAddressesWithAnInvalidCIDR(t *testing.T) { - addressSortOrder := "10.0.0.0/244" - - want := []v1.NodeAddress{ - {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, - {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, - {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, - {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, - } - - executeSortNodeAddressesTest(t, addressSortOrder, want) -} - -func TestSortNodeAddressesWithOneIPv4CIDR(t *testing.T) { - addressSortOrder := "10.0.0.0/8" - - want := []v1.NodeAddress{ - {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, - {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, - {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, - } - - executeSortNodeAddressesTest(t, addressSortOrder, want) -} - -func TestSortNodeAddressesWithOneIPv6CIDR(t *testing.T) { - addressSortOrder := "fd08:1374:fcee:916b::/64" - - want := []v1.NodeAddress{ - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, - {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, - {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, - {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, - {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, - } - - executeSortNodeAddressesTest(t, addressSortOrder, want) -} - -func TestSortNodeAddressesWithMultipleCIDRs(t *testing.T) { - addressSortOrder := "10.0.0.0/8, 172.16.0.0/16, 192.168.0.0/24, fd08:1374:fcee:916b::/64, 50.56.176.0/24, 2001:cafe:babe::/64" - - want := []v1.NodeAddress{ - {Type: v1.NodeInternalIP, Address: "10.0.0.31"}, - {Type: v1.NodeInternalIP, Address: "10.0.0.32"}, - {Type: v1.NodeInternalIP, Address: "172.16.0.1"}, - {Type: v1.NodeInternalIP, Address: "192.168.0.1"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:82a8"}, - {Type: v1.NodeInternalIP, Address: "fd08:1374:fcee:916b:be76:4eff:fe04:84a8"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.35"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.36"}, - {Type: v1.NodeInternalIP, Address: "50.56.176.37"}, - {Type: v1.NodeExternalIP, Address: "50.56.176.99"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:780e:510:be76:4eff:fe04:84a8"}, - {Type: v1.NodeExternalIP, Address: "2001:4800:790e:510:be76:4eff:fe04:82a8"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.novalocal"}, - {Type: v1.NodeHostName, Address: "a1-yinvcez57-0-bvynoyawrhcg-kube-minion-fg5i4jwcc2yy.exam.ple"}, - } - - executeSortNodeAddressesTest(t, addressSortOrder, want) -} - func Test_instanceIDFromProviderID(t *testing.T) { type args struct { providerID string diff --git a/pkg/openstack/instancesv2.go b/pkg/openstack/instancesv2.go deleted file mode 100644 index f8a9b5e74e..0000000000 --- a/pkg/openstack/instancesv2.go +++ /dev/null @@ -1,197 +0,0 @@ -/* -Copyright 2023 The Kubernetes Authors. - -Licensed 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 openstack - -import ( - "context" - "fmt" - sysos "os" - - "github.com/gophercloud/gophercloud/v2" - "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" - v1 "k8s.io/api/core/v1" - cloudprovider "k8s.io/cloud-provider" - "k8s.io/cloud-provider-openstack/pkg/client" - "k8s.io/cloud-provider-openstack/pkg/metrics" - "k8s.io/cloud-provider-openstack/pkg/util" - "k8s.io/cloud-provider-openstack/pkg/util/errors" - "k8s.io/klog/v2" -) - -// InstancesV2 encapsulates an implementation of InstancesV2 for OpenStack. -type InstancesV2 struct { - compute *gophercloud.ServiceClient - network *gophercloud.ServiceClient - region string - regionProviderID bool - networkingOpts NetworkingOpts -} - -// InstancesV2 returns an implementation of InstancesV2 for OpenStack. -func (os *OpenStack) InstancesV2() (cloudprovider.InstancesV2, bool) { - if !os.useV1Instances { - return os.instancesv2() - } - return nil, false -} - -func (os *OpenStack) instancesv2() (*InstancesV2, bool) { - klog.V(4).Info("openstack.Instancesv2() called") - - compute, err := client.NewComputeV2(os.provider, os.epOpts) - if err != nil { - klog.Errorf("unable to access compute v2 API : %v", err) - return nil, false - } - - network, err := client.NewNetworkV2(os.provider, os.epOpts) - if err != nil { - klog.Errorf("unable to access network v2 API : %v", err) - return nil, false - } - - regionalProviderID := false - if isRegionalProviderID := sysos.Getenv(RegionalProviderIDEnv); isRegionalProviderID == "true" { - regionalProviderID = true - } - - return &InstancesV2{ - compute: compute, - network: network, - region: os.epOpts.Region, - regionProviderID: regionalProviderID, - networkingOpts: os.networkingOpts, - }, true -} - -// InstanceExists indicates whether a given node exists according to the cloud provider -func (i *InstancesV2) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) { - _, err := i.getInstance(ctx, node) - if err == cloudprovider.InstanceNotFound { - klog.V(6).Infof("instance not found for node: %s", node.Name) - return false, nil - } - - if err != nil { - return false, err - } - - return true, nil -} - -// InstanceShutdown returns true if the instance is shutdown according to the cloud provider. -func (i *InstancesV2) InstanceShutdown(ctx context.Context, node *v1.Node) (bool, error) { - server, err := i.getInstance(ctx, node) - if err != nil { - return false, err - } - - // SHUTOFF is the only state where we can detach volumes immediately - if server.Status == instanceShutoff { - return true, nil - } - - return false, nil -} - -// InstanceMetadata returns the instance's metadata. -func (i *InstancesV2) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) { - srv, err := i.getInstance(ctx, node) - if err != nil { - return nil, err - } - var server servers.Server - if srv != nil { - server = *srv - } - - instanceType, err := srvInstanceType(i.compute, &server) - if err != nil { - return nil, err - } - - ports, err := getAttachedPorts(i.network, server.ID) - if err != nil { - return nil, err - } - - addresses, err := nodeAddresses(&server, ports, i.network, i.networkingOpts) - if err != nil { - return nil, err - } - - availabilityZone := util.SanitizeLabel(server.AvailabilityZone) - - return &cloudprovider.InstanceMetadata{ - ProviderID: i.makeInstanceID(&server), - InstanceType: instanceType, - NodeAddresses: addresses, - Zone: availabilityZone, - Region: i.region, - }, nil -} - -func (i *InstancesV2) makeInstanceID(srv *servers.Server) string { - if i.regionProviderID { - return fmt.Sprintf("%s://%s/%s", ProviderName, i.region, srv.ID) - } - return fmt.Sprintf("%s:///%s", ProviderName, srv.ID) -} - -func (i *InstancesV2) getInstance(ctx context.Context, node *v1.Node) (*servers.Server, error) { - if node.Spec.ProviderID == "" { - opt := servers.ListOpts{ - Name: fmt.Sprintf("^%s$", node.Name), - } - mc := metrics.NewMetricContext("server", "list") - allPages, err := servers.List(i.compute, opt).AllPages(context.TODO()) - if mc.ObserveRequest(err) != nil { - return nil, fmt.Errorf("error listing servers %v: %v", opt, err) - } - - serverList, err := servers.ExtractServers(allPages) - if err != nil { - return nil, fmt.Errorf("error extracting servers from pages: %v", err) - } - if len(serverList) == 0 { - return nil, cloudprovider.InstanceNotFound - } - if len(serverList) > 1 { - return nil, fmt.Errorf("getInstance: multiple instances found") - } - return &serverList[0], nil - } - - instanceID, instanceRegion, err := instanceIDFromProviderID(node.Spec.ProviderID) - if err != nil { - return nil, err - } - - if instanceRegion != "" && instanceRegion != i.region { - return nil, fmt.Errorf("ProviderID \"%s\" didn't match supported region \"%s\"", node.Spec.ProviderID, i.region) - } - - mc := metrics.NewMetricContext("server", "get") - server, err := servers.Get(context.TODO(), i.compute, instanceID).Extract() - if mc.ObserveRequest(err) != nil { - if errors.IsNotFound(err) { - return nil, cloudprovider.InstanceNotFound - } - return nil, err - } - return server, nil -} diff --git a/pkg/openstack/loadbalancer.go b/pkg/openstack/loadbalancer.go index cc64338f6a..ea39a3cc74 100644 --- a/pkg/openstack/loadbalancer.go +++ b/pkg/openstack/loadbalancer.go @@ -490,7 +490,7 @@ func getProxyProtocolFromServiceAnnotation(service *corev1.Service) *v2pools.Pro } // getSubnetIDForLB returns subnet-id for a specific node -func getSubnetIDForLB(network *gophercloud.ServiceClient, node corev1.Node, preferredIPFamily corev1.IPFamily) (string, error) { +func getSubnetIDForLB(ctx context.Context, network *gophercloud.ServiceClient, node corev1.Node, preferredIPFamily corev1.IPFamily) (string, error) { ipAddress, err := nodeAddressForLB(&node, preferredIPFamily) if err != nil { return "", err @@ -501,7 +501,7 @@ func getSubnetIDForLB(network *gophercloud.ServiceClient, node corev1.Node, pref return "", fmt.Errorf("can't determine instance ID from ProviderID when autodetecting LB subnet: %w", err) } - ports, err := getAttachedPorts(network, instanceID) + ports, err := getAttachedPorts(ctx, network, instanceID) if err != nil { return "", err } @@ -1266,7 +1266,7 @@ func (lbaas *LbaasV2) getNetworkID(service *corev1.Service, svcConf *serviceConf return "", nil } -func (lbaas *LbaasV2) checkServiceUpdate(service *corev1.Service, nodes []*corev1.Node, svcConf *serviceConfig) error { +func (lbaas *LbaasV2) checkServiceUpdate(ctx context.Context, service *corev1.Service, nodes []*corev1.Node, svcConf *serviceConfig) error { if len(service.Spec.Ports) == 0 { return fmt.Errorf("no ports provided to openstack load balancer") } @@ -1314,7 +1314,7 @@ func (lbaas *LbaasV2) checkServiceUpdate(service *corev1.Service, nodes []*corev } else { svcConf.lbMemberSubnetID = getStringFromServiceAnnotation(service, ServiceAnnotationLoadBalancerSubnetID, lbaas.opts.SubnetID) if len(svcConf.lbMemberSubnetID) == 0 && len(nodes) > 0 { - subnetID, err := getSubnetIDForLB(lbaas.network, *nodes[0], svcConf.preferredIPFamily) + subnetID, err := getSubnetIDForLB(ctx, lbaas.network, *nodes[0], svcConf.preferredIPFamily) if err != nil { return fmt.Errorf("no subnet-id found for service %s: %v", serviceName, err) } @@ -1355,7 +1355,7 @@ func (lbaas *LbaasV2) checkServiceDelete(service *corev1.Service, svcConf *servi return nil } -func (lbaas *LbaasV2) checkService(service *corev1.Service, nodes []*corev1.Node, svcConf *serviceConfig) error { +func (lbaas *LbaasV2) checkService(ctx context.Context, service *corev1.Service, nodes []*corev1.Node, svcConf *serviceConfig) error { serviceName := fmt.Sprintf("%s/%s", service.Namespace, service.Name) if len(nodes) == 0 { @@ -1449,7 +1449,7 @@ func (lbaas *LbaasV2) checkService(service *corev1.Service, nodes []*corev1.Node svcConf.lbMemberSubnetID = svcConf.lbSubnetID } if len(svcConf.lbNetworkID) == 0 && len(svcConf.lbSubnetID) == 0 { - subnetID, err := getSubnetIDForLB(lbaas.network, *nodes[0], svcConf.preferredIPFamily) + subnetID, err := getSubnetIDForLB(ctx, lbaas.network, *nodes[0], svcConf.preferredIPFamily) if err != nil { return fmt.Errorf("failed to get subnet to create load balancer for service %s: %v", serviceName, err) } @@ -1668,7 +1668,7 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName patcher := newServicePatcher(lbaas.kclient, service) defer func() { err = patcher.Patch(ctx, err) }() - if err := lbaas.checkService(service, nodes, svcConf); err != nil { + if err := lbaas.checkService(ctx, service, nodes, svcConf); err != nil { return nil, err } @@ -1887,7 +1887,7 @@ func (lbaas *LbaasV2) listSubnetsForNetwork(networkID string, tweak ...TweakSubN func (lbaas *LbaasV2) updateOctaviaLoadBalancer(ctx context.Context, clusterName string, service *corev1.Service, nodes []*corev1.Node) error { svcConf := new(serviceConfig) var err error - if err := lbaas.checkServiceUpdate(service, nodes, svcConf); err != nil { + if err := lbaas.checkServiceUpdate(ctx, service, nodes, svcConf); err != nil { return err } diff --git a/pkg/openstack/openstack.go b/pkg/openstack/openstack.go index 1eb9da87fe..0f45ed070a 100644 --- a/pkg/openstack/openstack.go +++ b/pkg/openstack/openstack.go @@ -22,7 +22,6 @@ import ( "io" "os" "slices" - "strings" "time" "github.com/gophercloud/gophercloud/v2" @@ -155,16 +154,13 @@ type RouterOpts struct { // OpenStack is an implementation of cloud provider Interface for OpenStack. type OpenStack struct { - provider *gophercloud.ProviderClient - epOpts *gophercloud.EndpointOpts - lbOpts LoadBalancerOpts - routeOpts RouterOpts - metadataOpts metadata.Opts - networkingOpts NetworkingOpts - // InstanceID of the server where this OpenStack object is instantiated. - localInstanceID string + provider *gophercloud.ProviderClient + epOpts *gophercloud.EndpointOpts + lbOpts LoadBalancerOpts + routeOpts RouterOpts + metadataOpts metadata.Opts + networkingOpts NetworkingOpts kclient kubernetes.Interface - useV1Instances bool // TODO: v1 instance apis can be deleted after the v2 is verified enough nodeInformer coreinformers.NodeInformer nodeInformerHasSynced func() bool @@ -299,12 +295,6 @@ func NewOpenStack(cfg Config) (*OpenStack, error) { } provider.HTTPClient.Timeout = cfg.Metadata.RequestTimeout.Duration - useV1Instances := false - v1instances := os.Getenv("OS_V1_INSTANCES") - if strings.ToLower(v1instances) == "true" { - useV1Instances = true - } - os := OpenStack{ provider: provider, epOpts: &gophercloud.EndpointOpts{ @@ -315,7 +305,6 @@ func NewOpenStack(cfg Config) (*OpenStack, error) { routeOpts: cfg.Route, metadataOpts: cfg.Metadata, networkingOpts: cfg.Networking, - useV1Instances: useV1Instances, } // ini file doesn't support maps so we are reusing top level sub sections @@ -330,6 +319,11 @@ func NewOpenStack(cfg Config) (*OpenStack, error) { return &os, nil } +// Instances v1 is no longer supported +func (os *OpenStack) Instances() (cloudprovider.Instances, bool) { + return nil, false +} + // Clusters is a no-op func (os *OpenStack) Clusters() (cloudprovider.Clusters, bool) { return nil, false @@ -442,7 +436,7 @@ func (os *OpenStack) GetZoneByNodeName(ctx context.Context, nodeName types.NodeN return cloudprovider.Zone{}, err } - srv, err := getServerByName(compute, nodeName) + srv, err := getServerByName(ctx, compute, string(nodeName)) if err != nil { if err == errors.ErrNotFound { return cloudprovider.Zone{}, cloudprovider.InstanceNotFound diff --git a/pkg/openstack/openstack_test.go b/pkg/openstack/openstack_test.go index 9e2ac70efc..7a826b868a 100644 --- a/pkg/openstack/openstack_test.go +++ b/pkg/openstack/openstack_test.go @@ -388,7 +388,7 @@ func TestNodeAddresses(t *testing.T) { }, } - addrs, err := nodeAddresses(&srv, ports, nil, networkingOpts) + addrs, err := nodeAddresses(context.TODO(), &srv, ports, nil, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -468,7 +468,7 @@ func TestNodeAddressesCustomPublicNetwork(t *testing.T) { }, } - addrs, err := nodeAddresses(&srv, ports, nil, networkingOpts) + addrs, err := nodeAddresses(context.TODO(), &srv, ports, nil, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -546,7 +546,7 @@ func TestNodeAddressesCustomPublicNetworkWithIntersectingFixedIP(t *testing.T) { }, } - addrs, err := nodeAddresses(&srv, ports, nil, networkingOpts) + addrs, err := nodeAddresses(context.TODO(), &srv, ports, nil, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -631,7 +631,7 @@ func TestNodeAddressesMultipleCustomInternalNetworks(t *testing.T) { }, } - addrs, err := nodeAddresses(&srv, ports, nil, networkingOpts) + addrs, err := nodeAddresses(context.TODO(), &srv, ports, nil, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -716,7 +716,7 @@ func TestNodeAddressesOneInternalNetwork(t *testing.T) { }, } - addrs, err := nodeAddresses(&srv, ports, nil, networkingOpts) + addrs, err := nodeAddresses(context.TODO(), &srv, ports, nil, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -793,7 +793,7 @@ func TestNodeAddressesIPv6Disabled(t *testing.T) { }, } - addrs, err := nodeAddresses(&srv, ports, nil, networkingOpts) + addrs, err := nodeAddresses(context.TODO(), &srv, ports, nil, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } @@ -875,7 +875,7 @@ func TestNodeAddressesWithAddressSortOrderOptions(t *testing.T) { }, } - addrs, err := nodeAddresses(&srv, ports, nil, networkingOpts) + addrs, err := nodeAddresses(context.TODO(), &srv, ports, nil, networkingOpts) if err != nil { t.Fatalf("nodeAddresses returned error: %v", err) } diff --git a/pkg/openstack/routes_test.go b/pkg/openstack/routes_test.go index c7d89c9864..8066233488 100644 --- a/pkg/openstack/routes_test.go +++ b/pkg/openstack/routes_test.go @@ -42,8 +42,7 @@ func TestRoutes(t *testing.T) { } vms := getServers(os) - _, err = os.InstanceID() - if err != nil || len(vms) == 0 { + if len(vms) == 0 { t.Skipf("Please run this test in an OpenStack vm or create at least one VM in OpenStack before you run this test.") } diff --git a/pkg/util/util.go b/pkg/util/util.go index cad6df9f8f..d3bce47d79 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -7,6 +7,7 @@ import ( "regexp" "strings" "time" + "unicode" "github.com/container-storage-interface/spec/lib/go/csi" v1 "k8s.io/api/core/v1" @@ -174,3 +175,12 @@ func SanitizeLabel(input string) string { return sanitized } + +// SplitTrim splits a string of values separated by sep rune into a slice of +// strings with trimmed spaces. +func SplitTrim(s string, sep rune) []string { + f := func(c rune) bool { + return unicode.IsSpace(c) || c == sep + } + return strings.FieldsFunc(s, f) +}