Allow Trunk configuration at a Port level.

Signed-off-by: Anwar Hassen <[email protected]>

api/v1alpha4/types.go

@@ -130,6 +130,8 @@ type PortOpts struct {
 	ProjectID           string        `json:"projectId,omitempty"`
 	SecurityGroups      *[]string     `json:"securityGroups,omitempty"`
 	AllowedAddressPairs []AddressPair `json:"allowedAddressPairs,omitempty"`
+	// Enables and disables trunk at port level. If not provided, openStackMachine.Spec.Trunk is inherited.
+	Trunk *bool `json:"trunk,omitempty"`
 
 	// The ID of the host where the port is allocated
 	HostID string `json:"hostId,omitempty"`

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

@@ -1314,6 +1314,10 @@ spec:
                               type: array
                             tenantId:
                               type: string
+                            trunk:
+                              description: Enables and disables trunk at port level.
+                                If not provided, openStackMachine.Spec.Trunk is inherited.
+                              type: boolean
                             vnicType:
                               description: The virtual network interface card (vNIC)
                                 type that is bound to the neutron port.
@@ -1755,6 +1759,10 @@ spec:
                               type: array
                             tenantId:
                               type: string
+                            trunk:
+                              description: Enables and disables trunk at port level.
+                                If not provided, openStackMachine.Spec.Trunk is inherited.
+                              type: boolean
                             vnicType:
                               description: The virtual network interface card (vNIC)
                                 type that is bound to the neutron port.
@@ -2024,6 +2032,10 @@ spec:
                         type: array
                       tenantId:
                         type: string
+                      trunk:
+                        description: Enables and disables trunk at port level. If
+                          not provided, openStackMachine.Spec.Trunk is inherited.
+                        type: boolean
                       vnicType:
                         description: The virtual network interface card (vNIC) type
                           that is bound to the neutron port.
@@ -2205,6 +2217,10 @@ spec:
                         type: array
                       tenantId:
                         type: string
+                      trunk:
+                        description: Enables and disables trunk at port level. If
+                          not provided, openStackMachine.Spec.Trunk is inherited.
+                        type: boolean
                       vnicType:
                         description: The virtual network interface card (vNIC) type
                           that is bound to the neutron port.

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

@@ -296,6 +296,11 @@ spec:
                                       type: array
                                     tenantId:
                                       type: string
+                                    trunk:
+                                      description: Enables and disables trunk at port
+                                        level. If not provided, openStackMachine.Spec.Trunk
+                                        is inherited.
+                                      type: boolean
                                     vnicType:
                                       description: The virtual network interface card
                                         (vNIC) type that is bound to the neutron port.

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

@@ -604,6 +604,10 @@ spec:
                       type: array
                     tenantId:
                       type: string
+                    trunk:
+                      description: Enables and disables trunk at port level. If not
+                        provided, openStackMachine.Spec.Trunk is inherited.
+                      type: boolean
                     vnicType:
                       description: The virtual network interface card (vNIC) type
                         that is bound to the neutron port.

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

@@ -553,6 +553,10 @@ spec:
                               type: array
                             tenantId:
                               type: string
+                            trunk:
+                              description: Enables and disables trunk at port level.
+                                If not provided, openStackMachine.Spec.Trunk is inherited.
+                              type: boolean
                             vnicType:
                               description: The virtual network interface card (vNIC)
                                 type that is bound to the neutron port.

pkg/cloud/services/compute/instance.go

@@ -152,19 +152,24 @@ func (s *Service) constructNetworks(openStackCluster *infrav1.OpenStackCluster,
 		}
 	}
 	for i, port := range openStackMachine.Spec.Ports {
+		pOpts := &openStackMachine.Spec.Ports[i]
+		// No Trunk field specified for the port, inherits openStackMachine.Spec.Trunk.
+		if pOpts.Trunk == nil {
+			pOpts.Trunk = &openStackMachine.Spec.Trunk
+		}
 		if port.NetworkID != "" {
 			nets = append(nets, infrav1.Network{
 				ID:       port.NetworkID,
 				Subnet:   &infrav1.Subnet{},
-				PortOpts: &openStackMachine.Spec.Ports[i],
+				PortOpts: pOpts,
 			})
 		} else {
 			nets = append(nets, infrav1.Network{
 				ID: openStackCluster.Status.Network.ID,
 				Subnet: &infrav1.Subnet{
 					ID: openStackCluster.Status.Network.Subnet.ID,
 				},
-				PortOpts: &openStackMachine.Spec.Ports[i],
+				PortOpts: pOpts,
 			})
 		}
 	}
@@ -175,6 +180,9 @@ func (s *Service) constructNetworks(openStackCluster *infrav1.OpenStackCluster,
 			Subnet: &infrav1.Subnet{
 				ID: openStackCluster.Status.Network.Subnet.ID,
 			},
+			PortOpts: &infrav1.PortOpts{
+				Trunk: &openStackMachine.Spec.Trunk,
+			},
 		}}
 	}
 	return &nets, nil
@@ -188,24 +196,16 @@ func (s *Service) createInstance(eventObject runtime.Object, clusterName string,
 		if network.ID == "" {
 			return nil, fmt.Errorf("no network was found or provided. Please check your machine configuration and try again")
 		}
-
+		iTags := []string{}
+		if len(instance.Tags) > 0 {
+			iTags = instance.Tags
+		}
 		portName := getPortName(instance.Name, network.PortOpts, i)
-		port, err := s.getOrCreatePort(eventObject, clusterName, portName, network, instance.SecurityGroups)
+		port, err := s.getOrCreatePort(eventObject, clusterName, portName, network, instance.SecurityGroups, iTags)
 		if err != nil {
 			return nil, err
 		}
 
-		if instance.Trunk {
-			trunk, err := s.getOrCreateTrunk(eventObject, clusterName, instance.Name, port.ID)
-			if err != nil {
-				return nil, err
-			}
-
-			if err = s.replaceAllAttributesTags(eventObject, trunk.ID, instance.Tags); err != nil {
-				return nil, err
-			}
-		}
-
 		for _, fip := range port.FixedIPs {
 			if fip.SubnetID == instance.Subnet {
 				accessIPv4 = fip.IPAddress
@@ -424,7 +424,7 @@ func (s *Service) getServerNetworks(networkParams []infrav1.NetworkParam) ([]inf
 	return nets, nil
 }
 
-func (s *Service) getOrCreatePort(eventObject runtime.Object, clusterName string, portName string, net infrav1.Network, instanceSecurityGroups *[]string) (*ports.Port, error) {
+func (s *Service) getOrCreatePort(eventObject runtime.Object, clusterName string, portName string, net infrav1.Network, instanceSecurityGroups *[]string, tags []string) (*ports.Port, error) {
 	mc := metrics.NewMetricPrometheusContext("port", "list")
 	allPages, err := ports.List(s.networkClient, ports.ListOpts{
 		Name:      portName,
@@ -527,21 +527,34 @@ func (s *Service) getOrCreatePort(eventObject runtime.Object, clusterName string
 	}
 
 	record.Eventf(eventObject, "SuccessfulCreatePort", "Created port %s with id %s", port.Name, port.ID)
+	if portOpts.Trunk != nil && *portOpts.Trunk {
+		trunkDescription := names.GetDescription(clusterName)
+		trunk, err := s.getOrCreateTrunk(eventObject, trunkDescription, port.Name, port.ID)
+		if err != nil {
+			record.Warnf(eventObject, "FailedCreateTrunk", "Failed to create trunk for port %s: %v", portName, err)
+			return nil, err
+		}
+		if err = s.replaceAllAttributesTags(eventObject, trunk.ID, tags); err != nil {
+			record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace trunk tags %s: %v", portName, err)
+			return nil, err
+		}
+	}
+
 	return port, nil
 }
 
-func (s *Service) getOrCreateTrunk(eventObject runtime.Object, clusterName, trunkName, portID string) (*trunks.Trunk, error) {
+func (s *Service) getOrCreateTrunk(eventObject runtime.Object, description, trunkName, portID string) (*trunks.Trunk, error) {
 	mc := metrics.NewMetricPrometheusContext("trunk", "list")
 	allPages, err := trunks.List(s.networkClient, trunks.ListOpts{
 		Name:   trunkName,
 		PortID: portID,
 	}).AllPages()
 	if mc.ObserveRequest(err) != nil {
-		return nil, fmt.Errorf("searching for existing trunk for server: %v", err)
+		return nil, fmt.Errorf("searching for existing trunk for port: %v", err)
 	}
 	trunkList, err := trunks.ExtractTrunks(allPages)
 	if err != nil {
-		return nil, fmt.Errorf("searching for existing trunk for server: %v", err)
+		return nil, fmt.Errorf("searching for existing trunk for port: %v", err)
 	}
 
 	if len(trunkList) != 0 {
@@ -551,7 +564,7 @@ func (s *Service) getOrCreateTrunk(eventObject runtime.Object, clusterName, trun
 	trunkCreateOpts := trunks.CreateOpts{
 		Name:        trunkName,
 		PortID:      portID,
-		Description: names.GetDescription(clusterName),
+		Description: description,
 	}
 
 	mc = metrics.NewMetricPrometheusContext("trunk", "create")

test/e2e/shared/openstack.go

@@ -34,6 +34,8 @@ import (
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/keypairs"
 	"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
 	"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/portsecurity"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/trunks"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/ports"
 	"github.com/gophercloud/utils/openstack/clientconfig"
 	. "github.com/onsi/ginkgo"
@@ -45,6 +47,11 @@ import (
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/provider"
 )
 
+type PortWithPortSecurityExtensions struct {
+	ports.Port
+	portsecurity.PortSecurityExt
+}
+
 // ensureSSHKeyPair ensures A SSH key is present under the name.
 func ensureSSHKeyPair(e2eCtx *E2EContext) {
 	Byf("Ensuring presence of SSH key %q in OpenStack", DefaultSSHKeyPairName)
@@ -124,7 +131,7 @@ func dumpOpenStackImages(providerClient *gophercloud.ProviderClient, clientOpts
 	return nil
 }
 
-func DumpOpenStackPorts(e2eCtx *E2EContext, filter ports.ListOpts) (*[]ports.Port, error) {
+func DumpOpenStackPorts(e2eCtx *E2EContext, filter ports.ListOpts) (*[]PortWithPortSecurityExtensions, error) {
 	providerClient, clientOpts, err := getProviderClient(e2eCtx)
 	if err != nil {
 		_, _ = fmt.Fprintf(GinkgoWriter, "error creating provider client: %s\n", err)
@@ -142,13 +149,44 @@ func DumpOpenStackPorts(e2eCtx *E2EContext, filter ports.ListOpts) (*[]ports.Por
 	if err != nil {
 		return nil, fmt.Errorf("error getting ports: %s", err)
 	}
-	portsList, err := ports.ExtractPorts(allPages)
+	var portsList []PortWithPortSecurityExtensions
+	err = ports.ExtractPortsInto(allPages, &portsList)
 	if err != nil {
 		return nil, fmt.Errorf("error extracting ports: %s", err)
 	}
 	return &portsList, nil
 }
 
+func DumpOpenStackTrunks(e2eCtx *E2EContext, portID string) (*trunks.Trunk, error) {
+	providerClient, clientOpts, err := getProviderClient(e2eCtx)
+	if err != nil {
+		_, _ = fmt.Fprintf(GinkgoWriter, "error creating provider client: %s\n", err)
+		return nil, err
+	}
+	networkClient, err := openstack.NewNetworkV2(providerClient, gophercloud.EndpointOpts{
+		Region: clientOpts.RegionName,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("error creating network client: %s", err)
+	}
+
+	allPages, err := trunks.List(networkClient, trunks.ListOpts{
+		PortID: portID,
+	}).AllPages()
+	if err != nil {
+		return nil, fmt.Errorf("error getting trunks: %s", err)
+	}
+	trunkList, err := trunks.ExtractTrunks(allPages)
+	if err != nil {
+		return nil, fmt.Errorf("searching for existing trunk for port: %v", err)
+	}
+
+	if len(trunkList) != 0 {
+		return &trunkList[0], nil
+	}
+	return nil, nil
+}
+
 // getOpenStackServers gets all OpenStack servers at once, to save on DescribeInstances
 // calls.
 func getOpenStackServers(e2eCtx *E2EContext) (map[string]server, error) {

test/e2e/suites/e2e/e2e_test.go

@@ -141,8 +141,16 @@ var _ = Describe("e2e tests", func() {
 
 			shared.Byf("Creating MachineDeployment with custom port options")
 			md3Name := clusterName + "-md-3"
+			// tTrue := true
+			disablePSTrue := true
+			addPair := infrav1.AddressPair{IPAddress: "10.10.10.10/24", MACAddress: "a4:b8:34:2b:ab:01"}
 			customPortOptions := &[]infrav1.PortOpts{
-				{Description: "primary"},
+				{
+					Description: "primary",
+					// Trunk:               &tTrue,
+					DisablePortSecurity: &disablePSTrue,
+					AllowedAddressPairs: []infrav1.AddressPair{addPair},
+				},
 			}
 
 			// Note that as the bootstrap config does not have cloud.conf, the node will not be added to the cluster.
@@ -155,7 +163,7 @@ var _ = Describe("e2e tests", func() {
 			})
 
 			shared.Byf("Waiting for custom port to be created")
-			var plist *[]ports.Port
+			var plist *[]shared.PortWithPortSecurityExtensions
 			var err error
 			Eventually(func() int {
 				plist, err = shared.DumpOpenStackPorts(e2eCtx, ports.ListOpts{Description: "primary"})
@@ -164,7 +172,18 @@ var _ = Describe("e2e tests", func() {
 			}, e2eCtx.E2EConfig.GetIntervals(specName, "wait-worker-nodes")...).Should(Equal(1))
 
 			port := (*plist)[0]
-			Expect(port.Description).To(Equal("primary"))
+			Expect(port.Port.Description).To(Equal("primary"))
+			Expect(port.PortSecurityEnabled).To(BeFalse())
+			Expect(len(port.Port.AllowedAddressPairs)).To(Equal(0))
+			// Trunk assertion
+			// var trunk *trunks.Trunk
+			// Eventually(func() int {
+			// 	trunk, err = shared.DumpOpenStackTrunks(e2eCtx, port.Port.ID)
+			// 	Expect(err).To(BeNil())
+			// 	Expect(trunk).NotTo(BeNil())
+			// 	return 0
+			// }, e2eCtx.E2EConfig.GetIntervals(specName, "wait-worker-nodes")...).Should(Equal(1))
+			// Expect(trunk.PortID).To(Equal(port.Port.ID))
 		})
 		It("It should be creatable and deletable", func() {
 			shared.Byf("Creating a cluster")