From 43df21f2b627ba85a3102921f1c4ff603e5da2dd Mon Sep 17 00:00:00 2001 From: Michael Captain Date: Wed, 6 Oct 2021 17:16:59 +0300 Subject: [PATCH] Add unit tests for getOrCreatePort func The getOrCreatePort func is quite complicated, taking many arguments and options and with conditional calls to OpenStack. This change adds unit tests for the function, which should make it clearer what OpenStack API calls are made under which conditions, and shows off all the supported features. --- pkg/cloud/services/networking/port_test.go | 371 +++++++++++++++++++++ 1 file changed, 371 insertions(+) create mode 100644 pkg/cloud/services/networking/port_test.go diff --git a/pkg/cloud/services/networking/port_test.go b/pkg/cloud/services/networking/port_test.go new file mode 100644 index 0000000000..92ec31812e --- /dev/null +++ b/pkg/cloud/services/networking/port_test.go @@ -0,0 +1,371 @@ +/* +Copyright 2021 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 networking + +import ( + "testing" + + "github.com/go-logr/logr" + "github.com/golang/mock/gomock" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/attributestags" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/portsbinding" + "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/onsi/gomega" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha4" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking/mock_networking" +) + +func Test_GetOrCreatePort(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + // Arbitrary GUIDs used in the tests + netID := "7fd24ceb-788a-441f-ad0a-d8e2f5d31a1d" + subnetID := "d9c88a6d-0b8c-48ff-8f0e-8d85a078c194" + portID1 := "50214c48-c09e-4a54-914f-97b40fd22802" + portID2 := "4c096384-f0a5-466d-9534-06a7ed281a79" + hostID := "825c1b11-3dca-4bfe-a2d8-a3cc1964c8d5" + tenantID := "62b523a7-f838-45fd-904f-d2db2bb58e04" + projectID := "063171b1-0595-4882-98cd-3ee79676ff87" + trunkID := "eb7541fa-5e2a-4cca-b2c3-dfa409b917ce" + + // Other arbitrary variables passed in to the tests + instanceSecurityGroups := []string{"instance-secgroup"} + portSecurityGroups := []string{"port-secgroup"} + + pointerToTrue := pointerTo(true) + pointerToFalse := pointerTo(false) + + tests := []struct { + name string + portName string + net infrav1.Network + instanceSecurityGroups *[]string + tags []string + expect func(m *mock_networking.MockNetworkClientMockRecorder) + // Note the 'wanted' port isn't so important, since it will be whatever we tell ListPort or CreatePort to return. + // Mostly in this test suite, we're checking that ListPort/CreatePort is called with the expected port opts. + want *ports.Port + wantErr bool + }{ + { + "gets and returns existing port if name matches", + "foo-port-1", + infrav1.Network{ + ID: netID, + Subnet: &infrav1.Subnet{}, + }, + nil, + []string{}, + func(m *mock_networking.MockNetworkClientMockRecorder) { + m. + ListPort(ports.ListOpts{ + Name: "foo-port-1", + NetworkID: netID, + }).Return([]ports.Port{{ + ID: portID1, + }}, nil) + }, + &ports.Port{ + ID: portID1, + }, + false, + }, + { + "errors if multiple matching ports are found", + "foo-port-1", + infrav1.Network{ + ID: netID, + Subnet: &infrav1.Subnet{}, + }, + nil, + []string{}, + func(m *mock_networking.MockNetworkClientMockRecorder) { + m. + ListPort(ports.ListOpts{ + Name: "foo-port-1", + NetworkID: netID, + }).Return([]ports.Port{ + { + ID: portID1, + NetworkID: netID, + Name: "foo-port-1", + }, + { + ID: portID2, + NetworkID: netID, + Name: "foo-port-2", + }, + }, nil) + }, + nil, + true, + }, + { + "creates port with defaults (description and secgroups) if not specified in portOpts", + "foo-port-1", + infrav1.Network{ + ID: netID, + PortOpts: &infrav1.PortOpts{}, + }, + &instanceSecurityGroups, + []string{}, + func(m *mock_networking.MockNetworkClientMockRecorder) { + // No ports found + m. + ListPort(ports.ListOpts{ + Name: "foo-port-1", + NetworkID: netID, + }).Return([]ports.Port{}, nil) + m. + CreatePort(portsbinding.CreateOptsExt{ + CreateOptsBuilder: ports.CreateOpts{ + Name: "foo-port-1", + Description: "Created by cluster-api-provider-openstack cluster test-cluster", + SecurityGroups: &instanceSecurityGroups, + NetworkID: netID, + AllowedAddressPairs: []ports.AddressPair{}, + }, + }).Return(&ports.Port{ID: portID1}, nil) + }, + &ports.Port{ID: portID1}, + false, + }, + { + "creates port with specified portOpts if no matching port exists", + "foo-port-bar", + infrav1.Network{ + ID: netID, + Subnet: &infrav1.Subnet{}, + PortOpts: &infrav1.PortOpts{ + NameSuffix: "bar", + Description: "this is a test port", + MACAddress: "fe:fe:fe:fe:fe:fe", + AdminStateUp: pointerToTrue, + FixedIPs: []infrav1.FixedIP{{ + SubnetID: subnetID, + IPAddress: "192.168.0.50", + }, {IPAddress: "192.168.1.50"}}, + TenantID: tenantID, + ProjectID: projectID, + SecurityGroups: &portSecurityGroups, + AllowedAddressPairs: []infrav1.AddressPair{{ + IPAddress: "10.10.10.10", + MACAddress: "f1:f1:f1:f1:f1:f1", + }}, + HostID: hostID, + VNICType: "direct", + Profile: map[string]string{"interface_name": "eno1"}, + DisablePortSecurity: pointerToFalse, + }, + }, + nil, + nil, + func(m *mock_networking.MockNetworkClientMockRecorder) { + portCreateOpts := ports.CreateOpts{ + NetworkID: netID, + Name: "foo-port-bar", + Description: "this is a test port", + AdminStateUp: pointerToTrue, + MACAddress: "fe:fe:fe:fe:fe:fe", + FixedIPs: []ports.IP{ + { + SubnetID: subnetID, + IPAddress: "192.168.0.50", + }, { + IPAddress: "192.168.1.50", + }, + }, + TenantID: tenantID, + ProjectID: projectID, + SecurityGroups: &portSecurityGroups, + AllowedAddressPairs: []ports.AddressPair{{ + IPAddress: "10.10.10.10", + MACAddress: "f1:f1:f1:f1:f1:f1", + }}, + } + portsecurityCreateOptsExt := portsecurity.PortCreateOptsExt{ + CreateOptsBuilder: portCreateOpts, + PortSecurityEnabled: pointerToTrue, + } + portbindingCreateOptsExt := portsbinding.CreateOptsExt{ + // Note for the test matching, the order in which the builders are composed + // must be the same as in the function we are testing. + CreateOptsBuilder: portsecurityCreateOptsExt, + HostID: hostID, + VNICType: "direct", + Profile: map[string]interface{}{"interface_name": "eno1"}, + } + m. + ListPort(ports.ListOpts{ + Name: "foo-port-bar", + NetworkID: netID, + }).Return([]ports.Port{}, nil) + m. + CreatePort(portbindingCreateOptsExt). + Return(&ports.Port{ + ID: portID1, + }, nil) + }, + &ports.Port{ + ID: portID1, + }, + false, + }, + { + "overrides default (instance) security groups if port security groups are specified", + "foo-port-1", + infrav1.Network{ + ID: netID, + PortOpts: &infrav1.PortOpts{ + SecurityGroups: &portSecurityGroups, + }, + }, + &instanceSecurityGroups, + []string{}, + func(m *mock_networking.MockNetworkClientMockRecorder) { + // No ports found + m. + ListPort(ports.ListOpts{ + Name: "foo-port-1", + NetworkID: netID, + }).Return([]ports.Port{}, nil) + m. + CreatePort(portsbinding.CreateOptsExt{ + CreateOptsBuilder: ports.CreateOpts{ + Name: "foo-port-1", + Description: "Created by cluster-api-provider-openstack cluster test-cluster", + SecurityGroups: &portSecurityGroups, + NetworkID: netID, + AllowedAddressPairs: []ports.AddressPair{}, + }, + }, + ).Return(&ports.Port{ID: portID1}, nil) + }, + &ports.Port{ID: portID1}, + false, + }, + { + "creates port with tags passed to function", + "foo-port-1", + infrav1.Network{ + ID: netID, + PortOpts: &infrav1.PortOpts{}, + }, + nil, + []string{"my-tag"}, + func(m *mock_networking.MockNetworkClientMockRecorder) { + // No ports found + m. + ListPort(ports.ListOpts{ + Name: "foo-port-1", + NetworkID: netID, + }).Return([]ports.Port{}, nil) + m.CreatePort(portsbinding.CreateOptsExt{ + CreateOptsBuilder: ports.CreateOpts{ + Name: "foo-port-1", + Description: "Created by cluster-api-provider-openstack cluster test-cluster", + NetworkID: netID, + AllowedAddressPairs: []ports.AddressPair{}, + }, + }).Return(&ports.Port{ID: portID1}, nil) + m.ReplaceAllAttributesTags("ports", portID1, attributestags.ReplaceAllOpts{Tags: []string{"my-tag"}}).Return([]string{"my-tag"}, nil) + }, + &ports.Port{ID: portID1}, + false, + }, + { + "creates port and trunk (with tags) if they aren't found", + "foo-port-1", + infrav1.Network{ + ID: netID, + PortOpts: &infrav1.PortOpts{ + Trunk: pointerToTrue, + }, + }, + nil, + []string{"my-tag"}, + func(m *mock_networking.MockNetworkClientMockRecorder) { + // No ports found + m. + ListPort(ports.ListOpts{ + Name: "foo-port-1", + NetworkID: netID, + }).Return([]ports.Port{}, nil) + m. + CreatePort(portsbinding.CreateOptsExt{ + CreateOptsBuilder: ports.CreateOpts{ + Name: "foo-port-1", + Description: "Created by cluster-api-provider-openstack cluster test-cluster", + NetworkID: netID, + AllowedAddressPairs: []ports.AddressPair{}, + }, + }).Return(&ports.Port{Name: "foo-port-1", ID: portID1}, nil) + m. + ListTrunk(trunks.ListOpts{ + Name: "foo-port-1", + PortID: portID1, + }).Return([]trunks.Trunk{}, nil) + m. + CreateTrunk(trunks.CreateOpts{ + Name: "foo-port-1", + PortID: portID1, + Description: "Created by cluster-api-provider-openstack cluster test-cluster", + }).Return(&trunks.Trunk{ID: trunkID}, nil) + + m.ReplaceAllAttributesTags("ports", portID1, attributestags.ReplaceAllOpts{Tags: []string{"my-tag"}}).Return([]string{"my-tag"}, nil) + m.ReplaceAllAttributesTags("trunks", trunkID, attributestags.ReplaceAllOpts{Tags: []string{"my-tag"}}).Return([]string{"my-tag"}, nil) + }, + &ports.Port{Name: "foo-port-1", ID: portID1}, + false, + }, + } + + eventObject := &infrav1.OpenStackMachine{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + mockClient := mock_networking.NewMockNetworkClient(mockCtrl) + tt.expect(mockClient.EXPECT()) + s := Service{ + client: mockClient, + logger: logr.DiscardLogger{}, + } + got, err := s.GetOrCreatePort( + eventObject, + "test-cluster", + tt.portName, + tt.net, + tt.instanceSecurityGroups, + tt.tags, + ) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func pointerTo(b bool) *bool { + return &b +}