From 77a32a058b88dcd6ead7cbc84a1e23ab81c56bb2 Mon Sep 17 00:00:00 2001 From: Nutrymaco Date: Wed, 28 Aug 2024 14:03:03 +0300 Subject: [PATCH 1/4] add Topology.NetworkConfigurations --- apis/v1alpha3/topology_conversion.go | 26 +++ apis/v1alpha3/zz_generated.conversion.go | 40 ++-- apis/v1alpha4/topology_conversion.go | 26 +++ apis/v1alpha4/zz_generated.conversion.go | 40 ++-- apis/v1beta1/vspherefailuredomain_types.go | 58 ++++++ apis/v1beta1/zz_generated.deepcopy.go | 59 ++++++ ...luster.x-k8s.io_vspherefailuredomains.yaml | 184 ++++++++++++++++++ ...vspheredeploymentzone_controller_domain.go | 9 +- internal/webhooks/vspherefailuredomain.go | 10 + pkg/services/vimmachine.go | 59 +++++- pkg/services/vimmachine_test.go | 178 +++++++++++++++++ 11 files changed, 657 insertions(+), 32 deletions(-) create mode 100644 apis/v1alpha3/topology_conversion.go create mode 100644 apis/v1alpha4/topology_conversion.go diff --git a/apis/v1alpha3/topology_conversion.go b/apis/v1alpha3/topology_conversion.go new file mode 100644 index 0000000000..6fe140b1e6 --- /dev/null +++ b/apis/v1alpha3/topology_conversion.go @@ -0,0 +1,26 @@ +/* +Copyright 2024 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 v1alpha3 + +import ( + conversion "k8s.io/apimachinery/pkg/conversion" + v1beta1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" +) + +func Convert_v1beta1_Topology_To_v1alpha3_Topology(in *v1beta1.Topology, out *Topology, s conversion.Scope) error { + return autoConvert_v1beta1_Topology_To_v1alpha3_Topology(in, out, s) +} diff --git a/apis/v1alpha3/zz_generated.conversion.go b/apis/v1alpha3/zz_generated.conversion.go index 966195ccbb..a3223189ae 100644 --- a/apis/v1alpha3/zz_generated.conversion.go +++ b/apis/v1alpha3/zz_generated.conversion.go @@ -155,11 +155,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.Topology)(nil), (*Topology)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_Topology_To_v1alpha3_Topology(a.(*v1beta1.Topology), b.(*Topology), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*VSphereCluster)(nil), (*v1beta1.VSphereCluster)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_VSphereCluster_To_v1beta1_VSphereCluster(a.(*VSphereCluster), b.(*v1beta1.VSphereCluster), scope) }); err != nil { @@ -460,6 +455,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.Topology)(nil), (*Topology)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_Topology_To_v1alpha3_Topology(a.(*v1beta1.Topology), b.(*Topology), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.VSphereClusterSpec)(nil), (*VSphereClusterSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_VSphereClusterSpec_To_v1alpha3_VSphereClusterSpec(a.(*v1beta1.VSphereClusterSpec), b.(*VSphereClusterSpec), scope) }); err != nil { @@ -859,15 +859,11 @@ func autoConvert_v1beta1_Topology_To_v1alpha3_Topology(in *v1beta1.Topology, out out.ComputeCluster = (*string)(unsafe.Pointer(in.ComputeCluster)) out.Hosts = (*FailureDomainHosts)(unsafe.Pointer(in.Hosts)) out.Networks = *(*[]string)(unsafe.Pointer(&in.Networks)) + // WARNING: in.NetworkConfigurations requires manual conversion: does not exist in peer-type out.Datastore = in.Datastore return nil } -// Convert_v1beta1_Topology_To_v1alpha3_Topology is an autogenerated conversion function. -func Convert_v1beta1_Topology_To_v1alpha3_Topology(in *v1beta1.Topology, out *Topology, s conversion.Scope) error { - return autoConvert_v1beta1_Topology_To_v1alpha3_Topology(in, out, s) -} - func autoConvert_v1alpha3_VSphereCluster_To_v1beta1_VSphereCluster(in *VSphereCluster, out *v1beta1.VSphereCluster, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_v1alpha3_VSphereClusterSpec_To_v1beta1_VSphereClusterSpec(&in.Spec, &out.Spec, s); err != nil { @@ -1219,7 +1215,17 @@ func Convert_v1beta1_VSphereFailureDomain_To_v1alpha3_VSphereFailureDomain(in *v func autoConvert_v1alpha3_VSphereFailureDomainList_To_v1beta1_VSphereFailureDomainList(in *VSphereFailureDomainList, out *v1beta1.VSphereFailureDomainList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v1beta1.VSphereFailureDomain)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v1beta1.VSphereFailureDomain, len(*in)) + for i := range *in { + if err := Convert_v1alpha3_VSphereFailureDomain_To_v1beta1_VSphereFailureDomain(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1230,7 +1236,17 @@ func Convert_v1alpha3_VSphereFailureDomainList_To_v1beta1_VSphereFailureDomainLi func autoConvert_v1beta1_VSphereFailureDomainList_To_v1alpha3_VSphereFailureDomainList(in *v1beta1.VSphereFailureDomainList, out *VSphereFailureDomainList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]VSphereFailureDomain)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]VSphereFailureDomain, len(*in)) + for i := range *in { + if err := Convert_v1beta1_VSphereFailureDomain_To_v1alpha3_VSphereFailureDomain(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } diff --git a/apis/v1alpha4/topology_conversion.go b/apis/v1alpha4/topology_conversion.go new file mode 100644 index 0000000000..fa5f515940 --- /dev/null +++ b/apis/v1alpha4/topology_conversion.go @@ -0,0 +1,26 @@ +/* +Copyright 2024 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 v1alpha4 + +import ( + conversion "k8s.io/apimachinery/pkg/conversion" + v1beta1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" +) + +func Convert_v1beta1_Topology_To_v1alpha4_Topology(in *v1beta1.Topology, out *Topology, s conversion.Scope) error { + return autoConvert_v1beta1_Topology_To_v1alpha4_Topology(in, out, s) +} diff --git a/apis/v1alpha4/zz_generated.conversion.go b/apis/v1alpha4/zz_generated.conversion.go index 147c1a9894..a348b92c47 100644 --- a/apis/v1alpha4/zz_generated.conversion.go +++ b/apis/v1alpha4/zz_generated.conversion.go @@ -155,11 +155,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.Topology)(nil), (*Topology)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_Topology_To_v1alpha4_Topology(a.(*v1beta1.Topology), b.(*Topology), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*VSphereCluster)(nil), (*v1beta1.VSphereCluster)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha4_VSphereCluster_To_v1beta1_VSphereCluster(a.(*VSphereCluster), b.(*v1beta1.VSphereCluster), scope) }); err != nil { @@ -500,6 +495,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.Topology)(nil), (*Topology)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_Topology_To_v1alpha4_Topology(a.(*v1beta1.Topology), b.(*Topology), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.VSphereClusterSpec)(nil), (*VSphereClusterSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_VSphereClusterSpec_To_v1alpha4_VSphereClusterSpec(a.(*v1beta1.VSphereClusterSpec), b.(*VSphereClusterSpec), scope) }); err != nil { @@ -895,15 +895,11 @@ func autoConvert_v1beta1_Topology_To_v1alpha4_Topology(in *v1beta1.Topology, out out.ComputeCluster = (*string)(unsafe.Pointer(in.ComputeCluster)) out.Hosts = (*FailureDomainHosts)(unsafe.Pointer(in.Hosts)) out.Networks = *(*[]string)(unsafe.Pointer(&in.Networks)) + // WARNING: in.NetworkConfigurations requires manual conversion: does not exist in peer-type out.Datastore = in.Datastore return nil } -// Convert_v1beta1_Topology_To_v1alpha4_Topology is an autogenerated conversion function. -func Convert_v1beta1_Topology_To_v1alpha4_Topology(in *v1beta1.Topology, out *Topology, s conversion.Scope) error { - return autoConvert_v1beta1_Topology_To_v1alpha4_Topology(in, out, s) -} - func autoConvert_v1alpha4_VSphereCluster_To_v1beta1_VSphereCluster(in *VSphereCluster, out *v1beta1.VSphereCluster, s conversion.Scope) error { out.ObjectMeta = in.ObjectMeta if err := Convert_v1alpha4_VSphereClusterSpec_To_v1beta1_VSphereClusterSpec(&in.Spec, &out.Spec, s); err != nil { @@ -1373,7 +1369,17 @@ func Convert_v1beta1_VSphereFailureDomain_To_v1alpha4_VSphereFailureDomain(in *v func autoConvert_v1alpha4_VSphereFailureDomainList_To_v1beta1_VSphereFailureDomainList(in *VSphereFailureDomainList, out *v1beta1.VSphereFailureDomainList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v1beta1.VSphereFailureDomain)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v1beta1.VSphereFailureDomain, len(*in)) + for i := range *in { + if err := Convert_v1alpha4_VSphereFailureDomain_To_v1beta1_VSphereFailureDomain(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1384,7 +1390,17 @@ func Convert_v1alpha4_VSphereFailureDomainList_To_v1beta1_VSphereFailureDomainLi func autoConvert_v1beta1_VSphereFailureDomainList_To_v1alpha4_VSphereFailureDomainList(in *v1beta1.VSphereFailureDomainList, out *VSphereFailureDomainList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]VSphereFailureDomain)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]VSphereFailureDomain, len(*in)) + for i := range *in { + if err := Convert_v1beta1_VSphereFailureDomain_To_v1alpha4_VSphereFailureDomain(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } diff --git a/apis/v1beta1/vspherefailuredomain_types.go b/apis/v1beta1/vspherefailuredomain_types.go index bcb139a97f..a5326a0b2b 100644 --- a/apis/v1beta1/vspherefailuredomain_types.go +++ b/apis/v1beta1/vspherefailuredomain_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta1 import ( + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -81,12 +82,69 @@ type Topology struct { // +optional Networks []string `json:"networks,omitempty"` + // NetworkConfigurations is a list of network configurations within this failure domain. + // +optional + // +listType=map + // +listMapKey=name + NetworkConfigurations []NetworkConfiguration `json:"networkConfigs,omitempty"` + // Datastore is the name or inventory path of the datastore in which the // virtual machine is created/located. // +optional Datastore string `json:"datastore,omitempty"` } +// NetworkConfiguration defines a network configuration that should be used when consuming +// a failure domain. +type NetworkConfiguration struct { + // NetworkName is the network name for this machine's VM. + // +kubebuilder:validation:Required + NetworkName string `json:"name"` + + // DHCP4 is a flag that indicates whether or not to use DHCP for IPv4. + // +optional + DHCP4 *bool `json:"dhcp4,omitempty"` + + // DHCP6 is a flag that indicates whether or not to use DHCP for IPv6. + // +optional + DHCP6 *bool `json:"dhcp6,omitempty"` + + // Nameservers is a list of IPv4 and/or IPv6 addresses used as DNS + // nameservers. + // Please note that Linux allows only three nameservers (https://linux.die.net/man/5/resolv.conf). + // +optional + Nameservers []string `json:"nameservers,omitempty"` + + // SearchDomains is a list of search domains used when resolving IP + // addresses with DNS. + // +optional + SearchDomains []string `json:"searchDomains,omitempty"` + + // DHCP4Overrides allows for the control over several DHCP behaviors. + // Overrides will only be applied when the corresponding DHCP flag is set. + // Only configured values will be sent, omitted values will default to + // distribution defaults. + // Dependent on support in the network stack for your distribution. + // For more information see the netplan reference (https://netplan.io/reference#dhcp-overrides) + // +optional + DHCP4Overrides *DHCPOverrides `json:"dhcp4Overrides,omitempty"` + + // DHCP6Overrides allows for the control over several DHCP behaviors. + // Overrides will only be applied when the corresponding DHCP flag is set. + // Only configured values will be sent, omitted values will default to + // distribution defaults. + // Dependent on support in the network stack for your distribution. + // For more information see the netplan reference (https://netplan.io/reference#dhcp-overrides) + // +optional + DHCP6Overrides *DHCPOverrides `json:"dhcp6Overrides,omitempty"` + + // AddressesFromPools is a list of IPAddressPools that should be assigned + // to IPAddressClaims. The machine's cloud-init metadata will be populated + // with IPAddresses fulfilled by an IPAM provider. + // +optional + AddressesFromPools []corev1.TypedLocalObjectReference `json:"addressesFromPools,omitempty"` +} + // FailureDomainHosts has information required for placement of machines on VSphere hosts. type FailureDomainHosts struct { // VMGroupName is the name of the VM group diff --git a/apis/v1beta1/zz_generated.deepcopy.go b/apis/v1beta1/zz_generated.deepcopy.go index 44d12a65fe..6398eb52e7 100644 --- a/apis/v1beta1/zz_generated.deepcopy.go +++ b/apis/v1beta1/zz_generated.deepcopy.go @@ -194,6 +194,58 @@ func (in *Network) DeepCopy() *Network { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NetworkConfiguration) DeepCopyInto(out *NetworkConfiguration) { + *out = *in + if in.DHCP4 != nil { + in, out := &in.DHCP4, &out.DHCP4 + *out = new(bool) + **out = **in + } + if in.DHCP6 != nil { + in, out := &in.DHCP6, &out.DHCP6 + *out = new(bool) + **out = **in + } + if in.Nameservers != nil { + in, out := &in.Nameservers, &out.Nameservers + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.SearchDomains != nil { + in, out := &in.SearchDomains, &out.SearchDomains + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.DHCP4Overrides != nil { + in, out := &in.DHCP4Overrides, &out.DHCP4Overrides + *out = new(DHCPOverrides) + (*in).DeepCopyInto(*out) + } + if in.DHCP6Overrides != nil { + in, out := &in.DHCP6Overrides, &out.DHCP6Overrides + *out = new(DHCPOverrides) + (*in).DeepCopyInto(*out) + } + if in.AddressesFromPools != nil { + in, out := &in.AddressesFromPools, &out.AddressesFromPools + *out = make([]v1.TypedLocalObjectReference, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkConfiguration. +func (in *NetworkConfiguration) DeepCopy() *NetworkConfiguration { + if in == nil { + return nil + } + out := new(NetworkConfiguration) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkDeviceSpec) DeepCopyInto(out *NetworkDeviceSpec) { *out = *in @@ -391,6 +443,13 @@ func (in *Topology) DeepCopyInto(out *Topology) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.NetworkConfigurations != nil { + in, out := &in.NetworkConfigurations, &out.NetworkConfigurations + *out = make([]NetworkConfiguration, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Topology. diff --git a/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml b/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml index aecf61e6b7..160dfb6a20 100644 --- a/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml +++ b/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml @@ -365,6 +365,190 @@ spec: - hostGroupName - vmGroupName type: object + networkConfigs: + description: NetworkConfigurations is a list of network configurations + within this failure domain. + items: + description: |- + NetworkConfiguration defines a network configuration that should be used when consuming + a failure domain. + properties: + addressesFromPools: + description: |- + AddressesFromPools is a list of IPAddressPools that should be assigned + to IPAddressClaims. The machine's cloud-init metadata will be populated + with IPAddresses fulfilled by an IPAM provider. + items: + description: |- + TypedLocalObjectReference contains enough information to let you locate the + typed referenced object inside the same namespace. + properties: + apiGroup: + description: |- + APIGroup is the group for the resource being referenced. + If APIGroup is not specified, the specified Kind must be in the core API group. + For any other third-party types, APIGroup is required. + type: string + kind: + description: Kind is the type of resource being referenced + type: string + name: + description: Name is the name of resource being referenced + type: string + required: + - kind + - name + type: object + x-kubernetes-map-type: atomic + type: array + dhcp4: + description: DHCP4 is a flag that indicates whether or not + to use DHCP for IPv4. + type: boolean + dhcp4Overrides: + description: |- + DHCP4Overrides allows for the control over several DHCP behaviors. + Overrides will only be applied when the corresponding DHCP flag is set. + Only configured values will be sent, omitted values will default to + distribution defaults. + Dependent on support in the network stack for your distribution. + For more information see the netplan reference (https://netplan.io/reference#dhcp-overrides) + properties: + hostname: + description: |- + Hostname is the name which will be sent to the DHCP server instead of + the machine's hostname. + type: string + routeMetric: + description: |- + RouteMetric is used to prioritize routes for devices. A lower metric for + an interface will have a higher priority. + type: integer + sendHostname: + description: |- + SendHostname when `true`, the hostname of the machine will be sent to the + DHCP server. + type: boolean + useDNS: + description: |- + UseDNS when `true`, the DNS servers in the DHCP server will be used and + take precedence. + type: boolean + useDomains: + description: |- + UseDomains can take the values `true`, `false`, or `route`. When `true`, + the domain name from the DHCP server will be used as the DNS search + domain for this device. When `route`, the domain name from the DHCP + response will be used for routing DNS only, not for searching. + type: string + useHostname: + description: |- + UseHostname when `true`, the hostname from the DHCP server will be set + as the transient hostname of the machine. + type: boolean + useMTU: + description: |- + UseMTU when `true`, the MTU from the DHCP server will be set as the + MTU of the device. + type: boolean + useNTP: + description: |- + UseNTP when `true`, the NTP servers from the DHCP server will be used + by systemd-timesyncd and take precedence. + type: boolean + useRoutes: + description: |- + UseRoutes when `true`, the routes from the DHCP server will be installed + in the routing table. + type: string + type: object + dhcp6: + description: DHCP6 is a flag that indicates whether or not + to use DHCP for IPv6. + type: boolean + dhcp6Overrides: + description: |- + DHCP6Overrides allows for the control over several DHCP behaviors. + Overrides will only be applied when the corresponding DHCP flag is set. + Only configured values will be sent, omitted values will default to + distribution defaults. + Dependent on support in the network stack for your distribution. + For more information see the netplan reference (https://netplan.io/reference#dhcp-overrides) + properties: + hostname: + description: |- + Hostname is the name which will be sent to the DHCP server instead of + the machine's hostname. + type: string + routeMetric: + description: |- + RouteMetric is used to prioritize routes for devices. A lower metric for + an interface will have a higher priority. + type: integer + sendHostname: + description: |- + SendHostname when `true`, the hostname of the machine will be sent to the + DHCP server. + type: boolean + useDNS: + description: |- + UseDNS when `true`, the DNS servers in the DHCP server will be used and + take precedence. + type: boolean + useDomains: + description: |- + UseDomains can take the values `true`, `false`, or `route`. When `true`, + the domain name from the DHCP server will be used as the DNS search + domain for this device. When `route`, the domain name from the DHCP + response will be used for routing DNS only, not for searching. + type: string + useHostname: + description: |- + UseHostname when `true`, the hostname from the DHCP server will be set + as the transient hostname of the machine. + type: boolean + useMTU: + description: |- + UseMTU when `true`, the MTU from the DHCP server will be set as the + MTU of the device. + type: boolean + useNTP: + description: |- + UseNTP when `true`, the NTP servers from the DHCP server will be used + by systemd-timesyncd and take precedence. + type: boolean + useRoutes: + description: |- + UseRoutes when `true`, the routes from the DHCP server will be installed + in the routing table. + type: string + type: object + name: + description: NetworkName is the network name for this machine's + VM. + type: string + nameservers: + description: |- + Nameservers is a list of IPv4 and/or IPv6 addresses used as DNS + nameservers. + Please note that Linux allows only three nameservers (https://linux.die.net/man/5/resolv.conf). + items: + type: string + type: array + searchDomains: + description: |- + SearchDomains is a list of search domains used when resolving IP + addresses with DNS. + items: + type: string + type: array + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map networks: description: Networks is the list of networks within this failure domain diff --git a/controllers/vspheredeploymentzone_controller_domain.go b/controllers/vspheredeploymentzone_controller_domain.go index b5b23ee830..32052bf12f 100644 --- a/controllers/vspheredeploymentzone_controller_domain.go +++ b/controllers/vspheredeploymentzone_controller_domain.go @@ -95,11 +95,18 @@ func (r vsphereDeploymentZoneReconciler) reconcileTopology(ctx context.Context, for _, network := range topology.Networks { if _, err := deploymentZoneCtx.AuthSession.Finder.Network(ctx, network); err != nil { - conditions.MarkFalse(deploymentZoneCtx.VSphereDeploymentZone, infrav1.VSphereFailureDomainValidatedCondition, infrav1.NetworkNotFoundReason, clusterv1.ConditionSeverityError, "network %s is misconfigured", network) + conditions.MarkFalse(deploymentZoneCtx.VSphereDeploymentZone, infrav1.VSphereFailureDomainValidatedCondition, infrav1.NetworkNotFoundReason, clusterv1.ConditionSeverityError, "network %s is not found", network) return errors.Wrapf(err, "unable to find network %s", network) } } + for _, networkConfig := range topology.NetworkConfigurations { + if _, err := deploymentZoneCtx.AuthSession.Finder.Network(ctx, networkConfig.NetworkName); err != nil { + conditions.MarkFalse(deploymentZoneCtx.VSphereDeploymentZone, infrav1.VSphereFailureDomainValidatedCondition, infrav1.NetworkNotFoundReason, clusterv1.ConditionSeverityError, "network %s is not found", networkConfig.NetworkName) + return errors.Wrapf(err, "unable to find network %s", networkConfig.NetworkName) + } + } + if hostPlacementInfo := topology.Hosts; hostPlacementInfo != nil { rule, err := cluster.VerifyAffinityRule(ctx, deploymentZoneCtx, *topology.ComputeCluster, hostPlacementInfo.HostGroupName, hostPlacementInfo.VMGroupName) switch { diff --git a/internal/webhooks/vspherefailuredomain.go b/internal/webhooks/vspherefailuredomain.go index 53c0a23886..7c466f8ba1 100644 --- a/internal/webhooks/vspherefailuredomain.go +++ b/internal/webhooks/vspherefailuredomain.go @@ -77,6 +77,16 @@ func (webhook *VSphereFailureDomainWebhook) ValidateCreate(_ context.Context, ra allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "Topology", "ComputeCluster"), fmt.Sprintf("cannot be nil if zone's Failure Domain type is %s", obj.Spec.Zone.Type))) } + if len(obj.Spec.Topology.NetworkConfigurations) != 0 && len(obj.Spec.Topology.Networks) != 0 { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "Topology", "Networks"), "cannot be set if spec.Topology.NetworkConfigs is already set")) + } + + for i, networkConfig := range obj.Spec.Topology.NetworkConfigurations { + if networkConfig.NetworkName == "" { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "Topology", "NetworkConfigurations", fmt.Sprint(i), "name"), "cannot be empty")) + } + } + return nil, AggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs) } diff --git a/pkg/services/vimmachine.go b/pkg/services/vimmachine.go index abda5dc598..44f6c371ca 100644 --- a/pkg/services/vimmachine.go +++ b/pkg/services/vimmachine.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/types" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/klog/v2" + "k8s.io/utils/integer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" clusterutilv1 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" @@ -436,7 +437,10 @@ func (v *VimMachineService) generateOverrideFunc(ctx context.Context, vimMachine vm.Spec.Datastore = vsphereFailureDomain.Spec.Topology.Datastore } if len(vsphereFailureDomain.Spec.Topology.Networks) > 0 { - vm.Spec.Network.Devices = overrideNetworkDeviceSpecs(vm.Spec.Network.Devices, vsphereFailureDomain.Spec.Topology.Networks) + vm.Spec.Network.Devices = overrideNetworkDeviceSpecs(vm.Spec.Network.Devices, vsphereFailureDomain.Spec.Topology.Networks, mergeFailureDomainNetworkName) + } + if len(vsphereFailureDomain.Spec.Topology.NetworkConfigurations) > 0 { + vm.Spec.Network.Devices = overrideNetworkDeviceSpecs(vm.Spec.Network.Devices, vsphereFailureDomain.Spec.Topology.NetworkConfigurations, mergeNetworkConfigurationInNetworkDeviceSpec) } } return overrideWithFailureDomainFunc, true @@ -446,25 +450,66 @@ func (v *VimMachineService) generateOverrideFunc(ctx context.Context, vimMachine // The substitution is done based on the order in which the network devices have been defined. // // In case there are more network definitions than the number of network devices specified, the definitions are appended to the list. -func overrideNetworkDeviceSpecs(deviceSpecs []infrav1.NetworkDeviceSpec, networks []string) []infrav1.NetworkDeviceSpec { +func overrideNetworkDeviceSpecs[T any](deviceSpecs []infrav1.NetworkDeviceSpec, networks []T, mergeFunc func(device *infrav1.NetworkDeviceSpec, network T)) []infrav1.NetworkDeviceSpec { index, length := 0, len(networks) - devices := make([]infrav1.NetworkDeviceSpec, 0, max(length, len(deviceSpecs))) + devices := make([]infrav1.NetworkDeviceSpec, 0, integer.IntMax(length, len(deviceSpecs))) // override the networks on the VM spec with placement constraint network definitions for i := range deviceSpecs { vmNetworkDeviceSpec := deviceSpecs[i] if i < length { index++ - vmNetworkDeviceSpec.NetworkName = networks[i] + mergeFunc(&vmNetworkDeviceSpec, networks[i]) } devices = append(devices, vmNetworkDeviceSpec) } // append the remaining network definitions to the VM spec for ; index < length; index++ { - devices = append(devices, infrav1.NetworkDeviceSpec{ - NetworkName: networks[index], - }) + device := infrav1.NetworkDeviceSpec{} + mergeFunc(&device, networks[index]) + devices = append(devices, device) } return devices } + +func mergeFailureDomainNetworkName(device *infrav1.NetworkDeviceSpec, network string) { + device.NetworkName = network +} + +func mergeNetworkConfigurationInNetworkDeviceSpec(device *infrav1.NetworkDeviceSpec, nc infrav1.NetworkConfiguration) { + if device == nil { + return + } + if nc.NetworkName != "" { + device.NetworkName = nc.NetworkName + } + if nc.DHCP4 != nil { + device.DHCP4 = *nc.DHCP4 + } + if nc.DHCP6 != nil { + device.DHCP6 = *nc.DHCP6 + } + if len(nc.Nameservers) > 0 { + device.Nameservers = make([]string, len(nc.Nameservers)) + copy(device.Nameservers, nc.Nameservers) + } + + if len(nc.SearchDomains) > 0 { + device.SearchDomains = make([]string, len(nc.SearchDomains)) + copy(device.SearchDomains, nc.SearchDomains) + } + + if nc.DHCP4Overrides != nil { + device.DHCP4Overrides = nc.DHCP4Overrides.DeepCopy() + } + + if nc.DHCP6Overrides != nil { + device.DHCP6Overrides = nc.DHCP6Overrides.DeepCopy() + } + + if len(nc.AddressesFromPools) > 0 { + device.AddressesFromPools = make([]corev1.TypedLocalObjectReference, len(nc.AddressesFromPools)) + copy(device.AddressesFromPools, nc.AddressesFromPools) + } +} diff --git a/pkg/services/vimmachine_test.go b/pkg/services/vimmachine_test.go index 27dab08545..7c7624f25e 100644 --- a/pkg/services/vimmachine_test.go +++ b/pkg/services/vimmachine_test.go @@ -65,6 +65,29 @@ func Test_VimMachineService_GenerateOverrideFunc(t *testing.T) { } } + failureDomainWithNetworkConfigs := func(suffix string) *infrav1.VSphereFailureDomain { + return &infrav1.VSphereFailureDomain{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("fd-%s", suffix)}, + Spec: infrav1.VSphereFailureDomainSpec{ + Topology: infrav1.Topology{ + Datacenter: fmt.Sprintf("dc-%s", suffix), + Datastore: fmt.Sprintf("ds-%s", suffix), + Networks: []string{fmt.Sprintf("nw-%s", suffix), "another-nw"}, + NetworkConfigurations: []infrav1.NetworkConfiguration{ + { + NetworkName: fmt.Sprintf("nw-%s", suffix), + DHCP4: ptr.To(true), + }, + { + NetworkName: "another-nw", + DHCP6: ptr.To(true), + }, + }, + }, + }, + } + } + t.Run("does not generate an override function when Failure Domain is not present", func(t *testing.T) { g := NewWithT(t) controllerManagerContext := fake.NewControllerManagerContext(deplZone("one"), deplZone("two"), failureDomain("one"), failureDomain("two")) @@ -145,6 +168,35 @@ func Test_VimMachineService_GenerateOverrideFunc(t *testing.T) { g.Expect(devices[1].NetworkName).To(Equal("another-nw")) }) + t.Run("overrides the n/w config from the network config list of the topology for equal number of networks", func(t *testing.T) { + g := NewWithT(t) + controllerManagerContext := fake.NewControllerManagerContext(deplZone("one"), deplZone("two"), failureDomainWithNetworkConfigs("one"), failureDomainWithNetworkConfigs("two")) + machineCtx := fake.NewMachineContext(ctx, fake.NewClusterContext(ctx, controllerManagerContext), controllerManagerContext) + machineCtx.Machine.Spec.FailureDomain = ptr.To("zone-one") + vimMachineService := &VimMachineService{controllerManagerContext.Client} + + vm := &infrav1.VSphereVM{ + Spec: infrav1.VSphereVMSpec{ + VirtualMachineCloneSpec: infrav1.VirtualMachineCloneSpec{ + Network: infrav1.NetworkSpec{Devices: []infrav1.NetworkDeviceSpec{{NetworkName: "foo", DHCP4: false}, {NetworkName: "bar", DHCP6: false}}}, + }, + }, + } + + overrideFunc, ok := vimMachineService.generateOverrideFunc(ctx, machineCtx) + g.Expect(ok).To(BeTrue()) + + overrideFunc(vm) + + devices := vm.Spec.Network.Devices + g.Expect(devices).To(HaveLen(2)) + g.Expect(devices[0].NetworkName).To(Equal("nw-one")) + g.Expect(devices[0].DHCP4).To(BeTrue()) + + g.Expect(devices[1].NetworkName).To(Equal("another-nw")) + g.Expect(devices[1].DHCP6).To(BeTrue()) + }) + t.Run("appends the n/w names present in the networks list of the topology with number of devices in VMSpec < number of networks in the placement constraint", func(t *testing.T) { g := NewWithT(t) controllerManagerContext := fake.NewControllerManagerContext(deplZone("one"), deplZone("two"), failureDomain("one"), failureDomain("two")) @@ -172,6 +224,35 @@ func Test_VimMachineService_GenerateOverrideFunc(t *testing.T) { g.Expect(devices[1].NetworkName).To(Equal("another-nw")) }) + t.Run("appends the n/w configs present in the network config list of the topology with number of devices in VMSpec < number of networks in the placement constraint", func(t *testing.T) { + g := NewWithT(t) + controllerManagerContext := fake.NewControllerManagerContext(deplZone("one"), deplZone("two"), failureDomainWithNetworkConfigs("one"), failureDomainWithNetworkConfigs("two")) + machineCtx := fake.NewMachineContext(ctx, fake.NewClusterContext(ctx, controllerManagerContext), controllerManagerContext) + machineCtx.Machine.Spec.FailureDomain = ptr.To("zone-one") + vimMachineService := &VimMachineService{controllerManagerContext.Client} + + vm := &infrav1.VSphereVM{ + Spec: infrav1.VSphereVMSpec{ + VirtualMachineCloneSpec: infrav1.VirtualMachineCloneSpec{ + Network: infrav1.NetworkSpec{Devices: []infrav1.NetworkDeviceSpec{{NetworkName: "foo", DHCP4: false}}}, + }, + }, + } + + overrideFunc, ok := vimMachineService.generateOverrideFunc(ctx, machineCtx) + g.Expect(ok).To(BeTrue()) + + overrideFunc(vm) + + devices := vm.Spec.Network.Devices + g.Expect(devices).To(HaveLen(2)) + g.Expect(devices[0].NetworkName).To(Equal("nw-one")) + g.Expect(devices[0].DHCP4).To(BeTrue()) + + g.Expect(devices[1].NetworkName).To(Equal("another-nw")) + g.Expect(devices[1].DHCP6).To(BeTrue()) + }) + t.Run("only overrides the n/w names present in the networks list of the topology with number of devices in VMSpec > number of networks in the placement constraint", func(t *testing.T) { g := NewWithT(t) controllerManagerContext := fake.NewControllerManagerContext(deplZone("one"), deplZone("two"), failureDomain("one"), failureDomain("two")) @@ -200,6 +281,103 @@ func Test_VimMachineService_GenerateOverrideFunc(t *testing.T) { g.Expect(devices[2].NetworkName).To(Equal("baz")) }) + + t.Run("only overrides the n/w configs present in the network config list of the topology with number of devices in VMSpec > number of networks in the placement constraint", func(t *testing.T) { + g := NewWithT(t) + controllerManagerContext := fake.NewControllerManagerContext(deplZone("one"), deplZone("two"), failureDomainWithNetworkConfigs("one"), failureDomainWithNetworkConfigs("two")) + machineCtx := fake.NewMachineContext(ctx, fake.NewClusterContext(ctx, controllerManagerContext), controllerManagerContext) + machineCtx.Machine.Spec.FailureDomain = ptr.To("zone-one") + vimMachineService := &VimMachineService{controllerManagerContext.Client} + + vm := &infrav1.VSphereVM{ + Spec: infrav1.VSphereVMSpec{ + VirtualMachineCloneSpec: infrav1.VirtualMachineCloneSpec{ + Network: infrav1.NetworkSpec{Devices: []infrav1.NetworkDeviceSpec{{NetworkName: "foo", DHCP4: false}, {NetworkName: "bar", DHCP6: false}, {NetworkName: "baz", DHCP6: false}}}, + }, + }, + } + + overrideFunc, ok := vimMachineService.generateOverrideFunc(ctx, machineCtx) + g.Expect(ok).To(BeTrue()) + + overrideFunc(vm) + + devices := vm.Spec.Network.Devices + g.Expect(devices).To(HaveLen(3)) + g.Expect(devices[0].NetworkName).To(Equal("nw-one")) + g.Expect(devices[0].DHCP4).To(BeTrue()) + + g.Expect(devices[1].NetworkName).To(Equal("another-nw")) + g.Expect(devices[1].DHCP6).To(BeTrue()) + + g.Expect(devices[2].NetworkName).To(Equal("baz")) + g.Expect(devices[2].DHCP6).To(BeFalse()) + }) +} + +func Test_mergeNetworkConfigurationToNetworkDeviceSpec(t *testing.T) { + t.Run("device is nil", func(t *testing.T) { + g := NewWithT(t) + + var device *infrav1.NetworkDeviceSpec + mergeNetworkConfigurationInNetworkDeviceSpec(device, infrav1.NetworkConfiguration{ + NetworkName: "ignored", + }) + + g.Expect(device).To(BeNil()) + }) + + t.Run("all fields from NetworkConfiguration are overrided", func(t *testing.T) { + g := NewWithT(t) + + device := infrav1.NetworkDeviceSpec{} + + mergeNetworkConfigurationInNetworkDeviceSpec(&device, infrav1.NetworkConfiguration{ + NetworkName: "nw-name", + DHCP4: ptr.To(true), + DHCP6: ptr.To(false), + Nameservers: []string{"1.1.1.1"}, + SearchDomains: []string{"vmware.ci"}, + DHCP4Overrides: &infrav1.DHCPOverrides{ + Hostname: ptr.To("hal"), + RouteMetric: ptr.To(12345), + }, + DHCP6Overrides: &infrav1.DHCPOverrides{ + Hostname: ptr.To("hal"), + RouteMetric: ptr.To(23456), + }, + AddressesFromPools: []corev1.TypedLocalObjectReference{ + { + APIGroup: ptr.To("api-group"), + Name: "my-pool-1", + Kind: "my-pool-kind", + }, + }, + }) + + g.Expect(device).To(Equal(infrav1.NetworkDeviceSpec{ + NetworkName: "nw-name", + DHCP4: true, + DHCP6: false, + Nameservers: []string{"1.1.1.1"}, + SearchDomains: []string{"vmware.ci"}, + DHCP4Overrides: &infrav1.DHCPOverrides{ + Hostname: ptr.To("hal"), + RouteMetric: ptr.To(12345), + }, + DHCP6Overrides: &infrav1.DHCPOverrides{ + Hostname: ptr.To("hal"), + RouteMetric: ptr.To(23456), + }, + AddressesFromPools: []corev1.TypedLocalObjectReference{ + { + APIGroup: ptr.To("api-group"), + Name: "my-pool-1", + Kind: "my-pool-kind", + }, + }, + })) + }) } func Test_VimMachineService_GetHostInfo(t *testing.T) { From 970ef581d42cb287d316d2ac8e91babbd2929a8b Mon Sep 17 00:00:00 2001 From: Efim Smykov Date: Wed, 23 Oct 2024 22:06:41 +0300 Subject: [PATCH 2/4] fix errs, conversions --- .../vspherefailuredomain_conversion.go | 28 +++++++++++++++++-- .../vspherefailuredomain_conversion.go | 28 +++++++++++++++++-- internal/webhooks/vspherefailuredomain.go | 4 +-- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/apis/v1alpha3/vspherefailuredomain_conversion.go b/apis/v1alpha3/vspherefailuredomain_conversion.go index 357490d221..9a0d262af9 100644 --- a/apis/v1alpha3/vspherefailuredomain_conversion.go +++ b/apis/v1alpha3/vspherefailuredomain_conversion.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha3 import ( + utilconversion "sigs.k8s.io/cluster-api/util/conversion" "sigs.k8s.io/controller-runtime/pkg/conversion" infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" @@ -25,13 +26,36 @@ import ( // ConvertTo converts this VSphereFailureDomain to the Hub version (v1beta1). func (src *VSphereFailureDomain) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*infrav1.VSphereFailureDomain) - return Convert_v1alpha3_VSphereFailureDomain_To_v1beta1_VSphereFailureDomain(src, dst, nil) + + if err := Convert_v1alpha3_VSphereFailureDomain_To_v1beta1_VSphereFailureDomain(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &infrav1.VSphereFailureDomain{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.Topology.NetworkConfigurations = restored.Spec.Topology.NetworkConfigurations + + return nil } // ConvertFrom converts from the Hub version (v1beta1) to this VSphereFailureDomain. func (dst *VSphereFailureDomain) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*infrav1.VSphereFailureDomain) - return Convert_v1beta1_VSphereFailureDomain_To_v1alpha3_VSphereFailureDomain(src, dst, nil) + + if err := Convert_v1beta1_VSphereFailureDomain_To_v1alpha3_VSphereFailureDomain(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion except for metadata + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } // ConvertTo converts this VSphereFailureDomainList to the Hub version (v1beta1). diff --git a/apis/v1alpha4/vspherefailuredomain_conversion.go b/apis/v1alpha4/vspherefailuredomain_conversion.go index 006033f971..1357cf8417 100644 --- a/apis/v1alpha4/vspherefailuredomain_conversion.go +++ b/apis/v1alpha4/vspherefailuredomain_conversion.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha4 import ( + utilconversion "sigs.k8s.io/cluster-api/util/conversion" "sigs.k8s.io/controller-runtime/pkg/conversion" infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" @@ -25,13 +26,36 @@ import ( // ConvertTo converts this VSphereFailureDomain to the Hub version (v1beta1). func (src *VSphereFailureDomain) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*infrav1.VSphereFailureDomain) - return Convert_v1alpha4_VSphereFailureDomain_To_v1beta1_VSphereFailureDomain(src, dst, nil) + + if err := Convert_v1alpha4_VSphereFailureDomain_To_v1beta1_VSphereFailureDomain(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &infrav1.VSphereFailureDomain{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.Topology.NetworkConfigurations = restored.Spec.Topology.NetworkConfigurations + + return nil } // ConvertFrom converts from the Hub version (v1beta1) to this VSphereFailureDomain. func (dst *VSphereFailureDomain) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*infrav1.VSphereFailureDomain) - return Convert_v1beta1_VSphereFailureDomain_To_v1alpha4_VSphereFailureDomain(src, dst, nil) + + if err := Convert_v1beta1_VSphereFailureDomain_To_v1alpha4_VSphereFailureDomain(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion except for metadata + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } // ConvertTo converts this VSphereFailureDomainList to the Hub version (v1beta1). diff --git a/internal/webhooks/vspherefailuredomain.go b/internal/webhooks/vspherefailuredomain.go index 7c466f8ba1..62dbea4dbd 100644 --- a/internal/webhooks/vspherefailuredomain.go +++ b/internal/webhooks/vspherefailuredomain.go @@ -78,12 +78,12 @@ func (webhook *VSphereFailureDomainWebhook) ValidateCreate(_ context.Context, ra } if len(obj.Spec.Topology.NetworkConfigurations) != 0 && len(obj.Spec.Topology.Networks) != 0 { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "Topology", "Networks"), "cannot be set if spec.Topology.NetworkConfigs is already set")) + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "topology", "networks"), "cannot be set if spec.topology.networkConfigs is already set")) } for i, networkConfig := range obj.Spec.Topology.NetworkConfigurations { if networkConfig.NetworkName == "" { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "Topology", "NetworkConfigurations", fmt.Sprint(i), "name"), "cannot be empty")) + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "topology", "networkConfigurations", fmt.Sprint(i), "name"), "cannot be empty")) } } From cc001dbe4a91ff0beb4cb883b659e58fe8a9ee44 Mon Sep 17 00:00:00 2001 From: Nutrymaco Date: Thu, 24 Oct 2024 22:47:16 +0300 Subject: [PATCH 3/4] add webhook test --- .../webhooks/vspherefailuredomain_test.go | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/internal/webhooks/vspherefailuredomain_test.go b/internal/webhooks/vspherefailuredomain_test.go index 5425d29f88..4d6e0d7875 100644 --- a/internal/webhooks/vspherefailuredomain_test.go +++ b/internal/webhooks/vspherefailuredomain_test.go @@ -132,6 +132,27 @@ func TestVSphereFailureDomain_ValidateCreate(t *testing.T) { }, }}, }, + { + name: "topology.networks and topology.networkConfigurations set simultaneously", + failureDomain: infrav1.VSphereFailureDomain{Spec: infrav1.VSphereFailureDomainSpec{ + Region: infrav1.FailureDomain{ + Name: "foo", + Type: infrav1.DatacenterFailureDomain, + TagCategory: "k8s-bar", + }, + Zone: infrav1.FailureDomain{ + Name: "foo", + Type: infrav1.ComputeClusterFailureDomain, + TagCategory: "k8s-bar", + }, + Topology: infrav1.Topology{ + Datacenter: "/blah", + ComputeCluster: ptr.To("blah2"), + Networks: []string{"nw-1"}, + NetworkConfigurations: []infrav1.NetworkConfiguration{{NetworkName: "nw-1"}}, + }, + }}, + }, } for _, tt := range tests { @@ -140,7 +161,7 @@ func TestVSphereFailureDomain_ValidateCreate(t *testing.T) { t.Run(tt.name, func(*testing.T) { webhook := &VSphereFailureDomainWebhook{} _, err := webhook.ValidateCreate(context.Background(), &tt.failureDomain) - if tt.errExpected == nil || !*tt.errExpected { + if tt.errExpected == nil || *tt.errExpected { g.Expect(err).To(HaveOccurred()) } else { g.Expect(err).NotTo(HaveOccurred()) From 4b8978c9a1db91f729e666fe1d405dea50cd3462 Mon Sep 17 00:00:00 2001 From: Nutrymaco Date: Wed, 13 Nov 2024 13:22:31 +0300 Subject: [PATCH 4/4] fix --- apis/v1beta1/vspherefailuredomain_types.go | 6 +++--- ...ure.cluster.x-k8s.io_vspherefailuredomains.yaml | 14 +++++++------- internal/webhooks/vspherefailuredomain.go | 4 ++-- pkg/services/vimmachine.go | 6 +----- pkg/services/vimmachine_test.go | 14 +------------- 5 files changed, 14 insertions(+), 30 deletions(-) diff --git a/apis/v1beta1/vspherefailuredomain_types.go b/apis/v1beta1/vspherefailuredomain_types.go index a5326a0b2b..bea116c7dd 100644 --- a/apis/v1beta1/vspherefailuredomain_types.go +++ b/apis/v1beta1/vspherefailuredomain_types.go @@ -85,8 +85,8 @@ type Topology struct { // NetworkConfigurations is a list of network configurations within this failure domain. // +optional // +listType=map - // +listMapKey=name - NetworkConfigurations []NetworkConfiguration `json:"networkConfigs,omitempty"` + // +listMapKey=networkName + NetworkConfigurations []NetworkConfiguration `json:"networkConfigurations,omitempty"` // Datastore is the name or inventory path of the datastore in which the // virtual machine is created/located. @@ -99,7 +99,7 @@ type Topology struct { type NetworkConfiguration struct { // NetworkName is the network name for this machine's VM. // +kubebuilder:validation:Required - NetworkName string `json:"name"` + NetworkName string `json:"networkName"` // DHCP4 is a flag that indicates whether or not to use DHCP for IPv4. // +optional diff --git a/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml b/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml index 160dfb6a20..3b81cd15e6 100644 --- a/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml +++ b/config/default/crd/bases/infrastructure.cluster.x-k8s.io_vspherefailuredomains.yaml @@ -365,7 +365,7 @@ spec: - hostGroupName - vmGroupName type: object - networkConfigs: + networkConfigurations: description: NetworkConfigurations is a list of network configurations within this failure domain. items: @@ -523,10 +523,6 @@ spec: in the routing table. type: string type: object - name: - description: NetworkName is the network name for this machine's - VM. - type: string nameservers: description: |- Nameservers is a list of IPv4 and/or IPv6 addresses used as DNS @@ -535,6 +531,10 @@ spec: items: type: string type: array + networkName: + description: NetworkName is the network name for this machine's + VM. + type: string searchDomains: description: |- SearchDomains is a list of search domains used when resolving IP @@ -543,11 +543,11 @@ spec: type: string type: array required: - - name + - networkName type: object type: array x-kubernetes-list-map-keys: - - name + - networkName x-kubernetes-list-type: map networks: description: Networks is the list of networks within this failure diff --git a/internal/webhooks/vspherefailuredomain.go b/internal/webhooks/vspherefailuredomain.go index 62dbea4dbd..6cc41add8b 100644 --- a/internal/webhooks/vspherefailuredomain.go +++ b/internal/webhooks/vspherefailuredomain.go @@ -78,12 +78,12 @@ func (webhook *VSphereFailureDomainWebhook) ValidateCreate(_ context.Context, ra } if len(obj.Spec.Topology.NetworkConfigurations) != 0 && len(obj.Spec.Topology.Networks) != 0 { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "topology", "networks"), "cannot be set if spec.topology.networkConfigs is already set")) + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "topology", "networks"), "cannot be set if spec.topology.networkConfigurations is already set")) } for i, networkConfig := range obj.Spec.Topology.NetworkConfigurations { if networkConfig.NetworkName == "" { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "topology", "networkConfigurations", fmt.Sprint(i), "name"), "cannot be empty")) + allErrs = append(allErrs, field.Required(field.NewPath("spec", "topology", "networkConfigurations").Index(i).Child("networkName"), "cannot be empty")) } } diff --git a/pkg/services/vimmachine.go b/pkg/services/vimmachine.go index 44f6c371ca..04268eaf6d 100644 --- a/pkg/services/vimmachine.go +++ b/pkg/services/vimmachine.go @@ -29,7 +29,6 @@ import ( "k8s.io/apimachinery/pkg/types" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/klog/v2" - "k8s.io/utils/integer" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" clusterutilv1 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" @@ -453,7 +452,7 @@ func (v *VimMachineService) generateOverrideFunc(ctx context.Context, vimMachine func overrideNetworkDeviceSpecs[T any](deviceSpecs []infrav1.NetworkDeviceSpec, networks []T, mergeFunc func(device *infrav1.NetworkDeviceSpec, network T)) []infrav1.NetworkDeviceSpec { index, length := 0, len(networks) - devices := make([]infrav1.NetworkDeviceSpec, 0, integer.IntMax(length, len(deviceSpecs))) + devices := make([]infrav1.NetworkDeviceSpec, 0, max(length, len(deviceSpecs))) // override the networks on the VM spec with placement constraint network definitions for i := range deviceSpecs { vmNetworkDeviceSpec := deviceSpecs[i] @@ -478,9 +477,6 @@ func mergeFailureDomainNetworkName(device *infrav1.NetworkDeviceSpec, network st } func mergeNetworkConfigurationInNetworkDeviceSpec(device *infrav1.NetworkDeviceSpec, nc infrav1.NetworkConfiguration) { - if device == nil { - return - } if nc.NetworkName != "" { device.NetworkName = nc.NetworkName } diff --git a/pkg/services/vimmachine_test.go b/pkg/services/vimmachine_test.go index 7c7624f25e..c3961bb822 100644 --- a/pkg/services/vimmachine_test.go +++ b/pkg/services/vimmachine_test.go @@ -72,7 +72,6 @@ func Test_VimMachineService_GenerateOverrideFunc(t *testing.T) { Topology: infrav1.Topology{ Datacenter: fmt.Sprintf("dc-%s", suffix), Datastore: fmt.Sprintf("ds-%s", suffix), - Networks: []string{fmt.Sprintf("nw-%s", suffix), "another-nw"}, NetworkConfigurations: []infrav1.NetworkConfiguration{ { NetworkName: fmt.Sprintf("nw-%s", suffix), @@ -316,18 +315,7 @@ func Test_VimMachineService_GenerateOverrideFunc(t *testing.T) { } func Test_mergeNetworkConfigurationToNetworkDeviceSpec(t *testing.T) { - t.Run("device is nil", func(t *testing.T) { - g := NewWithT(t) - - var device *infrav1.NetworkDeviceSpec - mergeNetworkConfigurationInNetworkDeviceSpec(device, infrav1.NetworkConfiguration{ - NetworkName: "ignored", - }) - - g.Expect(device).To(BeNil()) - }) - - t.Run("all fields from NetworkConfiguration are overrided", func(t *testing.T) { + t.Run("all fields from NetworkConfiguration are overridden", func(t *testing.T) { g := NewWithT(t) device := infrav1.NetworkDeviceSpec{}