From 607d731f4df1e294070b1b0799fd0d7d40cd7415 Mon Sep 17 00:00:00 2001 From: Michael Captain Date: Fri, 21 May 2021 11:28:38 +0300 Subject: [PATCH] Add feature to create ports with custom options This commit adds a new field to the v1alpha4 API: OpenStackMachineTemplateSpec.ports. The list of ports are added per instance. Each port may be customized with options. These ports are created in addition to any ports created in the `networks:` field. If `networks:` is not specified, only the ports specified in `ports:` will be created and attached to the instance. If neither `networks:` nor `ports:` are specified, the instance will be connected to the default cluster network and subnet. This feature is very much based on the work on jsen-, and a lot of credit goes there for the implementation: https://github.com/kubernetes-sigs/cluster-api-provider-openstack/pull/778 --- api/v1alpha3/conversion.go | 36 +++ api/v1alpha3/zz_generated.conversion.go | 122 +++++++-- api/v1alpha4/openstackmachine_types.go | 6 +- api/v1alpha4/types.go | 41 ++- api/v1alpha4/zz_generated.deepcopy.go | 81 ++++++ ...re.cluster.x-k8s.io_openstackclusters.yaml | 243 +++++++++++++++++- ...re.cluster.x-k8s.io_openstackmachines.yaml | 67 ++++- ...er.x-k8s.io_openstackmachinetemplates.yaml | 70 ++++- pkg/cloud/services/compute/instance.go | 185 +++++++++---- pkg/cloud/services/compute/instance_test.go | 59 +++++ 10 files changed, 818 insertions(+), 92 deletions(-) create mode 100644 pkg/cloud/services/compute/instance_test.go diff --git a/api/v1alpha3/conversion.go b/api/v1alpha3/conversion.go index e40506bc34..1c4944e1f8 100644 --- a/api/v1alpha3/conversion.go +++ b/api/v1alpha3/conversion.go @@ -118,3 +118,39 @@ func Convert_v1alpha3_OpenStackClusterSpec_To_v1alpha4_OpenStackClusterSpec(in * func Convert_v1alpha3_OpenStackMachineSpec_To_v1alpha4_OpenStackMachineSpec(in *OpenStackMachineSpec, out *v1alpha4.OpenStackMachineSpec, s conversion.Scope) error { return autoConvert_v1alpha3_OpenStackMachineSpec_To_v1alpha4_OpenStackMachineSpec(in, out, s) } + +// Convert_v1alpha3_OpenStackClusterSpec_To_v1alpha4_OpenStackClusterSpec has to be added by us for the new portOpts +// parameter in v1alpha4. There is no intention to support this parameter in v1alpha3, so the field is just dropped. +func Convert_v1alpha4_Network_To_v1alpha3_Network(in *v1alpha4.Network, out *Network, s conversion.Scope) error { + return autoConvert_v1alpha4_Network_To_v1alpha3_Network(in, out, s) +} + +// Convert_v1alpha3_OpenStackClusterSpec_To_v1alpha4_OpenStackClusterSpec has to be added by us for the new ports +// parameter in v1alpha4. There is no intention to support this parameter in v1alpha3, so the field is just dropped. +func Convert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec(in *v1alpha4.OpenStackMachineSpec, out *OpenStackMachineSpec, s conversion.Scope) error { + return autoConvert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec(in, out, s) +} + +func Convert_Slice_v1alpha4_Network_To_Slice_v1alpha3_Network(in *[]v1alpha4.Network, out *[]Network, s conversion.Scope) error { + for i := range *in { + inNet := &(*in)[i] + outNet := new(Network) + if err := autoConvert_v1alpha4_Network_To_v1alpha3_Network(inNet, outNet, s); err != nil { + return err + } + *out = append(*out, *outNet) + } + return nil +} + +func Convert_Slice_v1alpha3_Network_To_Slice_v1alpha4_Network(in *[]Network, out *[]v1alpha4.Network, s conversion.Scope) error { + for i := range *in { + inNet := &(*in)[i] + outNet := new(v1alpha4.Network) + if err := autoConvert_v1alpha3_Network_To_v1alpha4_Network(inNet, outNet, s); err != nil { + return err + } + *out = append(*out, *outNet) + } + return nil +} diff --git a/api/v1alpha3/zz_generated.conversion.go b/api/v1alpha3/zz_generated.conversion.go index 1a3b70eae8..ec31691ebc 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -94,11 +94,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1alpha4.Network)(nil), (*Network)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha4_Network_To_v1alpha3_Network(a.(*v1alpha4.Network), b.(*Network), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*NetworkParam)(nil), (*v1alpha4.NetworkParam)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_NetworkParam_To_v1alpha4_NetworkParam(a.(*NetworkParam), b.(*v1alpha4.NetworkParam), scope) }); err != nil { @@ -164,11 +159,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1alpha4.OpenStackMachineSpec)(nil), (*OpenStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec(a.(*v1alpha4.OpenStackMachineSpec), b.(*OpenStackMachineSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*OpenStackMachineStatus)(nil), (*v1alpha4.OpenStackMachineStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_OpenStackMachineStatus_To_v1alpha4_OpenStackMachineStatus(a.(*OpenStackMachineStatus), b.(*v1alpha4.OpenStackMachineStatus), scope) }); err != nil { @@ -309,6 +299,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*[]Network)(nil), (*[]v1alpha4.Network)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_Slice_v1alpha3_Network_To_Slice_v1alpha4_Network(a.(*[]Network), b.(*[]v1alpha4.Network), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*[]v1alpha4.Network)(nil), (*[]Network)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_Slice_v1alpha4_Network_To_Slice_v1alpha3_Network(a.(*[]v1alpha4.Network), b.(*[]Network), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*apiv1alpha3.APIEndpoint)(nil), (*apiv1alpha4.APIEndpoint)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_APIEndpoint_To_v1alpha4_APIEndpoint(a.(*apiv1alpha3.APIEndpoint), b.(*apiv1alpha4.APIEndpoint), scope) }); err != nil { @@ -329,6 +329,16 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha4.Network)(nil), (*Network)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha4_Network_To_v1alpha3_Network(a.(*v1alpha4.Network), b.(*Network), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*v1alpha4.OpenStackMachineSpec)(nil), (*OpenStackMachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec(a.(*v1alpha4.OpenStackMachineSpec), b.(*OpenStackMachineSpec), scope) + }); err != nil { + return err + } return nil } @@ -442,7 +452,15 @@ func autoConvert_v1alpha3_Instance_To_v1alpha4_Instance(in *Instance, out *v1alp out.Trunk = in.Trunk out.FailureDomain = in.FailureDomain out.SecurityGroups = (*[]string)(unsafe.Pointer(in.SecurityGroups)) - out.Networks = (*[]v1alpha4.Network)(unsafe.Pointer(in.Networks)) + if in.Networks != nil { + in, out := &in.Networks, &out.Networks + *out = new([]v1alpha4.Network) + if err := Convert_Slice_v1alpha3_Network_To_Slice_v1alpha4_Network(*in, *out, s); err != nil { + return err + } + } else { + out.Networks = nil + } out.Subnet = in.Subnet out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.Image = in.Image @@ -470,7 +488,15 @@ func autoConvert_v1alpha4_Instance_To_v1alpha3_Instance(in *v1alpha4.Instance, o out.Trunk = in.Trunk out.FailureDomain = in.FailureDomain out.SecurityGroups = (*[]string)(unsafe.Pointer(in.SecurityGroups)) - out.Networks = (*[]Network)(unsafe.Pointer(in.Networks)) + if in.Networks != nil { + in, out := &in.Networks, &out.Networks + *out = new([]Network) + if err := Convert_Slice_v1alpha4_Network_To_Slice_v1alpha3_Network(*in, *out, s); err != nil { + return err + } + } else { + out.Networks = nil + } out.Subnet = in.Subnet out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.Image = in.Image @@ -538,16 +564,12 @@ func autoConvert_v1alpha4_Network_To_v1alpha3_Network(in *v1alpha4.Network, out out.ID = in.ID out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.Subnet = (*Subnet)(unsafe.Pointer(in.Subnet)) + // WARNING: in.PortOpts requires manual conversion: does not exist in peer-type out.Router = (*Router)(unsafe.Pointer(in.Router)) out.APIServerLoadBalancer = (*LoadBalancer)(unsafe.Pointer(in.APIServerLoadBalancer)) return nil } -// Convert_v1alpha4_Network_To_v1alpha3_Network is an autogenerated conversion function. -func Convert_v1alpha4_Network_To_v1alpha3_Network(in *v1alpha4.Network, out *Network, s conversion.Scope) error { - return autoConvert_v1alpha4_Network_To_v1alpha3_Network(in, out, s) -} - func autoConvert_v1alpha3_NetworkParam_To_v1alpha4_NetworkParam(in *NetworkParam, out *v1alpha4.NetworkParam, s conversion.Scope) error { out.UUID = in.UUID out.FixedIP = in.FixedIP @@ -732,13 +754,37 @@ func Convert_v1alpha4_OpenStackClusterSpec_To_v1alpha3_OpenStackClusterSpec(in * func autoConvert_v1alpha3_OpenStackClusterStatus_To_v1alpha4_OpenStackClusterStatus(in *OpenStackClusterStatus, out *v1alpha4.OpenStackClusterStatus, s conversion.Scope) error { out.Ready = in.Ready - out.Network = (*v1alpha4.Network)(unsafe.Pointer(in.Network)) - out.ExternalNetwork = (*v1alpha4.Network)(unsafe.Pointer(in.ExternalNetwork)) + if in.Network != nil { + in, out := &in.Network, &out.Network + *out = new(v1alpha4.Network) + if err := Convert_v1alpha3_Network_To_v1alpha4_Network(*in, *out, s); err != nil { + return err + } + } else { + out.Network = nil + } + if in.ExternalNetwork != nil { + in, out := &in.ExternalNetwork, &out.ExternalNetwork + *out = new(v1alpha4.Network) + if err := Convert_v1alpha3_Network_To_v1alpha4_Network(*in, *out, s); err != nil { + return err + } + } else { + out.ExternalNetwork = nil + } out.FailureDomains = *(*apiv1alpha4.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) out.ControlPlaneSecurityGroup = (*v1alpha4.SecurityGroup)(unsafe.Pointer(in.ControlPlaneSecurityGroup)) out.WorkerSecurityGroup = (*v1alpha4.SecurityGroup)(unsafe.Pointer(in.WorkerSecurityGroup)) out.BastionSecurityGroup = (*v1alpha4.SecurityGroup)(unsafe.Pointer(in.BastionSecurityGroup)) - out.Bastion = (*v1alpha4.Instance)(unsafe.Pointer(in.Bastion)) + if in.Bastion != nil { + in, out := &in.Bastion, &out.Bastion + *out = new(v1alpha4.Instance) + if err := Convert_v1alpha3_Instance_To_v1alpha4_Instance(*in, *out, s); err != nil { + return err + } + } else { + out.Bastion = nil + } return nil } @@ -749,13 +795,37 @@ func Convert_v1alpha3_OpenStackClusterStatus_To_v1alpha4_OpenStackClusterStatus( func autoConvert_v1alpha4_OpenStackClusterStatus_To_v1alpha3_OpenStackClusterStatus(in *v1alpha4.OpenStackClusterStatus, out *OpenStackClusterStatus, s conversion.Scope) error { out.Ready = in.Ready - out.Network = (*Network)(unsafe.Pointer(in.Network)) - out.ExternalNetwork = (*Network)(unsafe.Pointer(in.ExternalNetwork)) + if in.Network != nil { + in, out := &in.Network, &out.Network + *out = new(Network) + if err := Convert_v1alpha4_Network_To_v1alpha3_Network(*in, *out, s); err != nil { + return err + } + } else { + out.Network = nil + } + if in.ExternalNetwork != nil { + in, out := &in.ExternalNetwork, &out.ExternalNetwork + *out = new(Network) + if err := Convert_v1alpha4_Network_To_v1alpha3_Network(*in, *out, s); err != nil { + return err + } + } else { + out.ExternalNetwork = nil + } out.FailureDomains = *(*apiv1alpha3.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) out.ControlPlaneSecurityGroup = (*SecurityGroup)(unsafe.Pointer(in.ControlPlaneSecurityGroup)) out.WorkerSecurityGroup = (*SecurityGroup)(unsafe.Pointer(in.WorkerSecurityGroup)) out.BastionSecurityGroup = (*SecurityGroup)(unsafe.Pointer(in.BastionSecurityGroup)) - out.Bastion = (*Instance)(unsafe.Pointer(in.Bastion)) + if in.Bastion != nil { + in, out := &in.Bastion, &out.Bastion + *out = new(Instance) + if err := Convert_v1alpha4_Instance_To_v1alpha3_Instance(*in, *out, s); err != nil { + return err + } + } else { + out.Bastion = nil + } return nil } @@ -869,6 +939,7 @@ func autoConvert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec( out.Image = in.Image out.SSHKeyName = in.SSHKeyName out.Networks = *(*[]NetworkParam)(unsafe.Pointer(&in.Networks)) + // WARNING: in.Ports requires manual conversion: does not exist in peer-type out.Subnet = in.Subnet out.FloatingIP = in.FloatingIP out.SecurityGroups = *(*[]SecurityGroupParam)(unsafe.Pointer(&in.SecurityGroups)) @@ -881,11 +952,6 @@ func autoConvert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec( return nil } -// Convert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec is an autogenerated conversion function. -func Convert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec(in *v1alpha4.OpenStackMachineSpec, out *OpenStackMachineSpec, s conversion.Scope) error { - return autoConvert_v1alpha4_OpenStackMachineSpec_To_v1alpha3_OpenStackMachineSpec(in, out, s) -} - func autoConvert_v1alpha3_OpenStackMachineStatus_To_v1alpha4_OpenStackMachineStatus(in *OpenStackMachineStatus, out *v1alpha4.OpenStackMachineStatus, s conversion.Scope) error { out.Ready = in.Ready out.Addresses = *(*[]v1.NodeAddress)(unsafe.Pointer(&in.Addresses)) diff --git a/api/v1alpha4/openstackmachine_types.go b/api/v1alpha4/openstackmachine_types.go index 8708cb1c02..8e2e03a4ee 100644 --- a/api/v1alpha4/openstackmachine_types.go +++ b/api/v1alpha4/openstackmachine_types.go @@ -55,9 +55,13 @@ type OpenStackMachineSpec struct { SSHKeyName string `json:"sshKeyName,omitempty"` // A networks object. Required parameter when there are multiple networks defined for the tenant. - // When you do not specify the networks parameter, the server attaches to the only network created for the current tenant. + // When you do not specify both networks and ports parameters, the server attaches to the only network created for the current tenant. Networks []NetworkParam `json:"networks,omitempty"` + // Ports to be attached to the server instance. They are created if a port with the given name does not already exist. + // When you do not specify both networks and ports parameters, the server attaches to the only network created for the current tenant. + Ports []PortOpts `json:"ports,omitempty"` + // UUID, IP address of a port from this subnet will be marked as AccessIPv4 on the created compute instance Subnet string `json:"subnet,omitempty"` diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go index 50c63860a4..0dff708269 100644 --- a/api/v1alpha4/types.go +++ b/api/v1alpha4/types.go @@ -58,7 +58,7 @@ type NetworkParam struct { // The UUID of the network. Required if you omit the port attribute. UUID string `json:"uuid,omitempty"` // A fixed IPv4 address for the NIC. - FixedIP string `json:"fixedIp,omitempty"` + FixedIP string `json:"fixedIP,omitempty"` // Filters for optional network query Filter Filter `json:"filter,omitempty"` // Subnet within a network to use @@ -116,6 +116,38 @@ type SubnetFilter struct { NotTagsAny string `json:"notTagsAny,omitempty"` } +type PortOpts struct { + // ID of the OpenStack network on which to create the port. If unspecified, create the port on the default cluster network. + NetworkID string `json:"networkId,omitempty"` + // Used to make the name of the port unique. If unspecified, instead the 0-based index of the port in the list is used. + NameSuffix string `json:"nameSuffix,omitempty"` + Description string `json:"description,omitempty"` + AdminStateUp *bool `json:"adminStateUp,omitempty"` + MACAddress string `json:"macAddress,omitempty"` + // Specify pairs of subnet and/or IP address. These should be subnets of the network with the given NetworkID. + FixedIPs []FixedIP `json:"fixedIps,omitempty"` + TenantID string `json:"tenantId,omitempty"` + ProjectID string `json:"projectId,omitempty"` + SecurityGroups *[]string `json:"securityGroups,omitempty"` + AllowedAddressPairs []AddressPair `json:"allowedAddressPairs,omitempty"` + + // The ID of the host where the port is allocated + HostID string `json:"hostId,omitempty"` + + // The virtual network interface card (vNIC) type that is bound to the neutron port. + VNICType string `json:"vnicType,omitempty"` +} + +type FixedIP struct { + SubnetID string `json:"subnetId"` + IPAddress string `json:"ipAddress,omitempty"` +} + +type AddressPair struct { + IPAddress string `json:"ipAddress,omitempty"` + MACAddress string `json:"macAddress,omitempty"` +} + type Instance struct { ID string `json:"id,omitempty"` Name string `json:"name,omitempty"` @@ -145,7 +177,7 @@ type RootVolume struct { Size int `json:"diskSize,omitempty"` } -// Network represents basic information about the associated OpenStach Neutron Network. +// Network represents basic information about an OpenStack Neutron Network associated with an instance's port. type Network struct { Name string `json:"name"` ID string `json:"id"` @@ -153,8 +185,9 @@ type Network struct { //+optional Tags []string `json:"tags,omitempty"` - Subnet *Subnet `json:"subnet,omitempty"` - Router *Router `json:"router,omitempty"` + Subnet *Subnet `json:"subnet,omitempty"` + PortOpts *PortOpts `json:"port,omitempty"` + Router *Router `json:"router,omitempty"` // Be careful when using APIServerLoadBalancer, because this field is optional and therefore not // set in all cases diff --git a/api/v1alpha4/zz_generated.deepcopy.go b/api/v1alpha4/zz_generated.deepcopy.go index ef1f2e546a..70df98126d 100644 --- a/api/v1alpha4/zz_generated.deepcopy.go +++ b/api/v1alpha4/zz_generated.deepcopy.go @@ -27,6 +27,21 @@ import ( "sigs.k8s.io/cluster-api/errors" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AddressPair) DeepCopyInto(out *AddressPair) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AddressPair. +func (in *AddressPair) DeepCopy() *AddressPair { + if in == nil { + return nil + } + out := new(AddressPair) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Bastion) DeepCopyInto(out *Bastion) { *out = *in @@ -84,6 +99,21 @@ func (in *Filter) DeepCopy() *Filter { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *FixedIP) DeepCopyInto(out *FixedIP) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new FixedIP. +func (in *FixedIP) DeepCopy() *FixedIP { + if in == nil { + return nil + } + out := new(FixedIP) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Instance) DeepCopyInto(out *Instance) { *out = *in @@ -169,6 +199,11 @@ func (in *Network) DeepCopyInto(out *Network) { *out = new(Subnet) (*in).DeepCopyInto(*out) } + if in.PortOpts != nil { + in, out := &in.PortOpts, &out.PortOpts + *out = new(PortOpts) + (*in).DeepCopyInto(*out) + } if in.Router != nil { in, out := &in.Router, &out.Router *out = new(Router) @@ -464,6 +499,13 @@ func (in *OpenStackMachineSpec) DeepCopyInto(out *OpenStackMachineSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = make([]PortOpts, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.SecurityGroups != nil { in, out := &in.SecurityGroups, &out.SecurityGroups *out = make([]SecurityGroupParam, len(*in)) @@ -628,6 +670,45 @@ func (in *OpenStackMachineTemplateSpec) DeepCopy() *OpenStackMachineTemplateSpec return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PortOpts) DeepCopyInto(out *PortOpts) { + *out = *in + if in.AdminStateUp != nil { + in, out := &in.AdminStateUp, &out.AdminStateUp + *out = new(bool) + **out = **in + } + if in.FixedIPs != nil { + in, out := &in.FixedIPs, &out.FixedIPs + *out = make([]FixedIP, len(*in)) + copy(*out, *in) + } + if in.SecurityGroups != nil { + in, out := &in.SecurityGroups, &out.SecurityGroups + *out = new([]string) + if **in != nil { + in, out := *in, *out + *out = make([]string, len(*in)) + copy(*out, *in) + } + } + if in.AllowedAddressPairs != nil { + in, out := &in.AllowedAddressPairs, &out.AllowedAddressPairs + *out = make([]AddressPair, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PortOpts. +func (in *PortOpts) DeepCopy() *PortOpts { + if in == nil { + return nil + } + out := new(PortOpts) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RootVolume) DeepCopyInto(out *RootVolume) { *out = *in diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index 34fc8986bb..33aec7802e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -1136,8 +1136,8 @@ spec: networks: description: A networks object. Required parameter when there are multiple networks defined for the tenant. When you do - not specify the networks parameter, the server attaches - to the only network created for the current tenant. + not specify both networks and ports parameters, the server + attaches to the only network created for the current tenant. items: properties: filter: @@ -1176,7 +1176,7 @@ spec: tenantId: type: string type: object - fixedIp: + fixedIP: description: A fixed IPv4 address for the NIC. type: string subnets: @@ -1241,6 +1241,70 @@ spec: type: string type: object type: array + ports: + description: Ports to be attached to the server instance. + They are created if a port with the given name does not + already exist. When you do not specify both networks and + ports parameters, the server attaches to the only network + created for the current tenant. + items: + properties: + adminStateUp: + type: boolean + allowedAddressPairs: + items: + properties: + ipAddress: + type: string + macAddress: + type: string + type: object + type: array + description: + type: string + fixedIps: + description: Specify pairs of subnet and/or IP address. + These should be subnets of the network with the given + NetworkID. + items: + properties: + ipAddress: + type: string + subnetId: + type: string + required: + - subnetId + type: object + type: array + hostId: + description: The ID of the host where the port is allocated + type: string + macAddress: + type: string + nameSuffix: + description: Used to make the name of the port unique. + If unspecified, instead the 0-based index of the port + in the list is used. + type: string + networkId: + description: ID of the OpenStack network on which to + create the port. If unspecified, create the port on + the default cluster network. + type: string + projectId: + type: string + securityGroups: + items: + type: string + type: array + tenantId: + type: string + vnicType: + description: The virtual network interface card (vNIC) + type that is bound to the neutron port. + type: string + type: object + type: array providerID: description: ProviderID is the unique identifier as specified by the cloud provider. @@ -1586,8 +1650,8 @@ spec: type: string networks: items: - description: Network represents basic information about the - associated OpenStach Neutron Network. + description: Network represents basic information about an OpenStack + Neutron Network associated with an instance's port. properties: apiServerLoadBalancer: description: Be careful when using APIServerLoadBalancer, @@ -1612,6 +1676,63 @@ spec: type: string name: type: string + port: + properties: + adminStateUp: + type: boolean + allowedAddressPairs: + items: + properties: + ipAddress: + type: string + macAddress: + type: string + type: object + type: array + description: + type: string + fixedIps: + description: Specify pairs of subnet and/or IP address. + These should be subnets of the network with the given + NetworkID. + items: + properties: + ipAddress: + type: string + subnetId: + type: string + required: + - subnetId + type: object + type: array + hostId: + description: The ID of the host where the port is allocated + type: string + macAddress: + type: string + nameSuffix: + description: Used to make the name of the port unique. + If unspecified, instead the 0-based index of the port + in the list is used. + type: string + networkId: + description: ID of the OpenStack network on which to + create the port. If unspecified, create the port on + the default cluster network. + type: string + projectId: + type: string + securityGroups: + items: + type: string + type: array + tenantId: + type: string + vnicType: + description: The virtual network interface card (vNIC) + type that is bound to the neutron port. + type: string + type: object router: description: Router represents basic information about the associated OpenStack Neutron Router. @@ -1820,6 +1941,62 @@ spec: type: string name: type: string + port: + properties: + adminStateUp: + type: boolean + allowedAddressPairs: + items: + properties: + ipAddress: + type: string + macAddress: + type: string + type: object + type: array + description: + type: string + fixedIps: + description: Specify pairs of subnet and/or IP address. These + should be subnets of the network with the given NetworkID. + items: + properties: + ipAddress: + type: string + subnetId: + type: string + required: + - subnetId + type: object + type: array + hostId: + description: The ID of the host where the port is allocated + type: string + macAddress: + type: string + nameSuffix: + description: Used to make the name of the port unique. If + unspecified, instead the 0-based index of the port in the + list is used. + type: string + networkId: + description: ID of the OpenStack network on which to create + the port. If unspecified, create the port on the default + cluster network. + type: string + projectId: + type: string + securityGroups: + items: + type: string + type: array + tenantId: + type: string + vnicType: + description: The virtual network interface card (vNIC) type + that is bound to the neutron port. + type: string + type: object router: description: Router represents basic information about the associated OpenStack Neutron Router. @@ -1908,6 +2085,62 @@ spec: type: string name: type: string + port: + properties: + adminStateUp: + type: boolean + allowedAddressPairs: + items: + properties: + ipAddress: + type: string + macAddress: + type: string + type: object + type: array + description: + type: string + fixedIps: + description: Specify pairs of subnet and/or IP address. These + should be subnets of the network with the given NetworkID. + items: + properties: + ipAddress: + type: string + subnetId: + type: string + required: + - subnetId + type: object + type: array + hostId: + description: The ID of the host where the port is allocated + type: string + macAddress: + type: string + nameSuffix: + description: Used to make the name of the port unique. If + unspecified, instead the 0-based index of the port in the + list is used. + type: string + networkId: + description: ID of the OpenStack network on which to create + the port. If unspecified, create the port on the default + cluster network. + type: string + projectId: + type: string + securityGroups: + items: + type: string + type: array + tenantId: + type: string + vnicType: + description: The virtual network interface card (vNIC) type + that is bound to the neutron port. + type: string + type: object router: description: Router represents basic information about the associated OpenStack Neutron Router. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml index a03d8050ec..baa23dc75e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -431,8 +431,8 @@ spec: networks: description: A networks object. Required parameter when there are multiple networks defined for the tenant. When you do not specify - the networks parameter, the server attaches to the only network - created for the current tenant. + both networks and ports parameters, the server attaches to the only + network created for the current tenant. items: properties: filter: @@ -471,7 +471,7 @@ spec: tenantId: type: string type: object - fixedIp: + fixedIP: description: A fixed IPv4 address for the NIC. type: string subnets: @@ -536,6 +536,67 @@ spec: type: string type: object type: array + ports: + description: Ports to be attached to the server instance. They are + created if a port with the given name does not already exist. When + you do not specify both networks and ports parameters, the server + attaches to the only network created for the current tenant. + items: + properties: + adminStateUp: + type: boolean + allowedAddressPairs: + items: + properties: + ipAddress: + type: string + macAddress: + type: string + type: object + type: array + description: + type: string + fixedIps: + description: Specify pairs of subnet and/or IP address. These + should be subnets of the network with the given NetworkID. + items: + properties: + ipAddress: + type: string + subnetId: + type: string + required: + - subnetId + type: object + type: array + hostId: + description: The ID of the host where the port is allocated + type: string + macAddress: + type: string + nameSuffix: + description: Used to make the name of the port unique. If unspecified, + instead the 0-based index of the port in the list is used. + type: string + networkId: + description: ID of the OpenStack network on which to create + the port. If unspecified, create the port on the default cluster + network. + type: string + projectId: + type: string + securityGroups: + items: + type: string + type: array + tenantId: + type: string + vnicType: + description: The virtual network interface card (vNIC) type + that is bound to the neutron port. + type: string + type: object + type: array providerID: description: ProviderID is the unique identifier as specified by the cloud provider. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml index 56d948e75d..d6f3a07263 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -375,8 +375,8 @@ spec: networks: description: A networks object. Required parameter when there are multiple networks defined for the tenant. When you do - not specify the networks parameter, the server attaches - to the only network created for the current tenant. + not specify both networks and ports parameters, the server + attaches to the only network created for the current tenant. items: properties: filter: @@ -415,7 +415,7 @@ spec: tenantId: type: string type: object - fixedIp: + fixedIP: description: A fixed IPv4 address for the NIC. type: string subnets: @@ -480,6 +480,70 @@ spec: type: string type: object type: array + ports: + description: Ports to be attached to the server instance. + They are created if a port with the given name does not + already exist. When you do not specify both networks and + ports parameters, the server attaches to the only network + created for the current tenant. + items: + properties: + adminStateUp: + type: boolean + allowedAddressPairs: + items: + properties: + ipAddress: + type: string + macAddress: + type: string + type: object + type: array + description: + type: string + fixedIps: + description: Specify pairs of subnet and/or IP address. + These should be subnets of the network with the given + NetworkID. + items: + properties: + ipAddress: + type: string + subnetId: + type: string + required: + - subnetId + type: object + type: array + hostId: + description: The ID of the host where the port is allocated + type: string + macAddress: + type: string + nameSuffix: + description: Used to make the name of the port unique. + If unspecified, instead the 0-based index of the port + in the list is used. + type: string + networkId: + description: ID of the OpenStack network on which to + create the port. If unspecified, create the port on + the default cluster network. + type: string + projectId: + type: string + securityGroups: + items: + type: string + type: array + tenantId: + type: string + vnicType: + description: The virtual network interface card (vNIC) + type that is bound to the neutron port. + type: string + type: object + type: array providerID: description: ProviderID is the unique identifier as specified by the cloud provider. diff --git a/pkg/cloud/services/compute/instance.go b/pkg/cloud/services/compute/instance.go index 2bb43e4969..85483f73a2 100644 --- a/pkg/cloud/services/compute/instance.go +++ b/pkg/cloud/services/compute/instance.go @@ -34,6 +34,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" netext "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions" "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/security/groups" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/trunks" "github.com/gophercloud/gophercloud/openstack/networking/v2/networks" @@ -125,6 +126,22 @@ func (s *Service) CreateInstance(openStackCluster *infrav1.OpenStackCluster, mac } input.SecurityGroups = &securityGroups + nets, err := s.constructNetworks(openStackCluster, openStackMachine) + if err != nil { + return nil, err + } + input.Networks = nets + + out, err := s.createInstance(openStackMachine, clusterName, input) + if err != nil { + return nil, err + } + return out, nil +} + +// constructNetworks builds an array of networks from the network, subnet and ports items in the machine spec. +// If no networks or ports are in the spec, returns a single network item for a network connection to the default cluster network. +func (s *Service) constructNetworks(openStackCluster *infrav1.OpenStackCluster, openStackMachine *infrav1.OpenStackMachine) (*[]infrav1.Network, error) { var nets []infrav1.Network if len(openStackMachine.Spec.Networks) > 0 { var err error @@ -132,7 +149,26 @@ func (s *Service) CreateInstance(openStackCluster *infrav1.OpenStackCluster, mac if err != nil { return nil, err } - } else { + } + for i, port := range openStackMachine.Spec.Ports { + if port.NetworkID != "" { + nets = append(nets, infrav1.Network{ + ID: port.NetworkID, + Subnet: &infrav1.Subnet{}, + PortOpts: &openStackMachine.Spec.Ports[i], + }) + } 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], + }) + } + } + // no networks or ports found in the spec, so create a port on the cluster network + if len(nets) == 0 { nets = []infrav1.Network{{ ID: openStackCluster.Status.Network.ID, Subnet: &infrav1.Subnet{ @@ -140,42 +176,37 @@ func (s *Service) CreateInstance(openStackCluster *infrav1.OpenStackCluster, mac }, }} } - input.Networks = &nets - - out, err := s.createInstance(openStackMachine, clusterName, input) - if err != nil { - return nil, err - } - return out, nil + return &nets, nil } -func (s *Service) createInstance(eventObject runtime.Object, clusterName string, i *infrav1.Instance) (*infrav1.Instance, error) { +func (s *Service) createInstance(eventObject runtime.Object, clusterName string, instance *infrav1.Instance) (*infrav1.Instance, error) { accessIPv4 := "" portList := []servers.Network{} - for _, network := range *i.Networks { + for i, network := range *instance.Networks { if network.ID == "" { return nil, fmt.Errorf("no network was found or provided. Please check your machine configuration and try again") } - port, err := s.getOrCreatePort(eventObject, clusterName, i.Name, network, i.SecurityGroups) + portName := getPortName(instance.Name, network.PortOpts, i) + port, err := s.getOrCreatePort(eventObject, clusterName, portName, network, instance.SecurityGroups) if err != nil { return nil, err } - if i.Trunk { - trunk, err := s.getOrCreateTrunk(eventObject, clusterName, i.Name, port.ID) + 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, i.Tags); err != nil { + if err = s.replaceAllAttributesTags(eventObject, trunk.ID, instance.Tags); err != nil { return nil, err } } for _, fip := range port.FixedIPs { - if fip.SubnetID == i.Subnet { + if fip.SubnetID == instance.Subnet { accessIPv4 = fip.IPAddress } } @@ -185,46 +216,46 @@ func (s *Service) createInstance(eventObject runtime.Object, clusterName string, }) } - if i.Subnet != "" && accessIPv4 == "" { + if instance.Subnet != "" && accessIPv4 == "" { if err := s.deletePorts(eventObject, portList); err != nil { return nil, err } - return nil, fmt.Errorf("no ports with fixed IPs found on Subnet %q", i.Subnet) + return nil, fmt.Errorf("no ports with fixed IPs found on Subnet %q", instance.Subnet) } - imageID, err := s.getImageID(i.Image) + imageID, err := s.getImageID(instance.Image) if err != nil { return nil, fmt.Errorf("create new server: %v", err) } - flavorID, err := flavors.IDFromName(s.computeClient, i.Flavor) + flavorID, err := flavors.IDFromName(s.computeClient, instance.Flavor) if err != nil { - return nil, fmt.Errorf("error getting flavor id from flavor name %s: %v", i.Flavor, err) + return nil, fmt.Errorf("error getting flavor id from flavor name %s: %v", instance.Flavor, err) } var serverCreateOpts servers.CreateOptsBuilder = servers.CreateOpts{ - Name: i.Name, + Name: instance.Name, ImageRef: imageID, FlavorRef: flavorID, - AvailabilityZone: i.FailureDomain, + AvailabilityZone: instance.FailureDomain, Networks: portList, - UserData: []byte(i.UserData), - SecurityGroups: *i.SecurityGroups, - Tags: i.Tags, - Metadata: i.Metadata, - ConfigDrive: i.ConfigDrive, + UserData: []byte(instance.UserData), + SecurityGroups: *instance.SecurityGroups, + Tags: instance.Tags, + Metadata: instance.Metadata, + ConfigDrive: instance.ConfigDrive, AccessIPv4: accessIPv4, } - serverCreateOpts = applyRootVolume(serverCreateOpts, i.RootVolume) + serverCreateOpts = applyRootVolume(serverCreateOpts, instance.RootVolume) - serverCreateOpts = applyServerGroupID(serverCreateOpts, i.ServerGroupID) + serverCreateOpts = applyServerGroupID(serverCreateOpts, instance.ServerGroupID) mc := metrics.NewMetricPrometheusContext("server", "create") server, err := servers.Create(s.computeClient, keypairs.CreateOptsExt{ CreateOptsBuilder: serverCreateOpts, - KeyName: i.SSHKeyName, + KeyName: instance.SSHKeyName, }).Extract() if mc.ObserveRequest(err) != nil { @@ -235,24 +266,32 @@ func (s *Service) createInstance(eventObject runtime.Object, clusterName string, } instanceCreateTimeout := getTimeout("CLUSTER_API_OPENSTACK_INSTANCE_CREATE_TIMEOUT", TimeoutInstanceCreate) instanceCreateTimeout *= time.Minute - var instance *infrav1.Instance + var createdInstance *infrav1.Instance err = util.PollImmediate(RetryIntervalInstanceStatus, instanceCreateTimeout, func() (bool, error) { - instance, err = s.GetInstance(server.ID) + createdInstance, err = s.GetInstance(server.ID) if err != nil { if capoerrors.IsRetryable(err) { return false, nil } return false, err } - return instance.State == infrav1.InstanceStateActive, nil + return createdInstance.State == infrav1.InstanceStateActive, nil }) if err != nil { - record.Warnf(eventObject, "FailedCreateServer", "Failed to create server %s: %v", instance.Name, err) + record.Warnf(eventObject, "FailedCreateServer", "Failed to create server %s: %v", createdInstance.Name, err) return nil, err } - record.Eventf(eventObject, "SuccessfulCreateServer", "Created server %s with id %s", instance.Name, instance.ID) - return instance, nil + record.Eventf(eventObject, "SuccessfulCreateServer", "Created server %s with id %s", createdInstance.Name, createdInstance.ID) + return createdInstance, nil +} + +// getPortName appends a suffix to an instance name in order to try and get a unique name per port. +func getPortName(instanceName string, opts *infrav1.PortOpts, netIndex int) string { + if opts != nil && opts.NameSuffix != "" { + return fmt.Sprintf("%s-%s", instanceName, opts.NameSuffix) + } + return fmt.Sprintf("%s-%d", instanceName, netIndex) } // applyRootVolume sets a root volume if the root volume Size is not 0. @@ -353,7 +392,8 @@ func (s *Service) getServerNetworks(networkParams []infrav1.NetworkParam) ([]inf for _, netID := range ids { if networkParam.Subnets == nil { nets = append(nets, infrav1.Network{ - ID: netID, + ID: netID, + Subnet: &infrav1.Subnet{}, }) continue } @@ -380,7 +420,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, securityGroups *[]string) (*ports.Port, error) { +func (s *Service) getOrCreatePort(eventObject runtime.Object, clusterName string, portName string, net infrav1.Network, instanceSecurityGroups *[]string) (*ports.Port, error) { allPages, err := ports.List(s.networkClient, ports.ListOpts{ Name: portName, NetworkID: net.ID, @@ -388,27 +428,76 @@ func (s *Service) getOrCreatePort(eventObject runtime.Object, clusterName string if err != nil { return nil, fmt.Errorf("searching for existing port for server: %v", err) } - portList, err := ports.ExtractPorts(allPages) + existingPorts, err := ports.ExtractPorts(allPages) if err != nil { return nil, fmt.Errorf("searching for existing port for server: %v", err) } - if len(portList) != 0 { - return &portList[0], nil + if len(existingPorts) == 1 { + return &existingPorts[0], nil + } + + if len(existingPorts) > 1 { + return nil, fmt.Errorf("multiple ports found with name \"%s\"", portName) + } + + // no port found, so create the port + portOpts := net.PortOpts + if portOpts == nil { + portOpts = &infrav1.PortOpts{} + } + + description := portOpts.Description + if description == "" { + description = names.GetDescription(clusterName) } - portCreateOpts := ports.CreateOpts{ - Name: portName, - NetworkID: net.ID, - SecurityGroups: securityGroups, - Description: names.GetDescription(clusterName), + // inherit port security groups from the instance if not explicitly specified + securityGroups := portOpts.SecurityGroups + if securityGroups == nil { + securityGroups = instanceSecurityGroups + } + + createOpts := ports.CreateOpts{ + Name: portName, + NetworkID: net.ID, + Description: description, + AdminStateUp: portOpts.AdminStateUp, + MACAddress: portOpts.MACAddress, + TenantID: portOpts.TenantID, + ProjectID: portOpts.ProjectID, + SecurityGroups: securityGroups, + AllowedAddressPairs: []ports.AddressPair{}, + } + + for _, ap := range portOpts.AllowedAddressPairs { + createOpts.AllowedAddressPairs = append(createOpts.AllowedAddressPairs, ports.AddressPair{ + IPAddress: ap.IPAddress, + MACAddress: ap.MACAddress, + }) + } + + fixedIPs := make([]ports.IP, 0, len(portOpts.FixedIPs)+1) + for _, fixedIP := range portOpts.FixedIPs { + fixedIPs = append(fixedIPs, ports.IP{ + SubnetID: fixedIP.SubnetID, + IPAddress: fixedIP.IPAddress, + }) } if net.Subnet.ID != "" { - portCreateOpts.FixedIPs = []ports.IP{{SubnetID: net.Subnet.ID}} + fixedIPs = append(fixedIPs, ports.IP{SubnetID: net.Subnet.ID}) + } + if len(fixedIPs) > 0 { + createOpts.FixedIPs = fixedIPs } mc := metrics.NewMetricPrometheusContext("port", "create") - port, err := ports.Create(s.networkClient, portCreateOpts).Extract() + port, err := ports.Create(s.networkClient, portsbinding.CreateOptsExt{ + CreateOptsBuilder: createOpts, + HostID: portOpts.HostID, + VNICType: portOpts.VNICType, + Profile: nil, + }).Extract() if mc.ObserveRequest(err) != nil { record.Warnf(eventObject, "FailedCreatePort", "Failed to create port %s: %v", portName, err) return nil, err diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go new file mode 100644 index 0000000000..a87eb24059 --- /dev/null +++ b/pkg/cloud/services/compute/instance_test.go @@ -0,0 +1,59 @@ +/* +Copyright 2018 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" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha4" +) + +func Test_getPortName(t *testing.T) { + type args struct { + instanceName string + opts *infrav1.PortOpts + netIndex int + } + tests := []struct { + name string + args args + want string + }{ + { + name: "with nil PortOpts", + args: args{"test-1-instance", nil, 2}, + want: "test-1-instance-2", + }, + { + name: "with PortOpts name suffix", + args: args{"test-1-instance", &infrav1.PortOpts{NameSuffix: "foo"}, 4}, + want: "test-1-instance-foo", + }, + { + name: "without PortOpts name suffix", + args: args{"test-1-instance", &infrav1.PortOpts{}, 4}, + want: "test-1-instance-4", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := getPortName(tt.args.instanceName, tt.args.opts, tt.args.netIndex); got != tt.want { + t.Errorf("getPortName() = %v, want %v", got, tt.want) + } + }) + } +}