Merge pull request #1016 from iamemilio/port_tags

Port Tagging Regression

Author: k8s-ci-robot

pkg/cloud/services/networking/port.go

@@ -161,15 +161,20 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
 		record.Warnf(eventObject, "FailedCreatePort", "Failed to create port %s: %v", portName, err)
 		return nil, err
 	}
-
+	if len(tags) > 0 {
+		if err = s.replaceAllAttributesTags(eventObject, portResource, port.ID, tags); err != nil {
+			record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace port tags %s: %v", portName, err)
+			return nil, err
+		}
+	}
 	record.Eventf(eventObject, "SuccessfulCreatePort", "Created port %s with id %s", port.Name, port.ID)
 	if portOpts.Trunk != nil && *portOpts.Trunk {
 		trunk, err := s.getOrCreateTrunk(eventObject, clusterName, 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 {
+		if err = s.replaceAllAttributesTags(eventObject, trunkResource, trunk.ID, tags); err != nil {
 			record.Warnf(eventObject, "FailedReplaceTags", "Failed to replace trunk tags %s: %v", portName, err)
 			return nil, err
 		}

pkg/cloud/services/networking/service.go

@@ -22,13 +22,18 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/gophercloud/openstack"
+	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags"
 	"github.com/gophercloud/utils/openstack/clientconfig"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/provider"
+	"sigs.k8s.io/cluster-api-provider-openstack/pkg/record"
 )
 
 const (
 	networkPrefix string = "k8s-clusterapi"
+	trunkResource string = "trunks"
+	portResource  string = "ports"
 )
 
 // Service interfaces with the OpenStack Networking API.
@@ -78,3 +83,23 @@ func NewTestService(projectID string, client NetworkClient, logger logr.Logger)
 		logger:    logger,
 	}
 }
+
+// replaceAllAttributesTags replaces all tags on a neworking resource.
+// the value of resourceType must match one of the allowed constants: trunkResource or portResource.
+func (s *Service) replaceAllAttributesTags(eventObject runtime.Object, resourceType string, resourceID string, tags []string) error {
+	if resourceType != trunkResource && resourceType != portResource {
+		record.Warnf(eventObject, "FailedReplaceAllAttributesTags", "Invalid resourceType argument in function call")
+		panic(fmt.Errorf("invalid argument: resourceType, %s, does not match allowed arguments: %s or %s", resourceType, trunkResource, portResource))
+	}
+
+	_, err := s.client.ReplaceAllAttributesTags(resourceType, resourceID, attributestags.ReplaceAllOpts{
+		Tags: tags,
+	})
+	if err != nil {
+		record.Warnf(eventObject, "FailedReplaceAllAttributesTags", "Failed to replace all attributestags, %s: %v", resourceID, err)
+		return err
+	}
+
+	record.Eventf(eventObject, "SuccessfulReplaceAllAttributeTags", "Replaced all attributestags for %s with tags %s", resourceID, tags)
+	return nil
+}

pkg/cloud/services/networking/trunk.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags"
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/trunks"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/cluster-api/util"
@@ -78,19 +77,6 @@ func (s *Service) getOrCreateTrunk(eventObject runtime.Object, clusterName, trun
 	return trunk, nil
 }
 
-func (s *Service) replaceAllAttributesTags(eventObject runtime.Object, trunkID string, tags []string) error {
-	_, err := s.client.ReplaceAllAttributesTags("trunks", trunkID, attributestags.ReplaceAllOpts{
-		Tags: tags,
-	})
-	if err != nil {
-		record.Warnf(eventObject, "FailedReplaceAllAttributesTags", "Failed to replace all attributestags, trunk %s: %v", trunkID, err)
-		return err
-	}
-
-	record.Eventf(eventObject, "SuccessfulReplaceAllAttributeTags", "Replaced all attributestags %s with tags %s", trunkID, tags)
-	return nil
-}
-
 func (s *Service) DeleteTrunk(eventObject runtime.Object, portID string) error {
 	listOpts := trunks.ListOpts{
 		PortID: portID,

test/e2e/suites/e2e/e2e_test.go

@@ -36,6 +36,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apimachinerytypes "k8s.io/apimachinery/pkg/types"
+	utilrand "k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/utils/pointer"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
 	bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4"
@@ -153,26 +154,30 @@ var _ = Describe("e2e tests", func() {
 				{Description: "primary"},
 			}
 
+			testTag := utilrand.String(6)
+			machineTags := []string{testTag}
+
 			// Note that as the bootstrap config does not have cloud.conf, the node will not be added to the cluster.
 			// We still expect the port for the machine to be created.
 			framework.CreateMachineDeployment(ctx, framework.CreateMachineDeploymentInput{
 				Creator:                 e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
 				MachineDeployment:       makeMachineDeployment(namespace.Name, md3Name, clusterName, "", 1),
 				BootstrapConfigTemplate: makeJoinBootstrapConfigTemplate(namespace.Name, md3Name),
-				InfraMachineTemplate:    makeOpenStackMachineTemplateWithPortOptions(namespace.Name, clusterName, md3Name, customPortOptions),
+				InfraMachineTemplate:    makeOpenStackMachineTemplateWithPortOptions(namespace.Name, clusterName, md3Name, customPortOptions, machineTags),
 			})
 
 			shared.Byf("Waiting for custom port to be created")
 			var plist []ports.Port
 			var err error
 			Eventually(func() int {
-				plist, err = shared.DumpOpenStackPorts(e2eCtx, ports.ListOpts{Description: "primary"})
+				plist, err = shared.DumpOpenStackPorts(e2eCtx, ports.ListOpts{Description: "primary", Tags: testTag})
 				Expect(err).To(BeNil())
 				return len(plist)
 			}, e2eCtx.E2EConfig.GetIntervals(specName, "wait-worker-nodes")...).Should(Equal(1))
 
 			port := plist[0]
 			Expect(port.Description).To(Equal("primary"))
+			Expect(port.Tags).To(ContainElement(testTag))
 		})
 		It("It should be creatable and deletable", func() {
 			shared.Byf("Creating a cluster")
@@ -465,7 +470,7 @@ func makeOpenStackMachineTemplate(namespace, clusterName, name string, subnetID
 	}
 }
 
-func makeOpenStackMachineTemplateWithPortOptions(namespace, clusterName, name string, portOpts *[]infrav1.PortOpts) *infrav1.OpenStackMachineTemplate {
+func makeOpenStackMachineTemplateWithPortOptions(namespace, clusterName, name string, portOpts *[]infrav1.PortOpts, machineTags []string) *infrav1.OpenStackMachineTemplate {
 	return &infrav1.OpenStackMachineTemplate{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -483,6 +488,7 @@ func makeOpenStackMachineTemplateWithPortOptions(namespace, clusterName, name st
 						Name: fmt.Sprintf("%s-cloud-config", clusterName),
 					},
 					Ports: *portOpts,
+					Tags:  machineTags,
 				},
 			},
 		},