Merge pull request #573 from hidekazuna/discover_external

✨Add external network discovery

Author: k8s-ci-robot

api/v1alpha3/openstackcluster_types.go

@@ -59,6 +59,7 @@ type OpenStackClusterSpec struct {
 	ExternalRouterIPs []ExternalRouterIPParam `json:"externalRouterIPs,omitempty"`
 	// ExternalNetworkID is the ID of an external OpenStack Network. This is necessary
 	// to get public internet to the VMs.
+	// +optional
 	ExternalNetworkID string `json:"externalNetworkId,omitempty"`
 
 	// UseOctavia is weather LoadBalancer Service is Octavia or not
@@ -129,6 +130,9 @@ type OpenStackClusterStatus struct {
 	// It includes Subnets and Router.
 	Network *Network `json:"network,omitempty"`
 
+	// External Network contains information about the created OpenStack external network.
+	ExternalNetwork *Network `json:"externalNetwork,omitempty"`
+
 	// FailureDomains represent OpenStack availability zones
 	FailureDomains clusterv1.FailureDomains `json:"failureDomains,omitempty"`
 

api/v1alpha3/zz_generated.deepcopy.go

@@ -412,6 +412,63 @@ spec:
                 - name
                 - rules
                 type: object
+              externalNetwork:
+                description: External Network contains information about the created
+                  OpenStack external network.
+                properties:
+                  apiServerLoadBalancer:
+                    description: Be careful when using APIServerLoadBalancer, because
+                      this field is optional and therefore not set in all cases
+                    properties:
+                      id:
+                        type: string
+                      internalIP:
+                        type: string
+                      ip:
+                        type: string
+                      name:
+                        type: string
+                    required:
+                    - id
+                    - internalIP
+                    - ip
+                    - name
+                    type: object
+                  id:
+                    type: string
+                  name:
+                    type: string
+                  router:
+                    description: Router represents basic information about the associated
+                      OpenStack Neutron Router
+                    properties:
+                      id:
+                        type: string
+                      name:
+                        type: string
+                    required:
+                    - id
+                    - name
+                    type: object
+                  subnet:
+                    description: Subnet represents basic information about the associated
+                      OpenStack Neutron Subnet
+                    properties:
+                      cidr:
+                        type: string
+                      id:
+                        type: string
+                      name:
+                        type: string
+                    required:
+                    - cidr
+                    - id
+                    - name
+                    type: object
+                required:
+                - id
+                - name
+                type: object
               failureDomains:
                 additionalProperties:
                   description: FailureDomainSpec is the Schema for Cluster API failure

config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml

@@ -214,35 +214,6 @@ func (r *OpenStackClusterReconciler) reconcileNormal(ctx context.Context, log lo
 		return reconcile.Result{}, err
 	}
 
-	// Set APIEndpoints so the Cluster API Cluster Controller can pull them
-	if openStackCluster.Spec.ManagedAPIServerLoadBalancer {
-		openStackCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
-			Host: openStackCluster.Spec.APIServerLoadBalancerFloatingIP,
-			Port: int32(openStackCluster.Spec.APIServerLoadBalancerPort),
-		}
-	} else {
-		controlPlaneMachine, err := getControlPlaneMachine(r.Client)
-		if err != nil {
-			return ctrl.Result{}, errors.Errorf("failed to get control plane machine: %v", err)
-		}
-		if controlPlaneMachine != nil {
-			var apiPort int
-			if cluster.Spec.ClusterNetwork.APIServerPort == nil {
-				log.Info("No API endpoint given, default to 6443")
-				apiPort = 6443
-			} else {
-				apiPort = int(*cluster.Spec.ClusterNetwork.APIServerPort)
-			}
-
-			openStackCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
-				Host: controlPlaneMachine.Spec.FloatingIP,
-				Port: int32(apiPort),
-			}
-		} else {
-			log.Info("No control plane node found yet, could not write OpenStackCluster.Spec.ControlPlaneEndpoint")
-		}
-	}
-
 	availabilityZones, err := computeService.GetAvailabilityZones()
 	if err != nil {
 		return ctrl.Result{}, err
@@ -292,6 +263,12 @@ func (r *OpenStackClusterReconciler) reconcileNetworkComponents(log logr.Logger,
 	}
 
 	log.Info("Reconciling network components")
+
+	err = networkingService.ReconcileExternalNetwork(openStackCluster)
+	if err != nil {
+		return errors.Errorf("failed to reconcile external network: %v", err)
+	}
+
 	if openStackCluster.Spec.NodeCIDR == "" {
 		log.V(4).Info("No need to reconcile network, searching network and subnet instead")
 
@@ -337,6 +314,35 @@ func (r *OpenStackClusterReconciler) reconcileNetworkComponents(log logr.Logger,
 		}
 	}
 
+	if openStackCluster.Spec.ControlPlaneEndpoint.IsZero() {
+		var controlPlaneEndpointHost string
+		var port int32
+		if openStackCluster.Spec.ManagedAPIServerLoadBalancer {
+			controlPlaneEndpointHost = openStackCluster.Spec.APIServerLoadBalancerFloatingIP
+			if openStackCluster.Spec.APIServerLoadBalancerPort == 0 {
+				port = 6443
+			} else {
+				port = int32(openStackCluster.Spec.APIServerLoadBalancerPort)
+			}
+		} else {
+			controlPlaneEndpointHost = openStackCluster.Spec.ControlPlaneEndpoint.Host
+			if openStackCluster.Spec.ControlPlaneEndpoint.Port == 0 {
+				port = 6443
+			} else {
+				port = openStackCluster.Spec.ControlPlaneEndpoint.Port
+			}
+		}
+		fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, controlPlaneEndpointHost)
+		if err != nil {
+			return errors.Errorf("Floating IP cannot be got or created: %v", err)
+		}
+		// Set APIEndpoints so the Cluster API Cluster Controller can pull them
+		openStackCluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
+			Host: fp.FloatingIP,
+			Port: port,
+		}
+	}
+
 	if openStackCluster.Spec.ManagedAPIServerLoadBalancer {
 		err = loadBalancerService.ReconcileLoadBalancer(clusterName, openStackCluster)
 		if err != nil {
@@ -359,33 +365,3 @@ func (r *OpenStackClusterReconciler) SetupWithManager(mgr ctrl.Manager, options
 		WithEventFilter(pausePredicates).
 		Complete(r)
 }
-
-func getControlPlaneMachine(client client.Client) (*infrav1.OpenStackMachine, error) {
-	machines := &clusterv1.MachineList{}
-	if err := client.List(context.TODO(), machines); err != nil {
-		return nil, err
-	}
-	openStackMachines := &infrav1.OpenStackMachineList{}
-	if err := client.List(context.TODO(), openStackMachines); err != nil {
-		return nil, err
-	}
-
-	var controlPlaneMachine *clusterv1.Machine
-	for _, machine := range machines.Items {
-		m := machine
-		if util.IsControlPlaneMachine(&m) {
-			controlPlaneMachine = &m
-			break
-		}
-	}
-	if controlPlaneMachine == nil {
-		return nil, nil
-	}
-
-	for _, openStackMachine := range openStackMachines.Items {
-		if openStackMachine.Name == controlPlaneMachine.Name {
-			return &openStackMachine, nil
-		}
-	}
-	return nil, nil
-}

controllers/openstackcluster_controller.go

@@ -335,20 +335,23 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, logger
 		return ctrl.Result{}, nil
 	}
 
-	if openStackMachine.Spec.FloatingIP != "" {
-		err = r.reconcileFloatingIP(computeService, networkingService, instance, openStackMachine, openStackCluster)
-		if err != nil {
-			handleUpdateMachineError(logger, openStackMachine, errors.Errorf("FloatingIP cannot be reconciled: %v", err))
-			return ctrl.Result{}, nil
-		}
-	}
-
 	if openStackCluster.Spec.ManagedAPIServerLoadBalancer {
 		err = r.reconcileLoadBalancerMember(logger, osProviderClient, clientOpts, instance, clusterName, machine, openStackMachine, openStackCluster)
 		if err != nil {
 			handleUpdateMachineError(logger, openStackMachine, errors.Errorf("LoadBalancerMember cannot be reconciled: %v", err))
 			return ctrl.Result{}, nil
 		}
+	} else if util.IsControlPlaneMachine(machine) {
+		fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster.Spec.ControlPlaneEndpoint.Host)
+		if err != nil {
+			handleUpdateMachineError(logger, openStackMachine, errors.Errorf("Floating IP cannot be got or created: %v", err))
+			return ctrl.Result{}, nil
+		}
+		err = computeService.AssociateFloatingIP(instance.ID, fp.FloatingIP)
+		if err != nil {
+			handleUpdateMachineError(logger, openStackMachine, errors.Errorf("Floating IP cannot be associated: %v", err))
+			return ctrl.Result{}, nil
+		}
 	}
 
 	logger.Info("Reconciled Machine create successfully")
@@ -404,19 +407,6 @@ func getTimeout(name string, timeout int) time.Duration {
 	return time.Duration(timeout)
 }
 
-func (r *OpenStackMachineReconciler) reconcileFloatingIP(computeService *compute.Service, networkingService *networking.Service, instance *compute.Instance, openStackMachine *infrav1.OpenStackMachine, openStackCluster *infrav1.OpenStackCluster) error {
-	err := networkingService.CreateFloatingIPIfNecessary(openStackCluster, openStackMachine.Spec.FloatingIP)
-	if err != nil {
-		return fmt.Errorf("error creating floatingIP: %v", err)
-	}
-
-	err = computeService.AssociateFloatingIP(instance.ID, openStackMachine.Spec.FloatingIP)
-	if err != nil {
-		return fmt.Errorf("error associating floatingIP: %v", err)
-	}
-	return nil
-}
-
 func (r *OpenStackMachineReconciler) reconcileLoadBalancerMember(logger logr.Logger, osProviderClient *gophercloud.ProviderClient, clientOpts *clientconfig.ClientOpts, instance *compute.Instance, clusterName string, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, openStackCluster *infrav1.OpenStackCluster) error {
 	ip, err := getIPFromInstance(instance)
 	if err != nil {

controllers/openstackmachine_controller.go

@@ -30,25 +30,13 @@ import (
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
 	"k8s.io/apimachinery/pkg/util/wait"
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha3"
+	"sigs.k8s.io/cluster-api-provider-openstack/pkg/record"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
 	"sigs.k8s.io/cluster-api/util"
 )
 
 func (s *Service) ReconcileLoadBalancer(clusterName string, openStackCluster *infrav1.OpenStackCluster) error {
 
-	if openStackCluster.Spec.ExternalNetworkID == "" {
-		s.logger.V(3).Info("No need to create loadbalancer, due to missing ExternalNetworkID")
-		return nil
-	}
-	if openStackCluster.Spec.APIServerLoadBalancerFloatingIP == "" {
-		s.logger.V(3).Info("No need to create loadbalancer, due to missing APIServerLoadBalancerFloatingIP")
-		return nil
-	}
-	if openStackCluster.Spec.APIServerLoadBalancerPort == 0 {
-		s.logger.V(3).Info("No need to create loadbalancer, due to missing APIServerLoadBalancerPort")
-		return nil
-	}
-
 	loadBalancerName := fmt.Sprintf("%s-cluster-%s-%s", networkPrefix, clusterName, kubeapiLBSuffix)
 	s.logger.Info("Reconciling loadbalancer", "name", loadBalancerName)
 
@@ -73,39 +61,26 @@ func (s *Service) ReconcileLoadBalancer(clusterName string, openStackCluster *in
 		return err
 	}
 
-	// floating ip
-	fp, err := checkIfFloatingIPExists(s.networkingClient, openStackCluster.Spec.APIServerLoadBalancerFloatingIP)
+	fp, err := getOrCreateFloatingIP(s.networkingClient, openStackCluster, openStackCluster.Spec.ControlPlaneEndpoint.Host)
 	if err != nil {
 		return err
 	}
-	if fp == nil {
-		s.logger.Info("Creating floating ip", "ip", openStackCluster.Spec.APIServerLoadBalancerFloatingIP)
-		fpCreateOpts := &floatingips.CreateOpts{
-			FloatingIP:        openStackCluster.Spec.APIServerLoadBalancerFloatingIP,
-			FloatingNetworkID: openStackCluster.Spec.ExternalNetworkID,
-		}
-		fp, err = floatingips.Create(s.networkingClient, fpCreateOpts).Extract()
-		if err != nil {
-			return fmt.Errorf("error allocating floating IP: %s", err)
-		}
-	}
-
-	// associate floating ip
-	s.logger.Info("Associating floating ip", "ip", openStackCluster.Spec.APIServerLoadBalancerFloatingIP)
+	s.logger.Info("Associating floating ip", "ip", fp.FloatingIP)
 	fpUpdateOpts := &floatingips.UpdateOpts{
 		PortID: &lb.VipPortID,
 	}
 	fp, err = floatingips.Update(s.networkingClient, fp.ID, fpUpdateOpts).Extract()
 	if err != nil {
-		return fmt.Errorf("error allocating floating IP: %s", err)
+		return fmt.Errorf("error associating floating IP: %s", err)
 	}
 	err = waitForFloatingIP(s.logger, s.networkingClient, fp.ID, "ACTIVE")
 	if err != nil {
 		return err
 	}
+	record.Eventf(openStackCluster, "SuccessfulAssociateFloatingIP", "Associate floating IP %s with port %s", fp.FloatingIP, &lb.VipPortID)
 
 	// lb listener
-	portList := []int{openStackCluster.Spec.APIServerLoadBalancerPort}
+	portList := []int{int(openStackCluster.Spec.ControlPlaneEndpoint.Port)}
 	portList = append(portList, openStackCluster.Spec.APIServerLoadBalancerAdditionalPorts...)
 	for _, port := range portList {
 		lbPortObjectsName := fmt.Sprintf("%s-%d", loadBalancerName, port)
@@ -212,7 +187,7 @@ func (s *Service) ReconcileLoadBalancerMember(clusterName string, machine *clust
 	lbID := openStackCluster.Status.Network.APIServerLoadBalancer.ID
 	subnetID := openStackCluster.Status.Network.Subnet.ID
 
-	portList := []int{openStackCluster.Spec.APIServerLoadBalancerPort}
+	portList := []int{int(openStackCluster.Spec.ControlPlaneEndpoint.Port)}
 	portList = append(portList, openStackCluster.Spec.APIServerLoadBalancerAdditionalPorts...)
 	for _, port := range portList {
 		lbPortObjectsName := fmt.Sprintf("%s-%d", loadBalancerName, port)
@@ -415,7 +390,7 @@ func (s *Service) DeleteLoadBalancerMember(clusterName string, machine *clusterv
 
 	lbID := openStackCluster.Status.Network.APIServerLoadBalancer.ID
 
-	portList := []int{openStackCluster.Spec.APIServerLoadBalancerPort}
+	portList := []int{int(openStackCluster.Spec.ControlPlaneEndpoint.Port)}
 	portList = append(portList, openStackCluster.Spec.APIServerLoadBalancerAdditionalPorts...)
 	for _, port := range portList {
 		lbPortObjectsName := fmt.Sprintf("%s-%d", loadBalancerName, port)
@@ -470,6 +445,32 @@ func checkIfLbExists(client *gophercloud.ServiceClient, name string) (*loadbalan
 	return &lbList[0], nil
 }
 
+func getOrCreateFloatingIP(client *gophercloud.ServiceClient, openStackCluster *infrav1.OpenStackCluster, ip string) (*floatingips.FloatingIP, error) {
+	var fp *floatingips.FloatingIP
+	var err error
+	if ip != "" {
+		fp, err = checkIfFloatingIPExists(client, ip)
+		if err != nil {
+			return nil, err
+		}
+	}
+	if fp == nil {
+		fpCreateOpts := &floatingips.CreateOpts{
+			FloatingNetworkID: openStackCluster.Status.ExternalNetwork.ID,
+		}
+		if ip != "" {
+			fpCreateOpts.FloatingIP = ip
+		}
+		fp, err = floatingips.Create(client, fpCreateOpts).Extract()
+		if err != nil {
+			return nil, fmt.Errorf("error creating floating IP: %s", err)
+		}
+		record.Eventf(openStackCluster, "SuccessfulCreateFloatingIP", "Created floating IP %s with id %s", fp.FloatingIP, fp.ID)
+
+	}
+	return fp, nil
+}
+
 func checkIfFloatingIPExists(client *gophercloud.ServiceClient, ip string) (*floatingips.FloatingIP, error) {
 	allPages, err := floatingips.List(client, floatingips.ListOpts{FloatingIP: ip}).AllPages()
 	if err != nil {