From deb124921c9a3da1fd19bd1dd295da0283904613 Mon Sep 17 00:00:00 2001 From: Mike Fedosin Date: Mon, 20 Apr 2020 23:29:00 +0200 Subject: [PATCH] Bug 1824426: Allow to define primary ip address for machines This patch allows to set primary ip address for the machine based on the primary network tag "-primaryClusterNetwork". In the case of multiple attached networks this tag should allow CAPO to define which IP address to set as the primary one for machines. Now CAPO can't do this, because Neutron returns the list of networks in alphabetical order. --- pkg/cloud/openstack/clients/machineservice.go | 82 ++---------- pkg/cloud/openstack/clients/utils.go | 107 ++++++++++++++++ pkg/cloud/openstack/machine/actuator.go | 119 +++++++++++++++--- 3 files changed, 216 insertions(+), 92 deletions(-) create mode 100644 pkg/cloud/openstack/clients/utils.go diff --git a/pkg/cloud/openstack/clients/machineservice.go b/pkg/cloud/openstack/clients/machineservice.go index 756aeb0d93..a77e0d57e9 100644 --- a/pkg/cloud/openstack/clients/machineservice.go +++ b/pkg/cloud/openstack/clients/machineservice.go @@ -17,11 +17,8 @@ limitations under the License. package clients import ( - "crypto/tls" - "crypto/x509" "encoding/base64" "fmt" - "net/http" "time" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups" @@ -155,33 +152,12 @@ func GetCloudFromSecret(kubeClient kubernetes.Interface, namespace string, secre // TODO: Eventually we'll have a NewInstanceServiceFromCluster too func NewInstanceServiceFromMachine(kubeClient kubernetes.Interface, machine *machinev1.Machine) (*InstanceService, error) { - machineSpec, err := openstackconfigv1.MachineSpecFromProviderSpec(machine.Spec.ProviderSpec) + cloud, err := GetCloud(kubeClient, machine) if err != nil { - return nil, fmt.Errorf("Failed to get Machine Spec from Provider Spec: %v", err) - } - cloud := clientconfig.Cloud{} - if machineSpec.CloudsSecret != nil && machineSpec.CloudsSecret.Name != "" { - namespace := machineSpec.CloudsSecret.Namespace - if namespace == "" { - namespace = machine.Namespace - } - cloud, err = GetCloudFromSecret(kubeClient, namespace, machineSpec.CloudsSecret.Name, machineSpec.CloudName) - if err != nil { - return nil, fmt.Errorf("Failed to get cloud from secret: %v", err) - } - } - - cloudConfig, err := kubeClient.CoreV1().ConfigMaps("openshift-config").Get("cloud-provider-config", metav1.GetOptions{}) - if err != nil { - klog.Warningf("failed to get configmap openshift-config/cloud-provider-config from kubernetes api: %v", err) - return NewInstanceServiceFromCloud(cloud, nil) - } - - if cacert, ok := cloudConfig.Data["ca-bundle.pem"]; ok { - return NewInstanceServiceFromCloud(cloud, []byte(cacert)) + return nil, err } - return NewInstanceServiceFromCloud(cloud, nil) + return NewInstanceServiceFromCloud(cloud, GetCACertificate(kubeClient)) } func NewInstanceService() (*InstanceService, error) { @@ -190,51 +166,11 @@ func NewInstanceService() (*InstanceService, error) { } func NewInstanceServiceFromCloud(cloud clientconfig.Cloud, cert []byte) (*InstanceService, error) { - clientOpts := new(clientconfig.ClientOpts) - - if cloud.AuthInfo != nil { - clientOpts.AuthInfo = cloud.AuthInfo - clientOpts.AuthType = cloud.AuthType - clientOpts.Cloud = cloud.Cloud - clientOpts.RegionName = cloud.RegionName - } - - opts, err := clientconfig.AuthOptions(clientOpts) - + provider, err := GetProviderClient(cloud, cert) if err != nil { return nil, err } - opts.AllowReauth = true - - provider, err := openstack.NewClient(opts.IdentityEndpoint) - if err != nil { - return nil, fmt.Errorf("Create new provider client failed: %v", err) - } - - if cert != nil { - certPool, err := x509.SystemCertPool() - if err != nil { - return nil, fmt.Errorf("Create system cert pool failed: %v", err) - } - certPool.AppendCertsFromPEM(cert) - client := http.Client{ - Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - RootCAs: certPool, - }, - }, - } - provider.HTTPClient = client - } else { - klog.Infof("Cloud provider CA cert not provided, using system trust bundle") - } - - err = openstack.Authenticate(provider, *opts) - if err != nil { - return nil, fmt.Errorf("Failed to authenticate provider client: %v", err) - } - identityClient, err := openstack.NewIdentityV3(provider, gophercloud.EndpointOpts{ Region: "", }) @@ -242,7 +178,7 @@ func NewInstanceServiceFromCloud(cloud clientconfig.Cloud, cert []byte) (*Instan return nil, fmt.Errorf("Create identityClient err: %v", err) } serverClient, err := openstack.NewComputeV2(provider, gophercloud.EndpointOpts{ - Region: clientOpts.RegionName, + Region: cloud.RegionName, }) if err != nil { @@ -250,21 +186,21 @@ func NewInstanceServiceFromCloud(cloud clientconfig.Cloud, cert []byte) (*Instan } networkingClient, err := openstack.NewNetworkV2(provider, gophercloud.EndpointOpts{ - Region: clientOpts.RegionName, + Region: cloud.RegionName, }) if err != nil { return nil, fmt.Errorf("Create networkingClient err: %v", err) } imagesClient, err := openstack.NewImageServiceV2(provider, gophercloud.EndpointOpts{ - Region: clientOpts.RegionName, + Region: cloud.RegionName, }) if err != nil { return nil, fmt.Errorf("Create ImageClient err: %v", err) } volumeClient, err := openstack.NewBlockStorageV3(provider, gophercloud.EndpointOpts{ - Region: clientOpts.RegionName, + Region: cloud.RegionName, }) if err != nil { return nil, fmt.Errorf("Create VolumeClient err: %v", err) @@ -277,7 +213,7 @@ func NewInstanceServiceFromCloud(cloud clientconfig.Cloud, cert []byte) (*Instan networkClient: networkingClient, imagesClient: imagesClient, volumeClient: volumeClient, - regionName: clientOpts.RegionName, + regionName: cloud.RegionName, }, nil } diff --git a/pkg/cloud/openstack/clients/utils.go b/pkg/cloud/openstack/clients/utils.go new file mode 100644 index 0000000000..c77f7dbe29 --- /dev/null +++ b/pkg/cloud/openstack/clients/utils.go @@ -0,0 +1,107 @@ +package clients + +import ( + "crypto/tls" + "crypto/x509" + "fmt" + "net/http" + + "github.com/gophercloud/gophercloud" + "github.com/gophercloud/gophercloud/openstack" + "github.com/gophercloud/utils/openstack/clientconfig" + machinev1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/klog" + openstackconfigv1 "sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1" +) + +// GetCloud fetches cloud credentials from a secret and return a parsed Cloud structure +func GetCloud(kubeClient kubernetes.Interface, machine *machinev1.Machine) (clientconfig.Cloud, error) { + cloud := clientconfig.Cloud{} + machineSpec, err := openstackconfigv1.MachineSpecFromProviderSpec(machine.Spec.ProviderSpec) + if err != nil { + return cloud, fmt.Errorf("Failed to get Machine Spec from Provider Spec: %v", err) + } + + if machineSpec.CloudsSecret == nil || machineSpec.CloudsSecret.Name == "" { + return cloud, fmt.Errorf("Cloud secret name can't be empty") + } + + namespace := machineSpec.CloudsSecret.Namespace + if namespace == "" { + namespace = machine.Namespace + } + cloud, err = GetCloudFromSecret(kubeClient, namespace, machineSpec.CloudsSecret.Name, machineSpec.CloudName) + if err != nil { + return cloud, fmt.Errorf("Failed to get cloud from secret: %v", err) + } + + return cloud, nil +} + +// GetCACertificate gets the CA certificate from the configmap +func GetCACertificate(kubeClient kubernetes.Interface) []byte { + cloudConfig, err := kubeClient.CoreV1().ConfigMaps("openshift-config").Get("cloud-provider-config", metav1.GetOptions{}) + if err != nil { + klog.Warningf("failed to get configmap openshift-config/cloud-provider-config from kubernetes api: %v", err) + return nil + } + + if cacert, ok := cloudConfig.Data["ca-bundle.pem"]; ok { + return []byte(cacert) + } + + return nil +} + +// GetProviderClient returns an authenticated provider client based on values in the cloud structure +func GetProviderClient(cloud clientconfig.Cloud, cert []byte) (*gophercloud.ProviderClient, error) { + clientOpts := new(clientconfig.ClientOpts) + + if cloud.AuthInfo != nil { + clientOpts.AuthInfo = cloud.AuthInfo + clientOpts.AuthType = cloud.AuthType + clientOpts.Cloud = cloud.Cloud + clientOpts.RegionName = cloud.RegionName + } + + opts, err := clientconfig.AuthOptions(clientOpts) + + if err != nil { + return nil, err + } + + opts.AllowReauth = true + + provider, err := openstack.NewClient(opts.IdentityEndpoint) + if err != nil { + return nil, fmt.Errorf("Create new provider client failed: %v", err) + } + + if cert != nil { + certPool, err := x509.SystemCertPool() + if err != nil { + return nil, fmt.Errorf("Create system cert pool failed: %v", err) + } + certPool.AppendCertsFromPEM(cert) + client := http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + RootCAs: certPool, + }, + }, + } + provider.HTTPClient = client + } else { + klog.Infof("Cloud provider CA cert not provided, using system trust bundle") + } + + err = openstack.Authenticate(provider, *opts) + if err != nil { + return nil, fmt.Errorf("Failed to authenticate provider client: %v", err) + } + + return provider, nil +} diff --git a/pkg/cloud/openstack/machine/actuator.go b/pkg/cloud/openstack/machine/actuator.go index 867e014a26..74963bc331 100644 --- a/pkg/cloud/openstack/machine/actuator.go +++ b/pkg/cloud/openstack/machine/actuator.go @@ -29,6 +29,10 @@ import ( "k8s.io/apimachinery/pkg/api/equality" "k8s.io/client-go/tools/record" + "github.com/gophercloud/gophercloud" + gophercloudopenstack "github.com/gophercloud/gophercloud/openstack" + "github.com/gophercloud/gophercloud/openstack/networking/v2/networks" + clusterv1 "github.com/openshift/cluster-api/pkg/apis/cluster/v1alpha1" machinev1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1" apierrors "github.com/openshift/cluster-api/pkg/errors" @@ -296,7 +300,7 @@ func (oc *OpenstackClient) Create(ctx context.Context, cluster *clusterv1.Cluste } oc.eventRecorder.Eventf(machine, corev1.EventTypeNormal, "Created", "Created machine %v", machine.Name) - return oc.updateAnnotation(machine, instance.ID) + return oc.updateAnnotation(machine, instance.ID, clusterInfraName) } func (oc *OpenstackClient) Delete(ctx context.Context, cluster *clusterv1.Cluster, machine *machinev1.Machine) error { @@ -327,6 +331,11 @@ func (oc *OpenstackClient) Delete(ctx context.Context, cluster *clusterv1.Cluste } func (oc *OpenstackClient) Update(ctx context.Context, cluster *clusterv1.Cluster, machine *machinev1.Machine) error { + clusterInfra, err := oc.params.ConfigClient.Infrastructures().Get("cluster", metav1.GetOptions{}) + if err != nil { + return fmt.Errorf("Failed to retrieve cluster Infrastructure object: %v", err) + } + status, err := oc.instanceStatus(machine) if err != nil { return err @@ -352,7 +361,7 @@ func (oc *OpenstackClient) Update(ctx context.Context, cluster *clusterv1.Cluste return nil } - return oc.updateAnnotation(machine, instance.ID) + return oc.updateAnnotation(machine, instance.ID, clusterInfra.Status.InfrastructureName) } else { return fmt.Errorf("Cannot retrieve current state to update machine %v", machine.ObjectMeta.Name) } @@ -410,21 +419,23 @@ func (oc *OpenstackClient) Exists(ctx context.Context, cluster *clusterv1.Cluste return instance != nil, err } -func getIPFromInstance(instance *clients.Instance) (string, error) { +func getIPsFromInstance(instance *clients.Instance) (map[string]string, error) { if instance.AccessIPv4 != "" && net.ParseIP(instance.AccessIPv4) != nil { - return instance.AccessIPv4, nil + return map[string]string{ + "": instance.AccessIPv4, + }, nil } type networkInterface struct { Address string `json:"addr"` Version float64 `json:"version"` Type string `json:"OS-EXT-IPS:type"` } - var addrList []string + addrMap := map[string]string{} - for _, b := range instance.Addresses { + for networkName, b := range instance.Addresses { list, err := json.Marshal(b) if err != nil { - return "", fmt.Errorf("extract IP from instance err: %v", err) + return nil, fmt.Errorf("extract IP from instance err: %v", err) } var networks []interface{} json.Unmarshal(list, &networks) @@ -433,17 +444,80 @@ func getIPFromInstance(instance *clients.Instance) (string, error) { b, _ := json.Marshal(network) json.Unmarshal(b, &netInterface) if netInterface.Version == 4.0 { - if netInterface.Type == "floating" { - return netInterface.Address, nil - } - addrList = append(addrList, netInterface.Address) + addrMap[networkName] = netInterface.Address } } } - if len(addrList) != 0 { - return addrList[0], nil + if len(addrMap) == 0 { + return nil, fmt.Errorf("extract IP from instance err") + } + + return addrMap, nil +} + +func getNetworkByPrimaryNetworkTag(client *gophercloud.ServiceClient, primaryNetworkTag string) (networks.Network, error) { + opts := networks.ListOpts{ + Tags: primaryNetworkTag, + } + + allPages, err := networks.List(client, opts).AllPages() + if err != nil { + return networks.Network{}, err + } + + allNetworks, err := networks.ExtractNetworks(allPages) + if err != nil { + return networks.Network{}, err + } + + switch len(allNetworks) { + case 0: + return networks.Network{}, fmt.Errorf("There are no networks with primary network tag: %v", primaryNetworkTag) + case 1: + return allNetworks[0], nil + } + return networks.Network{}, fmt.Errorf("Too many networks with the same primary network tag: %v", primaryNetworkTag) +} + +func (oc *OpenstackClient) getPrimaryMachineIP(mapAddr map[string]string, machine *machinev1.Machine, clusterInfraName string) (string, error) { + // If there is only one network in the list, we consider it as the primary one + if len(mapAddr) == 1 { + for _, addr := range mapAddr { + return addr, nil + } + } + + cloud, err := clients.GetCloud(oc.params.KubeClient, machine) + if err != nil { + return "", err + } + + provider, err := clients.GetProviderClient(cloud, clients.GetCACertificate(oc.params.KubeClient)) + if err != nil { + return "", err + } + + networkingClient, err := gophercloudopenstack.NewNetworkV2(provider, gophercloud.EndpointOpts{ + Region: cloud.RegionName, + }) + if err != nil { + return "", err + } + + primaryNetworkTag := clusterInfraName + "-primaryClusterNetwork" + network, err := getNetworkByPrimaryNetworkTag(networkingClient, primaryNetworkTag) + if err != nil { + return "", err + } + + // We're looking for the tag to identify the primary network + for networkName, addr := range mapAddr { + if networkName == network.Name { + return addr, nil + } } - return "", fmt.Errorf("extract IP from instance err") + + return "", fmt.Errorf("No primary network was found for the machine %v", machine.Name) } // If the OpenstackClient has a client for updating Machine objects, this will set @@ -475,17 +549,24 @@ func (oc *OpenstackClient) handleMachineError(machine *machinev1.Machine, err *a return err } -func (oc *OpenstackClient) updateAnnotation(machine *machinev1.Machine, id string) error { +func (oc *OpenstackClient) updateAnnotation(machine *machinev1.Machine, instanceID string, clusterInfraName string) error { if machine.ObjectMeta.Annotations == nil { machine.ObjectMeta.Annotations = make(map[string]string) } - machine.ObjectMeta.Annotations[openstack.OpenstackIdAnnotationKey] = id + machine.ObjectMeta.Annotations[openstack.OpenstackIdAnnotationKey] = instanceID instance, _ := oc.instanceExists(machine) - ip, err := getIPFromInstance(instance) + mapAddr, err := getIPsFromInstance(instance) + if err != nil { + return err + } + + primaryIP, err := oc.getPrimaryMachineIP(mapAddr, machine, clusterInfraName) if err != nil { return err } - machine.ObjectMeta.Annotations[openstack.OpenstackIPAnnotationKey] = ip + klog.Infof("Found the primary address for the machine %v: %v", machine.Name, primaryIP) + + machine.ObjectMeta.Annotations[openstack.OpenstackIPAnnotationKey] = primaryIP machine.ObjectMeta.Annotations[MachineInstanceStateAnnotationName] = instance.Status if err := oc.client.Update(nil, machine); err != nil { @@ -495,7 +576,7 @@ func (oc *OpenstackClient) updateAnnotation(machine *machinev1.Machine, id strin networkAddresses := []corev1.NodeAddress{} networkAddresses = append(networkAddresses, corev1.NodeAddress{ Type: corev1.NodeInternalIP, - Address: ip, + Address: primaryIP, }) networkAddresses = append(networkAddresses, corev1.NodeAddress{