Merge pull request openshift#7239 from shiftstack/osp_fd

OSASINFRA-3182: openstack: remove portTargets

Author: openshift-merge-robot

data/data/install.openshift.io_installconfigs.yaml

@@ -253,7 +253,7 @@ replace sigs.k8s.io/controller-tools => sigs.k8s.io/controller-tools v0.3.1-0.20
 
 // Override the OpenShift API version in hive
 
-replace github.com/openshift/api => github.com/openshift/api v0.0.0-20230531161518-2346bf94f19e
+replace github.com/openshift/api => github.com/openshift/api v0.0.0-20230607130528-611114dca681
 
 replace github.com/terraform-providers/terraform-provider-nutanix => github.com/nutanix/terraform-provider-nutanix v1.5.0
 

go.mod

@@ -1054,8 +1054,8 @@ github.com/opencontainers/runtime-spec v1.0.3-0.20200929063507-e6143ca7d51d/go.m
 github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/selinux v1.5.2/go.mod h1:yTcKuYAh6R95iDpefGLQaPaRwJFwyzAJufJyiTt7s0g=
 github.com/opencontainers/selinux v1.8.2/go.mod h1:MUIHuUEvKB1wtJjQdOyYRgOnLD2xAPP8dBsCoU0KuF8=
-github.com/openshift/api v0.0.0-20230531161518-2346bf94f19e h1:ELcgpoysjPXc3Gf4TrlP/OAJMgZeDUaQAFpfcnNdw2o=
-github.com/openshift/api v0.0.0-20230531161518-2346bf94f19e/go.mod h1:4VWG+W22wrB4HfBL88P40DxLEpSOaiBVxUnfalfJo9k=
+github.com/openshift/api v0.0.0-20230607130528-611114dca681 h1:kSvo4fjZyYRu7z7PVkZlqcYhoS4mZHVFYVUkG3WkvIE=
+github.com/openshift/api v0.0.0-20230607130528-611114dca681/go.mod h1:4VWG+W22wrB4HfBL88P40DxLEpSOaiBVxUnfalfJo9k=
 github.com/openshift/assisted-image-service v0.0.0-20220506122314-2f689a1084b8 h1:oZ3VAWiM8tPRBM+vYI4GBmlrqyoqizcgZ7pBy5EX2K8=
 github.com/openshift/assisted-image-service v0.0.0-20220506122314-2f689a1084b8/go.mod h1:bH4+AsmPy8mQQvtgedBm2Crs93TDWeXEMlIPrlEMpjA=
 github.com/openshift/assisted-service v0.0.0-20220928142635-a40422bdea61 h1:HtUYJBAdRgfVYide0bq3GsT/4n5uPWteA1rIZVXEL7k=

go.sum

@@ -11,6 +11,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 
+	machinev1 "github.com/openshift/api/machine/v1"
 	machinev1alpha1 "github.com/openshift/api/machine/v1alpha1"
 	machineapi "github.com/openshift/api/machine/v1beta1"
 	"github.com/openshift/installer/pkg/types"
@@ -57,14 +58,11 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine
 	}
 	machines := make([]machineapi.Machine, 0, total)
 	for idx := int64(0); idx < total; idx++ {
-		var failureDomain openstack.FailureDomain
-
-		if len(mpool.FailureDomains) > 0 {
-			failureDomain = mpool.FailureDomains[uint(idx)%uint(len(mpool.FailureDomains))]
-		} else {
-			// Zones have length at least one
-			failureDomain.ComputeAvailabilityZone = mpool.Zones[uint(idx)%uint(len(mpool.Zones))]
-			failureDomain.StorageAvailabilityZone = volumeAZs[uint(idx)%uint(len(volumeAZs))]
+		failureDomain := machinev1.OpenStackFailureDomain{
+			AvailabilityZone: mpool.Zones[uint(idx)%uint(len(mpool.Zones))],
+			RootVolume: &machinev1.RootVolume{
+				AvailabilityZone: volumeAZs[uint(idx)%uint(len(volumeAZs))],
+			},
 		}
 
 		var provider *machinev1alpha1.OpenstackProviderSpec
@@ -110,9 +108,9 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine
 	return machines, nil
 }
 
-func generateProvider(clusterID string, platform *openstack.Platform, mpool *openstack.MachinePool, osImage string, role, userDataSecret string, trunkSupport bool, failureDomain openstack.FailureDomain) (*machinev1alpha1.OpenstackProviderSpec, error) {
+func generateProvider(clusterID string, platform *openstack.Platform, mpool *openstack.MachinePool, osImage string, role, userDataSecret string, trunkSupport bool, failureDomain machinev1.OpenStackFailureDomain) (*machinev1alpha1.OpenstackProviderSpec, error) {
 	var controlPlaneNetwork machinev1alpha1.NetworkParam
-	additionalNetworks := make([]machinev1alpha1.NetworkParam, 0, len(failureDomain.PortTargets)+len(mpool.AdditionalNetworkIDs))
+	additionalNetworks := make([]machinev1alpha1.NetworkParam, 0, len(mpool.AdditionalNetworkIDs))
 	primarySubnet := platform.MachinesSubnet
 
 	if platform.MachinesSubnet != "" {
@@ -134,27 +132,6 @@ func generateProvider(clusterID string, platform *openstack.Platform, mpool *ope
 		}
 	}
 
-	for _, portTarget := range failureDomain.PortTargets {
-		networkParam := machinev1alpha1.NetworkParam{
-			UUID: portTarget.Network.ID,
-			Filter: machinev1alpha1.Filter{
-				Name: portTarget.Network.Name,
-			},
-		}
-		for i := range portTarget.FixedIPs {
-			networkParam.Subnets = append(networkParam.Subnets, machinev1alpha1.SubnetParam{Filter: portTarget.FixedIPs[i].Subnet})
-		}
-		if portTarget.ID == "control-plane" {
-			controlPlaneNetwork = networkParam
-			if role == "master" {
-				primarySubnet = ""
-			}
-		} else {
-			networkParam.NoAllowedAddressPairs = true
-			additionalNetworks = append(additionalNetworks, networkParam)
-		}
-	}
-
 	for _, networkID := range mpool.AdditionalNetworkIDs {
 		additionalNetworks = append(additionalNetworks, machinev1alpha1.NetworkParam{
 			UUID:                  networkID,
@@ -181,8 +158,8 @@ func generateProvider(clusterID string, platform *openstack.Platform, mpool *ope
 	// For the workers, we still use the AZ name as part of the server group name
 	// so the user can control the scheduling policy per AZ and change the MachineSets
 	// if needed on a day 2 operation.
-	if role == "worker" && failureDomain.ComputeAvailabilityZone != "" {
-		serverGroupName += "-" + failureDomain.ComputeAvailabilityZone
+	if role == "worker" && failureDomain.AvailabilityZone != "" {
+		serverGroupName += "-" + failureDomain.AvailabilityZone
 	}
 
 	spec := machinev1alpha1.OpenstackProviderSpec{
@@ -196,7 +173,7 @@ func generateProvider(clusterID string, platform *openstack.Platform, mpool *ope
 		UserDataSecret:   &corev1.SecretReference{Name: userDataSecret},
 		Networks:         append([]machinev1alpha1.NetworkParam{controlPlaneNetwork}, additionalNetworks...),
 		PrimarySubnet:    primarySubnet,
-		AvailabilityZone: failureDomain.ComputeAvailabilityZone,
+		AvailabilityZone: failureDomain.AvailabilityZone,
 		SecurityGroups:   securityGroups,
 		ServerGroupName:  serverGroupName,
 		Trunk:            trunkSupport,
@@ -213,7 +190,7 @@ func generateProvider(clusterID string, platform *openstack.Platform, mpool *ope
 			Size:       mpool.RootVolume.Size,
 			SourceUUID: osImage,
 			VolumeType: mpool.RootVolume.Type,
-			Zone:       failureDomain.StorageAvailabilityZone,
+			Zone:       failureDomain.RootVolume.AvailabilityZone,
 		}
 	} else {
 		spec.Image = osImage

pkg/asset/machines/openstack/machines.go

@@ -8,6 +8,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 
+	machinev1 "github.com/openshift/api/machine/v1"
 	clusterapi "github.com/openshift/api/machine/v1beta1"
 	"github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/types/openstack"
@@ -55,9 +56,11 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach
 			role,
 			userDataSecret,
 			trunkSupport,
-			openstack.FailureDomain{
-				ComputeAvailabilityZone: az,
-				StorageAvailabilityZone: volumeAZs[idx%len(volumeAZs)],
+			machinev1.OpenStackFailureDomain{
+				AvailabilityZone: az,
+				RootVolume: &machinev1.RootVolume{
+					AvailabilityZone: volumeAZs[idx%len(volumeAZs)],
+				},
 			},
 		)
 		if err != nil {

pkg/asset/machines/openstack/machinesets.go

@@ -224,20 +224,6 @@ func TFVars(
 		// Terraform expects each master to get an array, empty or otherwise.
 		additionalPorts[i] = []terraformPort{}
 		machinesPorts[i] = defaultMachinesPort
-		if mastermpool != nil && len(mastermpool.FailureDomains) > 0 {
-			failureDomain := &mastermpool.FailureDomains[i%len(mastermpool.FailureDomains)]
-			for j := range failureDomain.PortTargets {
-				terraformPort, err := portTargetToTerraformPort(networkClient, failureDomain.PortTargets[j].PortTarget)
-				if err != nil {
-					return nil, fmt.Errorf("failed to resolve portTarget %q of master %d :%w", failureDomain.PortTargets[j].ID, i, err)
-				}
-				if failureDomain.PortTargets[j].ID == "control-plane" {
-					machinesPorts[i] = &terraformPort
-				} else {
-					additionalPorts[i] = append(additionalPorts[i], terraformPort)
-				}
-			}
-		}
 	}
 
 	var additionalSecurityGroupIDs []string

pkg/tfvars/openstack/openstack.go

@@ -1,17 +1,5 @@
 package openstack
 
-import (
-	"fmt"
-
-	"github.com/gophercloud/gophercloud"
-	"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
-	"github.com/gophercloud/gophercloud/pagination"
-	network_utils "github.com/gophercloud/utils/openstack/networking/v2/networks"
-
-	machinev1alpha1 "github.com/openshift/api/machine/v1alpha1"
-	types_openstack "github.com/openshift/installer/pkg/types/openstack"
-)
-
 type terraformFixedIP struct {
 	SubnetID  string `json:"subnet_id"`
 	IPAddress string `json:"ip_address"`
@@ -21,85 +9,3 @@ type terraformPort struct {
 	NetworkID string             `json:"network_id"`
 	FixedIP   []terraformFixedIP `json:"fixed_ips"`
 }
-
-func portTargetToTerraformPort(networkClient *gophercloud.ServiceClient, portTarget types_openstack.PortTarget) (terraformPort, error) {
-	networkID := portTarget.Network.ID
-	if networkID == "" && portTarget.Network.Name != "" {
-		var err error
-		networkID, err = network_utils.IDFromName(networkClient, portTarget.Network.Name)
-		if err != nil {
-			return terraformPort{}, fmt.Errorf("failed to resolve network ID for network name %q: %w", portTarget.Network.Name, err)
-		}
-	}
-
-	terraformFixedIPs := make([]terraformFixedIP, len(portTarget.FixedIPs))
-	for i := range terraformFixedIPs {
-		resolvedSubnetID, resolvedNetworkID, err := resolveSubnetFilter(networkClient, networkID, portTarget.FixedIPs[i].Subnet)
-		if err != nil {
-			return terraformPort{}, fmt.Errorf("failed to resolve the subnet filter: %w", err)
-		}
-
-		if networkID == "" {
-			networkID = resolvedNetworkID
-		}
-
-		if networkID != resolvedNetworkID {
-			return terraformPort{}, fmt.Errorf("port target has ports on multiple networks")
-		}
-
-		terraformFixedIPs[i] = terraformFixedIP{
-			SubnetID: resolvedSubnetID,
-		}
-	}
-
-	return terraformPort{
-		NetworkID: networkID,
-		FixedIP:   terraformFixedIPs,
-	}, nil
-}
-
-func resolveSubnetFilter(networkClient *gophercloud.ServiceClient, networkID string, subnetFilter machinev1alpha1.SubnetFilter) (resolvedSubnetID, resolvedNetworkID string, err error) {
-	if subnetFilter.ProjectID != "" {
-		subnetFilter.TenantID = ""
-	}
-	if err = subnets.List(networkClient, subnets.ListOpts{
-		NetworkID:       networkID,
-		Name:            subnetFilter.Name,
-		Description:     subnetFilter.Description,
-		TenantID:        subnetFilter.TenantID,
-		ProjectID:       subnetFilter.ProjectID,
-		IPVersion:       subnetFilter.IPVersion,
-		GatewayIP:       subnetFilter.GatewayIP,
-		CIDR:            subnetFilter.CIDR,
-		IPv6AddressMode: subnetFilter.IPv6AddressMode,
-		IPv6RAMode:      subnetFilter.IPv6RAMode,
-		ID:              subnetFilter.ID,
-		SubnetPoolID:    subnetFilter.SubnetPoolID,
-		Tags:            subnetFilter.Tags,
-		TagsAny:         subnetFilter.TagsAny,
-		NotTags:         subnetFilter.NotTags,
-		NotTagsAny:      subnetFilter.NotTagsAny,
-	}).EachPage(func(page pagination.Page) (bool, error) {
-		returnedSubnets, err := subnets.ExtractSubnets(page)
-		if err != nil {
-			return false, err
-		}
-		for _, subnet := range returnedSubnets {
-			if resolvedSubnetID == "" {
-				resolvedSubnetID = subnet.ID
-				resolvedNetworkID = subnet.NetworkID
-			} else {
-				return false, fmt.Errorf("more than one subnet found")
-			}
-		}
-		return true, nil
-	}); err != nil {
-		return "", "", fmt.Errorf("failed to list subnets: %w", err)
-	}
-
-	if resolvedSubnetID == "" {
-		return "", "", fmt.Errorf("no subnet found")
-	}
-
-	return resolvedSubnetID, resolvedNetworkID, err
-}

pkg/tfvars/openstack/ports.go

@@ -33,10 +33,6 @@ type MachinePool struct {
 	// If no zones are provided, all instances will be deployed on OpenStack Nova default availability zone
 	// +optional
 	Zones []string `json:"zones,omitempty"`
-
-	// FailureDomains is a Tech Preview feature for resource placement. It
-	// is incompatible with zones and rootVolume zones.
-	FailureDomains []FailureDomain `json:"failureDomains,omitempty"`
 }
 
 // Set sets the values from `required` to `o`.
@@ -75,10 +71,6 @@ func (o *MachinePool) Set(required *MachinePool) {
 	if len(required.Zones) > 0 {
 		o.Zones = required.Zones
 	}
-
-	if len(required.FailureDomains) > 0 {
-		o.FailureDomains = required.FailureDomains
-	}
 }
 
 // RootVolume defines the storage for an instance.
@@ -96,38 +88,6 @@ type RootVolume struct {
 	Zones []string `json:"zones,omitempty"`
 }
 
-// -=**
-// FailureDomain and its types are part of a Tech Preview feature, and may change before they're moved to openshift/api
-// **=-
-
-// FailureDomain defines a set of correlated fault domains across different
-// OpenStack services: compute, storage, and network.
-type FailureDomain struct {
-	// ComputeAvailabilityZone is the name of a valid nova availability zone. The server will be created in this availability zone.
-	// +optional
-	ComputeAvailabilityZone string `json:"computeAvailabilityZone"`
-
-	// StorageAvailabilityZone is the name of a valid cinder availability
-	// zone. This will be the availability zone of the root volume if one is
-	// specified.
-	// +optional
-	StorageAvailabilityZone string `json:"storageAvailabilityZone"`
-
-	// Ports defines a set of port targets which can be referenced by machines in this failure domain.
-	//
-	// +optional
-	PortTargets []NamedPortTarget `json:"portTargets"`
-}
-
-// NamedPortTarget includes an ID to characterize a PortTarget with its
-// intended purpose. If ID is set to "control-plane", then the PortTarget will
-// replace the default cluster primary network (or the machinesSubnet if
-// defined) as the first network for the machine.
-type NamedPortTarget struct {
-	ID         string `json:"id"`
-	PortTarget `json:",inline"`
-}
-
 // PortTarget defines, directly or indirectly, one or more subnets where to attach a port.
 type PortTarget struct {
 	// Network is a query for an openstack network that the port will be created or discovered on.

pkg/types/openstack/machinepool.go

@@ -25,44 +25,5 @@ func ValidateMachinePool(_ *openstack.Platform, machinePool *openstack.MachinePo
 		errs = append(errs, field.NotSupported(fldPath.Child("serverGroupPolicy"), machinePool.ServerGroupPolicy, validServerGroupPolicies))
 	}
 
-	errs = append(errs, validateFailureDomains(machinePool, role, fldPath)...)
-
-	return errs
-}
-
-func validateFailureDomains(machinePool *openstack.MachinePool, role string, fldPath *field.Path) (errs field.ErrorList) {
-	if len(machinePool.FailureDomains) == 0 {
-		return nil
-	}
-
-	fldPath = fldPath.Child("failureDomains")
-
-	if role != "master" {
-		return append(errs, field.Forbidden(fldPath, "failure domains can only be set on the master machine-pool"))
-	}
-
-	// No failure domains together with zones
-	if len(machinePool.Zones) > 1 || len(machinePool.Zones) > 0 && machinePool.Zones[0] != "" {
-		errs = append(errs, field.Forbidden(fldPath, "failure domains can not be set together with zones"))
-	}
-
-	// No failure domains together with root volume zones
-	if machinePool.RootVolume != nil {
-		if len(machinePool.RootVolume.Zones) > 1 || len(machinePool.RootVolume.Zones) > 0 && machinePool.RootVolume.Zones[0] != "" {
-			errs = append(errs, field.Forbidden(fldPath, "failure domains can not be set together with rootVolume zones"))
-		}
-	}
-
-	// portTarget IDs must be unique
-	for i := range machinePool.FailureDomains {
-		ids := make(map[string]struct{})
-		for _, portTarget := range machinePool.FailureDomains[i].PortTargets {
-			if _, ok := ids[portTarget.ID]; ok {
-				errs = append(errs, field.Duplicate(fldPath.Index(i).Child("id"), portTarget.ID))
-			}
-			ids[portTarget.ID] = struct{}{}
-		}
-	}
-
 	return errs
 }