diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index 960feaee2e..b10504f126 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -27,6 +27,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/networking/v2/subnets" "github.com/gophercloud/utils/openstack/clientconfig" "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" @@ -206,16 +207,14 @@ func deleteBastion(log logr.Logger, osProviderClient *gophercloud.ProviderClient if err != nil { return err } + addresses := instanceNS.Addresses() - floatingIP := instanceNS.FloatingIP() - if floatingIP != "" { - if err = networkingService.DisassociateFloatingIP(openStackCluster, floatingIP); err != nil { - handleUpdateOSCError(openStackCluster, errors.Errorf("failed to disassociate floating IP: %v", err)) - return errors.Errorf("failed to disassociate floating IP: %v", err) - } - if err = networkingService.DeleteFloatingIP(openStackCluster, floatingIP); err != nil { - handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete floating IP: %v", err)) - return errors.Errorf("failed to delete floating IP: %v", err) + for _, address := range addresses { + if address.Type == corev1.NodeExternalIP { + if err = networkingService.DeleteFloatingIP(openStackCluster, address.Address); err != nil { + handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete floating IP: %v", err)) + return errors.Errorf("failed to delete floating IP: %v", err) + } } } @@ -319,7 +318,7 @@ func reconcileBastion(log logr.Logger, osProviderClient *gophercloud.ProviderCli return err } if instanceStatus != nil { - bastion, err := instanceStatus.APIInstance() + bastion, err := instanceStatus.APIInstance(openStackCluster) if err != nil { return err } @@ -343,7 +342,7 @@ func reconcileBastion(log logr.Logger, osProviderClient *gophercloud.ProviderCli handleUpdateOSCError(openStackCluster, errors.Errorf("failed to get or create floating IP for bastion: %v", err)) return errors.Errorf("failed to get or create floating IP for bastion: %v", err) } - port, err := computeService.GetManagementPort(instanceStatus) + port, err := computeService.GetManagementPort(openStackCluster, instanceStatus) if err != nil { err = errors.Errorf("getting management port for bastion: %v", err) handleUpdateOSCError(openStackCluster, err) @@ -355,7 +354,7 @@ func reconcileBastion(log logr.Logger, osProviderClient *gophercloud.ProviderCli return errors.Errorf("failed to associate floating IP with bastion: %v", err) } - bastion, err := instanceStatus.APIInstance() + bastion, err := instanceStatus.APIInstance(openStackCluster) if err != nil { return err } diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index 413fb947d7..25173d11da 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -224,28 +224,27 @@ func (r *OpenStackMachineReconciler) reconcileDelete(ctx context.Context, logger if instanceStatus == nil { logger.Info("Skipped deleting machine that is already deleted") - controllerutil.RemoveFinalizer(openStackMachine, infrav1.MachineFinalizer) - if err := patchHelper.Patch(ctx, openStackMachine); err != nil { - return ctrl.Result{}, err - } - return ctrl.Result{}, nil - } - - instanceNS, err := instanceStatus.NetworkStatus() - if err != nil { - handleUpdateMachineError(logger, openStackMachine, errors.Errorf("error getting network status for OpenStack instance %s with ID %s: %v", instanceStatus.Name(), instanceStatus.ID(), err)) - return ctrl.Result{}, nil - } + } else { + if !openStackCluster.Spec.ManagedAPIServerLoadBalancer && util.IsControlPlaneMachine(machine) && openStackCluster.Spec.APIServerFloatingIP == "" { + instanceNS, err := instanceStatus.NetworkStatus() + if err != nil { + handleUpdateMachineError(logger, openStackMachine, errors.Errorf("error getting network status for OpenStack instance %s with ID %s: %v", instanceStatus.Name(), instanceStatus.ID(), err)) + return ctrl.Result{}, nil + } - if err = computeService.DeleteInstance(openStackMachine, instanceStatus); err != nil { - handleUpdateMachineError(logger, openStackMachine, errors.Errorf("error deleting OpenStack instance %s with ID %s: %v", instanceStatus.Name(), instanceStatus.ID(), err)) - return ctrl.Result{}, nil - } + addresses := instanceNS.Addresses() + for _, address := range addresses { + if address.Type == corev1.NodeExternalIP { + if err = networkingService.DeleteFloatingIP(openStackCluster, address.Address); err != nil { + handleUpdateMachineError(logger, openStackMachine, errors.Errorf("error deleting Openstack floating IP: %v", err)) + return ctrl.Result{}, nil + } + } + } + } - floatingIP := instanceNS.FloatingIP() - if !openStackCluster.Spec.ManagedAPIServerLoadBalancer && util.IsControlPlaneMachine(machine) && openStackCluster.Spec.APIServerFloatingIP == "" && floatingIP != "" { - if err = networkingService.DeleteFloatingIP(openStackCluster, floatingIP); err != nil { - handleUpdateMachineError(logger, openStackMachine, errors.Errorf("error deleting Openstack floating IP: %v", err)) + if err := computeService.DeleteInstance(openStackMachine, instanceStatus); err != nil { + handleUpdateMachineError(logger, openStackMachine, errors.Errorf("error deleting OpenStack instance %s with ID %s: %v", instanceStatus.Name(), instanceStatus.ID(), err)) return ctrl.Result{}, nil } } @@ -331,10 +330,7 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, logger return ctrl.Result{}, nil } - addresses := []corev1.NodeAddress{{Type: corev1.NodeInternalIP, Address: instanceNS.IP()}} - if instanceNS.FloatingIP() != "" { - addresses = append(addresses, []corev1.NodeAddress{{Type: corev1.NodeExternalIP, Address: instanceNS.FloatingIP()}}...) - } + addresses := instanceNS.Addresses() openStackMachine.Status.Addresses = addresses // TODO(sbueringer) From CAPA: TODO(vincepri): Remove this annotation when clusterctl is no longer relevant. @@ -370,7 +366,7 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, logger handleUpdateMachineError(logger, openStackMachine, errors.Errorf("Floating IP cannot be got or created: %v", err)) return ctrl.Result{}, nil } - port, err := computeService.GetManagementPort(instanceStatus) + port, err := computeService.GetManagementPort(openStackCluster, instanceStatus) if err != nil { err = errors.Errorf("getting management port for control plane machine %s: %v", machine.Name, err) handleUpdateMachineError(logger, openStackMachine, err) @@ -413,7 +409,7 @@ func handleUpdateMachineError(logger logr.Logger, openstackMachine *infrav1.Open } func (r *OpenStackMachineReconciler) reconcileLoadBalancerMember(logger logr.Logger, osProviderClient *gophercloud.ProviderClient, clientOpts *clientconfig.ClientOpts, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine, instanceNS *compute.InstanceNetworkStatus, clusterName string) error { - ip := instanceNS.IP() + ip := instanceNS.IP(openStackCluster.Status.Network.Name) loadbalancerService, err := loadbalancer.NewService(osProviderClient, clientOpts, logger) if err != nil { return err diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index 2a34e88d48..d392e489d6 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -417,12 +417,12 @@ func (s *Service) getImageID(imageName string) (string, error) { // GetManagementPort returns the port which is used for management and external // traffic. Cluster floating IPs must be associated with this port. -func (s *Service) GetManagementPort(instanceStatus *InstanceStatus) (*ports.Port, error) { +func (s *Service) GetManagementPort(openStackCluster *infrav1.OpenStackCluster, instanceStatus *InstanceStatus) (*ports.Port, error) { ns, err := instanceStatus.NetworkStatus() if err != nil { return nil, err } - allPorts, err := s.networkingService.GetPortFromInstanceIP(instanceStatus.ID(), ns.IP()) + allPorts, err := s.networkingService.GetPortFromInstanceIP(instanceStatus.ID(), ns.IP(openStackCluster.Status.Network.Name)) if err != nil { return nil, fmt.Errorf("lookup management port for server %s: %w", instanceStatus.ID(), err) } @@ -551,7 +551,7 @@ func (s *Service) GetInstanceStatus(resourceID string) (instance *InstanceStatus return nil, fmt.Errorf("get server %q detail failed: %v", resourceID, err) } - return &InstanceStatus{server: &server}, nil + return &InstanceStatus{&server, s.logger}, nil } func (s *Service) GetInstanceStatusByName(eventObject runtime.Object, name string) (instance *InstanceStatus, err error) { @@ -584,7 +584,7 @@ func (s *Service) GetInstanceStatusByName(eventObject runtime.Object, name strin // Return the first returned server, if any for i := range serverList { - return &InstanceStatus{server: &serverList[i]}, nil + return &InstanceStatus{&serverList[i], s.logger}, nil } return nil, nil } diff --git a/pkg/cloud/services/compute/instance_types.go b/pkg/cloud/services/compute/instance_types.go index 130a2c9ec6..656fed9f07 100644 --- a/pkg/cloud/services/compute/instance_types.go +++ b/pkg/cloud/services/compute/instance_types.go @@ -19,9 +19,12 @@ package compute import ( "encoding/json" "fmt" + "sort" + "github.com/go-logr/logr" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/availabilityzones" "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" + corev1 "k8s.io/api/core/v1" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha4" ) @@ -63,10 +66,11 @@ type InstanceIdentifier struct { // InstanceStatus represents instance data which has been returned by OpenStack. type InstanceStatus struct { server *ServerExt + logger logr.Logger } -func NewInstanceStatusFromServer(server *ServerExt) *InstanceStatus { - return &InstanceStatus{server} +func NewInstanceStatusFromServer(server *ServerExt, logger logr.Logger) *InstanceStatus { + return &InstanceStatus{server, logger} } type networkInterface struct { @@ -79,7 +83,7 @@ type networkInterface struct { // as used by CAPO. Therefore it may use more context than just data which was // returned by OpenStack. type InstanceNetworkStatus struct { - addresses map[string][]networkInterface + addresses map[string][]corev1.NodeAddress } func (is *InstanceStatus) ID() string { @@ -103,7 +107,7 @@ func (is *InstanceStatus) AvailabilityZone() string { } // APIInstance returns an infrav1.Instance object for use by the API. -func (is *InstanceStatus) APIInstance() (*infrav1.Instance, error) { +func (is *InstanceStatus) APIInstance(openStackCluster *infrav1.OpenStackCluster) (*infrav1.Instance, error) { i := infrav1.Instance{ ID: is.ID(), Name: is.Name(), @@ -116,8 +120,9 @@ func (is *InstanceStatus) APIInstance() (*infrav1.Instance, error) { return nil, err } - i.IP = ns.IP() - i.FloatingIP = ns.FloatingIP() + clusterNetwork := openStackCluster.Status.Network.Name + i.IP = ns.IP(clusterNetwork) + i.FloatingIP = ns.FloatingIP(clusterNetwork) return &i, nil } @@ -132,51 +137,102 @@ func (is *InstanceStatus) InstanceIdentifier() *InstanceIdentifier { // NetworkStatus returns an InstanceNetworkStatus object for an InstanceStatus. func (is *InstanceStatus) NetworkStatus() (*InstanceNetworkStatus, error) { - addresses := make(map[string][]networkInterface) - + // Gophercloud doesn't give us a struct for server addresses: we get a + // map of networkname -> interface{}. That interface{} is a list of + // addresses as in the example output here: + // https://docs.openstack.org/api-ref/compute/?expanded=show-server-details-detail#show-server-details + // + // Here we convert the interface{} into something more usable by + // marshalling it to json, then unmarshalling it back into our own + // struct. + addressesByNetwork := make(map[string][]corev1.NodeAddress) for networkName, b := range is.server.Addresses { list, err := json.Marshal(b) if err != nil { return nil, fmt.Errorf("error marshalling addresses for instance %s: %w", is.ID(), err) } - var networkList []networkInterface - err = json.Unmarshal(list, &networkList) + var interfaceList []networkInterface + err = json.Unmarshal(list, &interfaceList) if err != nil { return nil, fmt.Errorf("error unmarshalling addresses for instance %s: %w", is.ID(), err) } - addresses[networkName] = networkList - } + var addresses []corev1.NodeAddress + for i := range interfaceList { + address := &interfaceList[i] - return &InstanceNetworkStatus{addresses}, nil -} + // Only consider IPv4 + if address.Version != 4 { + is.logger.V(6).Info("Ignoring IPv%d address %s: only IPv4 is supported", address.Version, address.Address) + continue + } -func (ns *InstanceNetworkStatus) IP() string { - // Return the last listed non-floating IPv4 from the last listed network - // This behaviour is wrong, but consistent with the previous behaviour - // https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/4debc1fc4742e483302b0c36b16c076977bd165d/pkg/cloud/services/compute/instance.go#L973-L998 - // XXX: Fix this - for _, vifs := range ns.addresses { - for _, vif := range vifs { - if vif.Version == 4.0 && vif.Type != "floating" { - return vif.Address + var addressType corev1.NodeAddressType + switch address.Type { + case "floating": + addressType = corev1.NodeExternalIP + case "fixed": + addressType = corev1.NodeInternalIP + default: + is.logger.V(6).Info("Ignoring address %s with unknown type '%s'", address.Address, address.Type) + continue } + + addresses = append(addresses, corev1.NodeAddress{ + Type: addressType, + Address: address.Address, + }) } + + addressesByNetwork[networkName] = addresses } - return "" + + return &InstanceNetworkStatus{addressesByNetwork}, nil +} + +// Addresses returns a list of NodeAddresses containing all addresses which will +// be reported on the OpenStackMachine object. +func (ns *InstanceNetworkStatus) Addresses() []corev1.NodeAddress { + // We want the returned order of addresses to be deterministic to make + // it easy to detect changes and avoid unnecessary updates. Iteration + // over maps is non-deterministic, so we explicitly iterate over the + // address map in lexical order of network names. This order is + // arbitrary. + // Pull out addresses map keys (network names) and sort them lexically + networks := make([]string, 0, len(ns.addresses)) + for network := range ns.addresses { + networks = append(networks, network) + } + sort.Strings(networks) + + var addresses []corev1.NodeAddress + for _, network := range networks { + addressList := ns.addresses[network] + addresses = append(addresses, addressList...) + } + + return addresses } -func (ns *InstanceNetworkStatus) FloatingIP() string { - // Return the last listed floating IPv4 from the last listed network - // This behaviour is wrong, but consistent with the previous behaviour - // https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/4debc1fc4742e483302b0c36b16c076977bd165d/pkg/cloud/services/compute/instance.go#L973-L998 - // XXX: Fix this - for _, vifs := range ns.addresses { - for _, vif := range vifs { - if vif.Version == 4.0 && vif.Type == "floating" { - return vif.Address +func (ns *InstanceNetworkStatus) firstAddressByNetworkAndType(networkName string, addressType corev1.NodeAddressType) string { + if addressList, ok := ns.addresses[networkName]; ok { + for i := range addressList { + address := &addressList[i] + if address.Type == addressType { + return address.Address } } } return "" } + +// IP returns the first listed ip of an instance for the given network name. +func (ns *InstanceNetworkStatus) IP(networkName string) string { + return ns.firstAddressByNetworkAndType(networkName, corev1.NodeInternalIP) +} + +// FloatingIP returns the first listed floating ip of an instance for the given +// network name. +func (ns *InstanceNetworkStatus) FloatingIP(networkName string) string { + return ns.firstAddressByNetworkAndType(networkName, corev1.NodeExternalIP) +} diff --git a/pkg/cloud/services/compute/instance_types_test.go b/pkg/cloud/services/compute/instance_types_test.go new file mode 100644 index 0000000000..de161643c9 --- /dev/null +++ b/pkg/cloud/services/compute/instance_types_test.go @@ -0,0 +1,388 @@ +/* +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 compute + +import ( + "testing" + + "github.com/go-logr/logr" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" +) + +// Some arbitrary MAC addresses. +const ( + macAddr1 = "d1:b2:18:25:70:ab" + macAddr2 = "d1:b2:18:25:70:ac" + macAddr3 = "d1:b2:18:25:70:ad" + macAddr4 = "d1:b2:18:25:70:ae" +) + +// An address structure as generated by OpenStack. e.g. +// https://docs.openstack.org/api-ref/compute/?expanded=show-server-details-detail#show-server-details +type networkAddress struct { + Version int `json:"version"` + Addr string `json:"addr"` + Type string `json:"OS-EXT-IPS:type"` + MacAddr string `json:"OS-EXT-IPS:mac_addr"` +} + +func serverWithAddresses(addresses map[string][]networkAddress) *ServerExt { + var server ServerExt + + server.Addresses = make(map[string]interface{}) + for network, addressList := range addresses { + server.Addresses[network] = addressList + } + + return &server +} + +func TestNetworkStatus_Addresses(t *testing.T) { + tests := []struct { + name string + addresses map[string][]networkAddress + want []corev1.NodeAddress + }{ + { + name: "Single network single address", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr1, + }, + }, + }, + want: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "192.168.0.1", + }, + }, + }, + { + name: "Fixed and floating addresses", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr1, + }, { + Version: 4, + Addr: "10.0.0.1", + Type: "floating", + MacAddr: macAddr2, + }, + }, + }, + want: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "192.168.0.1", + }, { + Type: corev1.NodeExternalIP, + Address: "10.0.0.1", + }, + }, + }, + { + name: "Ignore IPv6", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 6, + Addr: "fe80::f816:3eff:fe56:3174", + Type: "fixed", + MacAddr: macAddr1, + }, { + Version: 6, + Addr: "fe80::f816:3eff:fe56:3175", + Type: "floating", + MacAddr: macAddr2, + }, { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr3, + }, + }, + }, + want: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "192.168.0.1", + }, + }, + }, + { + name: "Multiple networks", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr1, + }, { + Version: 4, + Addr: "10.0.0.1", + Type: "floating", + MacAddr: macAddr2, + }, + }, + "extraNet1": { + { + Version: 4, + Addr: "192.168.1.1", + Type: "fixed", + MacAddr: macAddr3, + }, + }, + "extraNet2": { + { + Version: 4, + Addr: "192.168.2.1", + Type: "fixed", + MacAddr: macAddr4, + }, + }, + }, + want: []corev1.NodeAddress{ + { + Type: corev1.NodeInternalIP, + Address: "192.168.1.1", + }, { + Type: corev1.NodeInternalIP, + Address: "192.168.2.1", + }, { + Type: corev1.NodeInternalIP, + Address: "192.168.0.1", + }, { + Type: corev1.NodeExternalIP, + Address: "10.0.0.1", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + is := &InstanceStatus{ + server: serverWithAddresses(tt.addresses), + logger: logr.Discard(), + } + instanceNS, err := is.NetworkStatus() + g.Expect(err).NotTo(HaveOccurred()) + + got := instanceNS.Addresses() + g.Expect(got).To(Equal(tt.want)) + }) + } +} + +func TestInstanceNetworkStatus(t *testing.T) { + tests := []struct { + name string + addresses map[string][]networkAddress + networkName string + wantIP string + wantFloatingIP string + }{ + { + name: "Single network single address", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr1, + }, + }, + }, + networkName: "primary", + wantIP: "192.168.0.1", + wantFloatingIP: "", + }, + { + name: "Fixed and floating addresses", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr1, + }, { + Version: 4, + Addr: "10.0.0.1", + Type: "floating", + MacAddr: macAddr2, + }, + }, + }, + networkName: "primary", + wantIP: "192.168.0.1", + wantFloatingIP: "10.0.0.1", + }, + { + name: "Ignore IPv6", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 6, + Addr: "fe80::f816:3eff:fe56:3174", + Type: "fixed", + MacAddr: macAddr1, + }, { + Version: 6, + Addr: "fe80::f816:3eff:fe56:3175", + Type: "floating", + MacAddr: macAddr2, + }, { + Version: 4, + Addr: "10.0.0.1", + Type: "floating", + MacAddr: macAddr3, + }, { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr4, + }, + }, + }, + networkName: "primary", + wantIP: "192.168.0.1", + wantFloatingIP: "10.0.0.1", + }, + { + name: "Multiple networks", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr1, + }, { + Version: 4, + Addr: "10.0.0.1", + Type: "floating", + MacAddr: macAddr2, + }, + }, + "extraNet1": { + { + Version: 4, + Addr: "192.168.1.1", + Type: "fixed", + MacAddr: macAddr3, + }, + }, + "extraNet2": { + { + Version: 4, + Addr: "192.168.2.1", + Type: "fixed", + MacAddr: macAddr4, + }, + }, + }, + networkName: "primary", + wantIP: "192.168.0.1", + wantFloatingIP: "10.0.0.1", + }, + { + name: "First IP", + addresses: map[string][]networkAddress{ + "primary": { + { + Version: 4, + Addr: "192.168.0.1", + Type: "fixed", + MacAddr: macAddr1, + }, { + Version: 4, + Addr: "10.0.0.1", + Type: "floating", + MacAddr: macAddr2, + }, { + Version: 4, + Addr: "192.168.0.2", + Type: "fixed", + MacAddr: macAddr3, + }, { + Version: 4, + Addr: "10.0.0.2", + Type: "floating", + MacAddr: macAddr4, + }, + }, + }, + networkName: "primary", + wantIP: "192.168.0.1", + wantFloatingIP: "10.0.0.1", + }, + { + name: "Network not found", + addresses: map[string][]networkAddress{ + "extraNet1": { + { + Version: 4, + Addr: "192.168.1.1", + Type: "fixed", + MacAddr: macAddr1, + }, + { + Version: 4, + Addr: "10.0.1.1", + Type: "floating", + MacAddr: macAddr2, + }, + }, + }, + networkName: "primary", + wantIP: "", + wantFloatingIP: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + is := &InstanceStatus{ + server: serverWithAddresses(tt.addresses), + logger: logr.Discard(), + } + ns, err := is.NetworkStatus() + g.Expect(err).NotTo(HaveOccurred()) + + ip := ns.IP(tt.networkName) + g.Expect(ip).To(Equal(tt.wantIP)) + + floatingIP := ns.FloatingIP(tt.networkName) + g.Expect(floatingIP).To(Equal(tt.wantFloatingIP)) + }) + } +} diff --git a/test/e2e/data/infrastructure-openstack/cluster-template-multi-network.yaml b/test/e2e/data/infrastructure-openstack/cluster-template-multi-network.yaml new file mode 100644 index 0000000000..c807931d33 --- /dev/null +++ b/test/e2e/data/infrastructure-openstack/cluster-template-multi-network.yaml @@ -0,0 +1,223 @@ +--- +apiVersion: cluster.x-k8s.io/v1alpha4 +kind: Cluster +metadata: + name: ${CLUSTER_NAME} + labels: + cni: "${CLUSTER_NAME}-crs-0" +spec: + clusterNetwork: + pods: + cidrBlocks: ["192.168.0.0/16"] # CIDR block used by Calico. + serviceDomain: "cluster.local" + infrastructureRef: + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4 + kind: OpenStackCluster + name: ${CLUSTER_NAME} + controlPlaneRef: + kind: KubeadmControlPlane + apiVersion: controlplane.cluster.x-k8s.io/v1alpha4 + name: ${CLUSTER_NAME}-control-plane +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4 +kind: OpenStackCluster +metadata: + name: ${CLUSTER_NAME} +spec: + cloudName: ${OPENSTACK_CLOUD} + identityRef: + name: ${CLUSTER_NAME}-cloud-config + kind: Secret + managedAPIServerLoadBalancer: true + managedSecurityGroups: true + nodeCidr: 10.6.0.0/24 + dnsNameservers: + - ${OPENSTACK_DNS_NAMESERVERS} + bastion: + enabled: true + instance: + flavor: ${OPENSTACK_BASTION_MACHINE_FLAVOR} + image: ${OPENSTACK_BASTION_IMAGE_NAME} + sshKeyName: ${OPENSTACK_SSH_KEY_NAME} + externalNetworkId: ${OPENSTACK_EXTERNAL_NETWORK_ID} +--- +kind: KubeadmControlPlane +apiVersion: controlplane.cluster.x-k8s.io/v1alpha4 +metadata: + name: "${CLUSTER_NAME}-control-plane" +spec: + replicas: ${CONTROL_PLANE_MACHINE_COUNT} + machineTemplate: + infrastructureRef: + kind: OpenStackMachineTemplate + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4 + name: "${CLUSTER_NAME}-control-plane" + kubeadmConfigSpec: + initConfiguration: + nodeRegistration: + name: '{{ local_hostname }}' + kubeletExtraArgs: + cloud-provider: openstack + cloud-config: /etc/kubernetes/cloud.conf + clusterConfiguration: + imageRepository: k8s.gcr.io + apiServer: + extraArgs: + cloud-provider: openstack + cloud-config: /etc/kubernetes/cloud.conf + extraVolumes: + - name: cloud + hostPath: /etc/kubernetes/cloud.conf + mountPath: /etc/kubernetes/cloud.conf + readOnly: true + controllerManager: + extraArgs: + cloud-provider: openstack + cloud-config: /etc/kubernetes/cloud.conf + extraVolumes: + - name: cloud + hostPath: /etc/kubernetes/cloud.conf + mountPath: /etc/kubernetes/cloud.conf + readOnly: true + - name: cacerts + hostPath: /etc/certs/cacert + mountPath: /etc/certs/cacert + readOnly: true + joinConfiguration: + nodeRegistration: + name: '{{ local_hostname }}' + kubeletExtraArgs: + cloud-config: /etc/kubernetes/cloud.conf + cloud-provider: openstack + files: + - path: /etc/kubernetes/cloud.conf + owner: root + permissions: "0600" + content: ${OPENSTACK_CLOUD_PROVIDER_CONF_B64} + encoding: base64 + - path: /etc/certs/cacert + owner: root + permissions: "0600" + content: ${OPENSTACK_CLOUD_CACERT_B64} + encoding: base64 + version: "${KUBERNETES_VERSION}" +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4 +kind: OpenStackMachineTemplate +metadata: + name: ${CLUSTER_NAME}-control-plane +spec: + template: + spec: + flavor: ${OPENSTACK_CONTROL_PLANE_MACHINE_FLAVOR} + image: ${OPENSTACK_IMAGE_NAME} + sshKeyName: ${OPENSTACK_SSH_KEY_NAME} + cloudName: ${OPENSTACK_CLOUD} + identityRef: + name: ${CLUSTER_NAME}-cloud-config + kind: Secret + ports: + - description: "primary" + - description: "Extra Network 1" + networkId: "${CLUSTER_EXTRA_NET_1}" + - description: "Extra Network 2" + networkId: "${CLUSTER_EXTRA_NET_2}" +--- +apiVersion: cluster.x-k8s.io/v1alpha4 +kind: MachineDeployment +metadata: + name: "${CLUSTER_NAME}-md-0" +spec: + clusterName: "${CLUSTER_NAME}" + replicas: ${WORKER_MACHINE_COUNT} + selector: + matchLabels: + template: + spec: + clusterName: "${CLUSTER_NAME}" + version: "${KUBERNETES_VERSION}" + failureDomain: ${OPENSTACK_FAILURE_DOMAIN} + bootstrap: + configRef: + name: "${CLUSTER_NAME}-md-0" + apiVersion: bootstrap.cluster.x-k8s.io/v1alpha4 + kind: KubeadmConfigTemplate + infrastructureRef: + name: "${CLUSTER_NAME}-md-0" + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4 + kind: OpenStackMachineTemplate +--- +apiVersion: infrastructure.cluster.x-k8s.io/v1alpha4 +kind: OpenStackMachineTemplate +metadata: + name: ${CLUSTER_NAME}-md-0 +spec: + template: + spec: + cloudName: ${OPENSTACK_CLOUD} + identityRef: + name: ${CLUSTER_NAME}-cloud-config + kind: Secret + flavor: ${OPENSTACK_NODE_MACHINE_FLAVOR} + image: ${OPENSTACK_IMAGE_NAME} + sshKeyName: ${OPENSTACK_SSH_KEY_NAME} + ports: + - description: "primary" + - description: "Extra Network 1" + networkId: "${CLUSTER_EXTRA_NET_1}" + - description: "Extra Network 2" + networkId: "${CLUSTER_EXTRA_NET_2}" +--- +apiVersion: bootstrap.cluster.x-k8s.io/v1alpha4 +kind: KubeadmConfigTemplate +metadata: + name: ${CLUSTER_NAME}-md-0 +spec: + template: + spec: + files: + - content: ${OPENSTACK_CLOUD_PROVIDER_CONF_B64} + encoding: base64 + owner: root + path: /etc/kubernetes/cloud.conf + permissions: "0600" + - content: ${OPENSTACK_CLOUD_CACERT_B64} + encoding: base64 + owner: root + path: /etc/certs/cacert + permissions: "0600" + joinConfiguration: + nodeRegistration: + name: '{{ local_hostname }}' + kubeletExtraArgs: + cloud-config: /etc/kubernetes/cloud.conf + cloud-provider: openstack +--- +apiVersion: v1 +kind: Secret +metadata: + name: ${CLUSTER_NAME}-cloud-config + labels: + clusterctl.cluster.x-k8s.io/move: "true" +data: + clouds.yaml: ${OPENSTACK_CLOUD_YAML_B64} + cacert: ${OPENSTACK_CLOUD_CACERT_B64} +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: "cni-${CLUSTER_NAME}-crs-0" +data: ${CNI_RESOURCES} +--- +apiVersion: addons.cluster.x-k8s.io/v1alpha4 +kind: ClusterResourceSet +metadata: + name: "${CLUSTER_NAME}-crs-0" +spec: + strategy: ApplyOnce + clusterSelector: + matchLabels: + cni: "${CLUSTER_NAME}-crs-0" + resources: + - name: "cni-${CLUSTER_NAME}-crs-0" + kind: ConfigMap diff --git a/test/e2e/shared/common.go b/test/e2e/shared/common.go index 1a7b48027f..eb381e2ec9 100644 --- a/test/e2e/shared/common.go +++ b/test/e2e/shared/common.go @@ -81,7 +81,7 @@ func DumpSpecResourcesAndCleanup(ctx context.Context, specName string, namespace } func dumpMachines(ctx context.Context, e2eCtx *E2EContext, namespace *corev1.Namespace) { - cluster, err := clusterForSpec(ctx, e2eCtx.Environment.BootstrapClusterProxy, namespace) + cluster, err := ClusterForSpec(ctx, e2eCtx, namespace) if err != nil { _, _ = fmt.Fprintf(GinkgoWriter, "cannot dump machines, couldn't get cluster in namespace %s: %v\n", namespace.Name, err) return @@ -95,7 +95,7 @@ func dumpMachines(ctx context.Context, e2eCtx *E2EContext, namespace *corev1.Nam _, _ = fmt.Fprintf(GinkgoWriter, "cannot dump machines, could not get machines: %v\n", err) return } - srvs, err := getOpenStackServers(e2eCtx) + srvs, err := getOpenStackServers(e2eCtx, cluster) if err != nil { _, _ = fmt.Fprintf(GinkgoWriter, "cannot dump machines, could not get servers from OpenStack: %v\n", err) return @@ -109,8 +109,8 @@ func dumpMachines(ctx context.Context, e2eCtx *E2EContext, namespace *corev1.Nam } } -func clusterForSpec(ctx context.Context, clusterProxy framework.ClusterProxy, namespace *corev1.Namespace) (*infrav1.OpenStackCluster, error) { - lister := clusterProxy.GetClient() +func ClusterForSpec(ctx context.Context, e2eCtx *E2EContext, namespace *corev1.Namespace) (*infrav1.OpenStackCluster, error) { + lister := e2eCtx.Environment.BootstrapClusterProxy.GetClient() list := new(infrav1.OpenStackClusterList) if err := lister.List(ctx, list, client.InNamespace(namespace.GetName())); err != nil { return nil, fmt.Errorf("error listing cluster: %v", err) diff --git a/test/e2e/shared/defaults.go b/test/e2e/shared/defaults.go index c706033c8e..a27f2e748c 100644 --- a/test/e2e/shared/defaults.go +++ b/test/e2e/shared/defaults.go @@ -43,6 +43,7 @@ const ( FlavorDefault = "ci-artifacts" FlavorWithoutLB = "without-lb-ci-artifacts" FlavorExternalCloudProvider = "external-cloud-provider-ci-artifacts" + FlavorMultiNetwork = "multi-network-ci-artifacts" ) // DefaultScheme returns the default scheme to use for testing. diff --git a/test/e2e/shared/openstack.go b/test/e2e/shared/openstack.go index 6469255bbc..4f90245700 100644 --- a/test/e2e/shared/openstack.go +++ b/test/e2e/shared/openstack.go @@ -30,18 +30,22 @@ import ( "path/filepath" "strings" + "github.com/go-logr/logr" "github.com/gophercloud/gophercloud" "github.com/gophercloud/gophercloud/openstack" "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/networks" "github.com/gophercloud/gophercloud/openstack/networking/v2/ports" + "github.com/gophercloud/gophercloud/openstack/networking/v2/subnets" "github.com/gophercloud/utils/openstack/clientconfig" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "gopkg.in/ini.v1" "sigs.k8s.io/yaml" + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha4" "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute" "sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/provider" ) @@ -125,7 +129,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) ([]ports.Port, error) { providerClient, clientOpts, err := getProviderClient(e2eCtx) if err != nil { _, _ = fmt.Fprintf(GinkgoWriter, "error creating provider client: %s\n", err) @@ -147,12 +151,12 @@ func DumpOpenStackPorts(e2eCtx *E2EContext, filter ports.ListOpts) (*[]ports.Por if err != nil { return nil, fmt.Errorf("error extracting ports: %s", err) } - return &portsList, nil + return portsList, nil } // getOpenStackServers gets all OpenStack servers at once, to save on DescribeInstances // calls. -func getOpenStackServers(e2eCtx *E2EContext) (map[string]server, error) { +func getOpenStackServers(e2eCtx *E2EContext, openStackCluster *infrav1.OpenStackCluster) (map[string]server, error) { providerClient, clientOpts, err := getProviderClient(e2eCtx) if err != nil { _, _ = fmt.Fprintf(GinkgoWriter, "error creating provider client: %s\n", err) @@ -179,13 +183,14 @@ func getOpenStackServers(e2eCtx *E2EContext) (map[string]server, error) { srvs := map[string]server{} for i := range serverList { srv := &serverList[i] - instanceStatus := compute.NewInstanceStatusFromServer(srv) + instanceStatus := compute.NewInstanceStatusFromServer(srv, logr.Discard()) instanceNS, err := instanceStatus.NetworkStatus() if err != nil { return nil, fmt.Errorf("error getting network status for server %s: %v", srv.Name, err) } - if instanceNS.IP() == "" { + ip := instanceNS.IP(openStackCluster.Status.Network.Name) + if ip == "" { _, _ = fmt.Fprintf(GinkgoWriter, "error getting internal ip for server %s: internal ip doesn't exist (yet)\n", srv.Name) continue } @@ -193,7 +198,7 @@ func getOpenStackServers(e2eCtx *E2EContext) (map[string]server, error) { srvs[srv.Name] = server{ name: srv.Name, id: srv.ID, - ip: instanceNS.IP(), + ip: ip, } } return srvs, nil @@ -320,3 +325,57 @@ func getOpenStackCloudYAML(cloudYAML string) []byte { Expect(err).NotTo(HaveOccurred()) return cloudYAMLContent } + +func CreateOpenStackNetwork(e2eCtx *E2EContext, name, cidr string) (*networks.Network, 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) + } + + netCreateOpts := networks.CreateOpts{ + Name: name, + AdminStateUp: gophercloud.Enabled, + } + net, err := networks.Create(networkClient, netCreateOpts).Extract() + if err != nil { + return net, err + } + + subnetCreateOpts := subnets.CreateOpts{ + Name: name, + NetworkID: net.ID, + IPVersion: 4, + CIDR: cidr, + } + _, err = subnets.Create(networkClient, subnetCreateOpts).Extract() + if err != nil { + networks.Delete(networkClient, net.ID) + return nil, err + } + return net, nil +} + +func DeleteOpenStackNetwork(e2eCtx *E2EContext, id string) error { + providerClient, clientOpts, err := getProviderClient(e2eCtx) + if err != nil { + _, _ = fmt.Fprintf(GinkgoWriter, "error creating provider client: %s\n", err) + return err + } + + networkClient, err := openstack.NewNetworkV2(providerClient, gophercloud.EndpointOpts{ + Region: clientOpts.RegionName, + }) + if err != nil { + return fmt.Errorf("error creating network client: %s", err) + } + + return networks.Delete(networkClient, id).ExtractErr() +} diff --git a/test/e2e/suites/e2e/e2e_test.go b/test/e2e/suites/e2e/e2e_test.go index f75abbfbf6..7749cc929c 100644 --- a/test/e2e/suites/e2e/e2e_test.go +++ b/test/e2e/suites/e2e/e2e_test.go @@ -23,10 +23,12 @@ import ( "context" "errors" "fmt" + "os" "path/filepath" "strings" "time" + "github.com/gophercloud/gophercloud/openstack/networking/v2/networks" "github.com/gophercloud/gophercloud/openstack/networking/v2/ports" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -46,11 +48,15 @@ import ( "sigs.k8s.io/cluster-api-provider-openstack/test/e2e/shared" ) +const specName = "e2e" + var _ = Describe("e2e tests", func() { var ( namespace *corev1.Namespace ctx context.Context - specName = "e2e" + + // Cleanup functions which cannot run until after the cluster has been deleted + postClusterCleanup []func() ) BeforeEach(func() { @@ -61,6 +67,7 @@ var _ = Describe("e2e tests", func() { Expect(e2eCtx.E2EConfig).ToNot(BeNil(), "Invalid argument. e2eConfig can't be nil when calling %s spec", specName) Expect(e2eCtx.E2EConfig.Variables).To(HaveKey(shared.KubernetesVersion)) shared.SetEnvVar("USE_CI_ARTIFACTS", "true", false) + postClusterCleanup = nil }) Describe("Workload cluster (default)", func() { @@ -71,7 +78,7 @@ var _ = Describe("e2e tests", func() { configCluster.ControlPlaneMachineCount = pointer.Int64Ptr(3) configCluster.WorkerMachineCount = pointer.Int64Ptr(1) configCluster.Flavor = shared.FlavorDefault - md := createCluster(ctx, configCluster, specName) + md := createCluster(ctx, configCluster) workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{ Lister: e2eCtx.Environment.BootstrapClusterProxy.GetClient(), @@ -97,7 +104,7 @@ var _ = Describe("e2e tests", func() { configCluster.ControlPlaneMachineCount = pointer.Int64Ptr(1) configCluster.WorkerMachineCount = pointer.Int64Ptr(1) configCluster.Flavor = shared.FlavorExternalCloudProvider - md := createCluster(ctx, configCluster, specName) + md := createCluster(ctx, configCluster) workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{ Lister: e2eCtx.Environment.BootstrapClusterProxy.GetClient(), @@ -138,7 +145,7 @@ var _ = Describe("e2e tests", func() { configCluster.ControlPlaneMachineCount = pointer.Int64Ptr(1) configCluster.WorkerMachineCount = pointer.Int64Ptr(1) configCluster.Flavor = shared.FlavorWithoutLB - _ = createCluster(ctx, configCluster, specName) + _ = createCluster(ctx, configCluster) shared.Byf("Creating MachineDeployment with custom port options") md3Name := clusterName + "-md-3" @@ -156,15 +163,15 @@ var _ = Describe("e2e tests", func() { }) shared.Byf("Waiting for custom port to be created") - var plist *[]ports.Port + var plist []ports.Port var err error Eventually(func() int { plist, err = shared.DumpOpenStackPorts(e2eCtx, ports.ListOpts{Description: "primary"}) Expect(err).To(BeNil()) - return len(*plist) + return len(plist) }, e2eCtx.E2EConfig.GetIntervals(specName, "wait-worker-nodes")...).Should(Equal(1)) - port := (*plist)[0] + port := plist[0] Expect(port.Description).To(Equal("primary")) }) It("It should be creatable and deletable", func() { @@ -174,7 +181,7 @@ var _ = Describe("e2e tests", func() { configCluster.ControlPlaneMachineCount = pointer.Int64Ptr(1) configCluster.WorkerMachineCount = pointer.Int64Ptr(1) configCluster.Flavor = shared.FlavorWithoutLB - md := createCluster(ctx, configCluster, specName) + md := createCluster(ctx, configCluster) workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{ Lister: e2eCtx.Environment.BootstrapClusterProxy.GetClient(), @@ -192,6 +199,117 @@ var _ = Describe("e2e tests", func() { }) }) + Describe("Workload cluster (multiple attached networks)", func() { + var extraNet1, extraNet2 *networks.Network + + BeforeEach(func() { + var err error + + // Create 2 additional networks to be attached to all cluster nodes + // We can't clean up these networks in a corresponding AfterEach because they will still be in use by the cluster. + // Instead we clean them up after the cluster has been deleted. + + shared.Byf("Creating additional networks") + + extraNet1, err = shared.CreateOpenStackNetwork(e2eCtx, fmt.Sprintf("%s-extraNet1", namespace.Name), "10.14.0.0/24") + Expect(err).NotTo(HaveOccurred()) + postClusterCleanup = append(postClusterCleanup, func() { + shared.Byf("Deleting additional network %s", extraNet1.Name) + err := shared.DeleteOpenStackNetwork(e2eCtx, extraNet1.ID) + Expect(err).NotTo(HaveOccurred()) + }) + + extraNet2, err = shared.CreateOpenStackNetwork(e2eCtx, fmt.Sprintf("%s-extraNet2", namespace.Name), "10.14.1.0/24") + Expect(err).NotTo(HaveOccurred()) + postClusterCleanup = append(postClusterCleanup, func() { + shared.Byf("Deleting additional network %s", extraNet2.Name) + err := shared.DeleteOpenStackNetwork(e2eCtx, extraNet2.ID) + Expect(err).NotTo(HaveOccurred()) + }) + + os.Setenv("CLUSTER_EXTRA_NET_1", extraNet1.ID) + os.Setenv("CLUSTER_EXTRA_NET_2", extraNet2.ID) + }) + + It("should attach all machines to multiple networks", func() { + shared.Byf("Creating a cluster") + clusterName := fmt.Sprintf("cluster-%s", namespace.Name) + configCluster := defaultConfigCluster(clusterName, namespace.Name) + configCluster.ControlPlaneMachineCount = pointer.Int64Ptr(1) + configCluster.WorkerMachineCount = pointer.Int64Ptr(1) + configCluster.Flavor = shared.FlavorMultiNetwork + md := createCluster(ctx, configCluster) + + workerMachines := framework.GetMachinesByMachineDeployments(ctx, framework.GetMachinesByMachineDeploymentsInput{ + Lister: e2eCtx.Environment.BootstrapClusterProxy.GetClient(), + ClusterName: clusterName, + Namespace: namespace.Name, + MachineDeployment: *md[0], + }) + controlPlaneMachines := framework.GetControlPlaneMachinesByCluster(ctx, framework.GetControlPlaneMachinesByClusterInput{ + Lister: e2eCtx.Environment.BootstrapClusterProxy.GetClient(), + ClusterName: clusterName, + Namespace: namespace.Name, + }) + Expect(workerMachines).To(HaveLen(int(*configCluster.WorkerMachineCount))) + Expect(controlPlaneMachines).To(HaveLen(int(*configCluster.ControlPlaneMachineCount))) + + openStackCluster, err := shared.ClusterForSpec(ctx, e2eCtx, namespace) + Expect(err).NotTo(HaveOccurred()) + + var allMachines []clusterv1.Machine + allMachines = append(allMachines, controlPlaneMachines...) + allMachines = append(allMachines, workerMachines...) + + // We expect each machine to have 3 ports in each of these 3 networks with the given description. + expectedPorts := map[string]string{ + openStackCluster.Status.Network.ID: "primary", + extraNet1.ID: "Extra Network 1", + extraNet2.ID: "Extra Network 2", + } + + for i := range allMachines { + machine := &allMachines[i] + shared.Byf("Checking ports for machine %s", machine.Name) + instanceID := getInstanceIDForMachine(machine) + + shared.Byf("Fetching ports for instance %s", instanceID) + ports, err := shared.DumpOpenStackPorts(e2eCtx, ports.ListOpts{ + DeviceID: instanceID, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(ports).To(HaveLen(len(expectedPorts))) + + var seen []string + var seenAddresses clusterv1.MachineAddresses + for j := range ports { + port := &ports[j] + + // Check that the port has an expected network ID + expectedDescription, ok := expectedPorts[port.NetworkID] + Expect(ok).To(BeTrue()) + + // Check that the port has the expected description for that network ID + Expect(port.Description).To(Equal(expectedDescription)) + + // Check that we don't have duplicate networks + Expect(seen).ToNot(ContainElement(port.NetworkID)) + seen = append(seen, port.NetworkID) + + for k := range port.FixedIPs { + seenAddresses = append(seenAddresses, clusterv1.MachineAddress{ + Type: clusterv1.MachineInternalIP, + Address: port.FixedIPs[k].IPAddress, + }) + } + } + + // All IP addresses on all ports should be reported in Addresses + Expect(machine.Status.Addresses).To(ContainElements(seenAddresses)) + } + }) + }) + Describe("MachineDeployment misconfigurations", func() { It("Should fail to create MachineDeployment with invalid subnet or invalid availability zone", func() { shared.Byf("Creating a cluster") @@ -200,7 +318,7 @@ var _ = Describe("e2e tests", func() { configCluster.ControlPlaneMachineCount = pointer.Int64Ptr(1) configCluster.WorkerMachineCount = pointer.Int64Ptr(0) configCluster.Flavor = shared.FlavorDefault - _ = createCluster(ctx, configCluster, specName) + _ = createCluster(ctx, configCluster) shared.Byf("Creating Machine Deployment with invalid subnet id") md1Name := clusterName + "-md-1" @@ -240,10 +358,15 @@ var _ = Describe("e2e tests", func() { shared.SetEnvVar("USE_CI_ARTIFACTS", "false", false) // Dumps all the resources in the spec namespace, then cleanups the cluster object and the spec namespace itself. shared.DumpSpecResourcesAndCleanup(ctx, specName, namespace, e2eCtx) + + // Cleanup resources which can't be cleaned up until the cluster has been deleted + for _, cleanup := range postClusterCleanup { + cleanup() + } }) }) -func createCluster(ctx context.Context, configCluster clusterctl.ConfigClusterInput, specName string) []*clusterv1.MachineDeployment { +func createCluster(ctx context.Context, configCluster clusterctl.ConfigClusterInput) []*clusterv1.MachineDeployment { result := &clusterctl.ApplyClusterTemplateAndWaitResult{} clusterctl.ApplyClusterTemplateAndWait(ctx, clusterctl.ApplyClusterTemplateAndWaitInput{ ClusterProxy: e2eCtx.Environment.BootstrapClusterProxy, @@ -277,6 +400,15 @@ func getEvents(namespace string) *corev1.EventList { return eventsList } +func getInstanceIDForMachine(machine *clusterv1.Machine) string { + providerID := machine.Spec.ProviderID + Expect(providerID).NotTo(BeNil()) + + providerIDSplit := strings.SplitN(*providerID, ":///", 2) + Expect(providerIDSplit[0]).To(Equal("openstack")) + return providerIDSplit[1] +} + func isErrorEventExists(namespace, machineDeploymentName, eventReason, errorMsg string, eList *corev1.EventList) bool { ctrlClient := e2eCtx.Environment.BootstrapClusterProxy.GetClient() machineDeployment := &clusterv1.MachineDeployment{}