diff --git a/apis/vmware/v1beta1/vspheremachine_types.go b/apis/vmware/v1beta1/vspheremachine_types.go index b35b8b6226..5f5cd27137 100644 --- a/apis/vmware/v1beta1/vspheremachine_types.go +++ b/apis/vmware/v1beta1/vspheremachine_types.go @@ -87,6 +87,30 @@ type VSphereMachineSpec struct { // // +optional MinHardwareVersion string `json:"minHardwareVersion,omitempty"` + + // NamingStrategy allows configuring the naming strategy used when calculating the name of the VirtualMachine. + // +optional + NamingStrategy *VirtualMachineNamingStrategy `json:"namingStrategy,omitempty"` +} + +// VirtualMachineNamingStrategy defines the naming strategy for the VirtualMachines. +type VirtualMachineNamingStrategy struct { + // Template defines the template to use for generating the name of the VirtualMachine object. + // If not defined, it will fall back to `{{ .machine.name }}`. + // The templating has the following data available: + // * `.machine.name`: The name of the Machine object. + // The templating also has the following funcs available: + // * `trimSuffix`: same as strings.TrimSuffix + // * `trunc`: truncates a string, e.g. `trunc 2 "hello"` or `trunc -2 "hello"` + // Notes: + // * While the template offers some flexibility, we would like the name to link to the Machine name + // to ensure better user experience when troubleshooting + // * Generated names must be valid Kubernetes names as they are used to create a VirtualMachine object + // and usually also as the name of the Node object. + // * Names are automatically truncated at 63 characters. Please note that this can lead to name conflicts, + // so we highly recommend to use a template which leads to a name shorter than 63 characters. + // +optional + Template *string `json:"template,omitempty"` } // VSphereMachineStatus defines the observed state of VSphereMachine. diff --git a/apis/vmware/v1beta1/zz_generated.deepcopy.go b/apis/vmware/v1beta1/zz_generated.deepcopy.go index 68f72ea580..565bcc8b36 100644 --- a/apis/vmware/v1beta1/zz_generated.deepcopy.go +++ b/apis/vmware/v1beta1/zz_generated.deepcopy.go @@ -401,6 +401,11 @@ func (in *VSphereMachineSpec) DeepCopyInto(out *VSphereMachineSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.NamingStrategy != nil { + in, out := &in.NamingStrategy, &out.NamingStrategy + *out = new(VirtualMachineNamingStrategy) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VSphereMachineSpec. @@ -566,3 +571,23 @@ func (in *VSphereMachineVolume) DeepCopy() *VSphereMachineVolume { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *VirtualMachineNamingStrategy) DeepCopyInto(out *VirtualMachineNamingStrategy) { + *out = *in + if in.Template != nil { + in, out := &in.Template, &out.Template + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VirtualMachineNamingStrategy. +func (in *VirtualMachineNamingStrategy) DeepCopy() *VirtualMachineNamingStrategy { + if in == nil { + return nil + } + out := new(VirtualMachineNamingStrategy) + in.DeepCopyInto(out) + return out +} diff --git a/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml index 87ae67d5cf..fdc2b81ee7 100644 --- a/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml +++ b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml @@ -76,6 +76,28 @@ spec: of the VM is at least set to the specified value. The expected format of the field is vmx-15. type: string + namingStrategy: + description: NamingStrategy allows configuring the naming strategy + used when calculating the name of the VirtualMachine. + properties: + template: + description: |- + Template defines the template to use for generating the name of the VirtualMachine object. + If not defined, it will fall back to `{{ .machine.name }}`. + The templating has the following data available: + * `.machine.name`: The name of the Machine object. + The templating also has the following funcs available: + * `trimSuffix`: same as strings.TrimSuffix + * `trunc`: truncates a string, e.g. `trunc 2 "hello"` or `trunc -2 "hello"` + Notes: + * While the template offers some flexibility, we would like the name to link to the Machine name + to ensure better user experience when troubleshooting + * Generated names must be valid Kubernetes names as they are used to create a VirtualMachine object + and usually also as the name of the Node object. + * Names are automatically truncated at 63 characters. Please note that this can lead to name conflicts, + so we highly recommend to use a template which leads to a name shorter than 63 characters. + type: string + type: object powerOffMode: default: hard description: |- diff --git a/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml index a63bcd3716..b11c86f071 100644 --- a/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml +++ b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml @@ -72,6 +72,28 @@ spec: of the VM is at least set to the specified value. The expected format of the field is vmx-15. type: string + namingStrategy: + description: NamingStrategy allows configuring the naming + strategy used when calculating the name of the VirtualMachine. + properties: + template: + description: |- + Template defines the template to use for generating the name of the VirtualMachine object. + If not defined, it will fall back to `{{ .machine.name }}`. + The templating has the following data available: + * `.machine.name`: The name of the Machine object. + The templating also has the following funcs available: + * `trimSuffix`: same as strings.TrimSuffix + * `trunc`: truncates a string, e.g. `trunc 2 "hello"` or `trunc -2 "hello"` + Notes: + * While the template offers some flexibility, we would like the name to link to the Machine name + to ensure better user experience when troubleshooting + * Generated names must be valid Kubernetes names as they are used to create a VirtualMachine object + and usually also as the name of the Node object. + * Names are automatically truncated at 63 characters. Please note that this can lead to name conflicts, + so we highly recommend to use a template which leads to a name shorter than 63 characters. + type: string + type: object powerOffMode: default: hard description: |- diff --git a/config/supervisor/webhook/manifests.yaml b/config/supervisor/webhook/manifests.yaml index c14c8c4426..f241f0788a 100644 --- a/config/supervisor/webhook/manifests.yaml +++ b/config/supervisor/webhook/manifests.yaml @@ -52,3 +52,24 @@ webhooks: resources: - vspheremachines sideEffects: None +- admissionReviewVersions: + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachinetemplate + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.vspheremachinetemplate.vmware.infrastructure.cluster.x-k8s.io + rules: + - apiGroups: + - vmware.infrastructure.cluster.x-k8s.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - vspheremachinetemplates + sideEffects: None diff --git a/controllers/vmware/test/controllers_test.go b/controllers/vmware/test/controllers_test.go index da573fcda4..44e3298156 100644 --- a/controllers/vmware/test/controllers_test.go +++ b/controllers/vmware/test/controllers_test.go @@ -252,6 +252,9 @@ func getManager(cfg *rest.Config, networkProvider string, withWebhooks bool) man } if withWebhooks { + if err := (&vmwarewebhooks.VSphereMachineTemplateWebhook{}).SetupWebhookWithManager(mgr); err != nil { + return err + } if err := (&vmwarewebhooks.VSphereMachineWebhook{}).SetupWebhookWithManager(mgr); err != nil { return err } diff --git a/internal/webhooks/vmware/vspheremachine_test.go b/internal/webhooks/vmware/vspheremachine_test.go index acde154723..aa482b64c3 100644 --- a/internal/webhooks/vmware/vspheremachine_test.go +++ b/internal/webhooks/vmware/vspheremachine_test.go @@ -26,8 +26,6 @@ import ( ) func TestVSphereMachine_ValidateUpdate(t *testing.T) { - g := NewWithT(t) - fakeProviderID := "fake-000000" tests := []struct { name string @@ -67,7 +65,9 @@ func TestVSphereMachine_ValidateUpdate(t *testing.T) { }, } for _, tc := range tests { - t.Run(tc.name, func(_ *testing.T) { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + webhook := &VSphereMachineWebhook{} _, err := webhook.ValidateUpdate(context.Background(), tc.oldVSphereMachine, tc.vsphereMachine) if tc.wantErr { diff --git a/internal/webhooks/vmware/vspheremachinetemplate.go b/internal/webhooks/vmware/vspheremachinetemplate.go new file mode 100644 index 0000000000..51d4f9e4cc --- /dev/null +++ b/internal/webhooks/vmware/vspheremachinetemplate.go @@ -0,0 +1,108 @@ +/* +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 vmware is the package for webhooks of vmware resources. +package vmware + +import ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" + "sigs.k8s.io/cluster-api-provider-vsphere/pkg/services/vmoperator" +) + +// +kubebuilder:webhook:verbs=create;update,path=/validate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachinetemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=vmware.infrastructure.cluster.x-k8s.io,resources=vspheremachinetemplates,versions=v1beta1,name=validation.vspheremachinetemplate.vmware.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 + +// VSphereMachineTemplateWebhook implements a validation webhook for VSphereMachineTemplate. +type VSphereMachineTemplateWebhook struct{} + +var _ webhook.CustomValidator = &VSphereMachineTemplateWebhook{} + +func (webhook *VSphereMachineTemplateWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&vmwarev1.VSphereMachineTemplate{}). + WithValidator(webhook). + Complete() +} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. +func (webhook *VSphereMachineTemplateWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + vSphereMachineTemplate, ok := obj.(*vmwarev1.VSphereMachineTemplate) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereMachineTemplate but got a %T", obj)) + } + return webhook.validate(ctx, nil, vSphereMachineTemplate) +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. +func (webhook *VSphereMachineTemplateWebhook) ValidateUpdate(ctx context.Context, _, newRaw runtime.Object) (admission.Warnings, error) { + vSphereMachineTemplate, ok := newRaw.(*vmwarev1.VSphereMachineTemplate) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereMachineTemplate but got a %T", newRaw)) + } + return webhook.validate(ctx, nil, vSphereMachineTemplate) +} + +func (webhook *VSphereMachineTemplateWebhook) validate(_ context.Context, _, newVSphereMachineTemplate *vmwarev1.VSphereMachineTemplate) (admission.Warnings, error) { + var allErrs field.ErrorList + + // Validate namingStrategy + namingStrategy := newVSphereMachineTemplate.Spec.Template.Spec.NamingStrategy + if namingStrategy != nil && + namingStrategy.Template != nil { + name, err := vmoperator.GenerateVirtualMachineName("machine", namingStrategy) + templateFldPath := field.NewPath("spec", "template", "spec", "namingStrategy", "template") + if err != nil { + allErrs = append(allErrs, + field.Invalid( + templateFldPath, + *namingStrategy.Template, + fmt.Sprintf("invalid VirtualMachine name template: %v", err), + ), + ) + } else { + // Note: This validates that the resulting name is a valid Kubernetes object name. + for _, err := range validation.IsDNS1123Subdomain(name) { + allErrs = append(allErrs, + field.Invalid( + templateFldPath, + *namingStrategy.Template, + fmt.Sprintf("invalid VirtualMachine name template, generated name is not a valid Kubernetes object name: %v", err), + ), + ) + } + } + } + + if len(allErrs) > 0 { + return nil, apierrors.NewInvalid(vmwarev1.GroupVersion.WithKind("VSphereMachineTemplate").GroupKind(), newVSphereMachineTemplate.Name, allErrs) + } + return nil, nil +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. +func (webhook *VSphereMachineTemplateWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} diff --git a/internal/webhooks/vmware/vspheremachinetemplate_test.go b/internal/webhooks/vmware/vspheremachinetemplate_test.go new file mode 100644 index 0000000000..c7fd406640 --- /dev/null +++ b/internal/webhooks/vmware/vspheremachinetemplate_test.go @@ -0,0 +1,99 @@ +/* +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 vmware + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + "k8s.io/utils/ptr" + + vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" +) + +func TestVSphereMachineTemplate_Validate(t *testing.T) { + tests := []struct { + name string + namingStrategy *vmwarev1.VirtualMachineNamingStrategy + wantErr bool + }{ + { + name: "Should succeed if namingStrategy not set", + namingStrategy: nil, + wantErr: false, + }, + { + name: "Should succeed if namingStrategy.template not set", + namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{ + Template: nil, + }, + wantErr: false, + }, + { + name: "Should succeed if namingStrategy.template is set to the fallback value", + namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{ + Template: ptr.To[string]("{{ .machine.name }}"), + }, + wantErr: false, + }, + { + name: "Should succeed if namingStrategy.template is set to the Windows example", + namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{ + Template: ptr.To[string]("{{ if le (len .machine.name) 20 }}{{ .machine.name }}{{else}}{{ trimSuffix \"-\" (trunc 14 .machine.name) }}-{{ trunc -5 .machine.name }}{{end}}"), + }, + wantErr: false, + }, + { + name: "Should fail if namingStrategy.template is set to an invalid template", + namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{ + Template: ptr.To[string]("{{ invalid"), + }, + wantErr: true, + }, + { + name: "Should fail if namingStrategy.template is set to a valid template that renders an invalid name", + namingStrategy: &vmwarev1.VirtualMachineNamingStrategy{ + Template: ptr.To[string]("-{{ .machine.name }}"), // Leading - is not valid for names. + }, + wantErr: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + vSphereMachineTemplate := &vmwarev1.VSphereMachineTemplate{ + Spec: vmwarev1.VSphereMachineTemplateSpec{ + Template: vmwarev1.VSphereMachineTemplateResource{ + Spec: vmwarev1.VSphereMachineSpec{ + NamingStrategy: tc.namingStrategy, + }, + }, + }, + } + + webhook := &VSphereMachineTemplateWebhook{} + _, err := webhook.validate(context.Background(), nil, vSphereMachineTemplate) + if tc.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} diff --git a/main.go b/main.go index 650e73b25c..8631c549b9 100644 --- a/main.go +++ b/main.go @@ -379,6 +379,9 @@ func setupVAPIControllers(ctx context.Context, controllerCtx *capvcontext.Contro } func setupSupervisorControllers(ctx context.Context, controllerCtx *capvcontext.ControllerManagerContext, mgr ctrlmgr.Manager, tracker *remote.ClusterCacheTracker) error { + if err := (&vmwarewebhooks.VSphereMachineTemplateWebhook{}).SetupWebhookWithManager(mgr); err != nil { + return err + } if err := (&vmwarewebhooks.VSphereMachineWebhook{}).SetupWebhookWithManager(mgr); err != nil { return err } diff --git a/pkg/services/vmoperator/vmopmachine.go b/pkg/services/vmoperator/vmopmachine.go index 159ff7f6b5..467447a32c 100644 --- a/pkg/services/vmoperator/vmopmachine.go +++ b/pkg/services/vmoperator/vmopmachine.go @@ -17,9 +17,12 @@ limitations under the License. package vmoperator import ( + "bytes" "context" "encoding/json" "fmt" + "strings" + "text/template" "github.com/pkg/errors" vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" @@ -117,7 +120,11 @@ func (v *VmopMachineService) ReconcileDelete(ctx context.Context, machineCtx cap // First, check to see if it's already deleted vmopVM := vmoprv1.VirtualMachine{} - if err := v.Client.Get(ctx, apitypes.NamespacedName{Namespace: supervisorMachineCtx.Machine.Namespace, Name: supervisorMachineCtx.Machine.Name}, &vmopVM); err != nil { + key, err := virtualMachineObjectKey(supervisorMachineCtx.Machine.Name, supervisorMachineCtx.Machine.Namespace, supervisorMachineCtx.VSphereMachine.Spec.NamingStrategy) + if err != nil { + return err + } + if err := v.Client.Get(ctx, *key, &vmopVM); err != nil { // If debug logging is enabled, report the number of vms in the cluster before and after the reconcile if apierrors.IsNotFound(err) { supervisorMachineCtx.VSphereMachine.Status.VMStatus = vmwarev1.VirtualMachineStateNotFound @@ -181,15 +188,21 @@ func (v *VmopMachineService) ReconcileNormal(ctx context.Context, machineCtx cap // Check for the presence of an existing object vmOperatorVM := &vmoprv1.VirtualMachine{} - if err := v.Client.Get(ctx, client.ObjectKey{ - Namespace: supervisorMachineCtx.Machine.Namespace, - Name: supervisorMachineCtx.Machine.Name, - }, vmOperatorVM); err != nil { + key, err := virtualMachineObjectKey(supervisorMachineCtx.Machine.Name, supervisorMachineCtx.Machine.Namespace, supervisorMachineCtx.VSphereMachine.Spec.NamingStrategy) + if err != nil { + return false, err + } + if err := v.Client.Get(ctx, *key, vmOperatorVM); err != nil { if !apierrors.IsNotFound(err) { return false, err } // Define the VM Operator VirtualMachine resource to reconcile. - vmOperatorVM = v.newVMOperatorVM(supervisorMachineCtx) + vmOperatorVM = &vmoprv1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: key.Name, + Namespace: key.Namespace, + }, + } } // Reconcile the VM Operator VirtualMachine. @@ -270,6 +283,76 @@ func (v *VmopMachineService) ReconcileNormal(ctx context.Context, machineCtx cap return false, nil } +const ( + maxNameLength = 63 +) + +// Note: Inlining these functions from sprig to avoid introducing a dependency. +var nameTemplateFuncs = map[string]any{ + "trimSuffix": func(a, b string) string { return strings.TrimSuffix(b, a) }, + "trunc": func(c int, s string) string { + if c < 0 && len(s)+c > 0 { + return s[len(s)+c:] + } + if c >= 0 && len(s) > c { + return s[:c] + } + return s + }, +} + +var nameTpl = template.New("name generator").Funcs(nameTemplateFuncs).Option("missingkey=error") + +// virtualMachineObjectKey returns the object key of the VirtualMachine. +// Part of this is generating the name of the VirtualMachine based on the naming strategy. +func virtualMachineObjectKey(machineName, machineNamespace string, namingStrategy *vmwarev1.VirtualMachineNamingStrategy) (*client.ObjectKey, error) { + name, err := GenerateVirtualMachineName(machineName, namingStrategy) + if err != nil { + return nil, err + } + + return &client.ObjectKey{ + Namespace: machineNamespace, + Name: name, + }, nil +} + +// GenerateVirtualMachineName generates the name of a VirtualMachine based on the naming strategy. +func GenerateVirtualMachineName(machineName string, namingStrategy *vmwarev1.VirtualMachineNamingStrategy) (string, error) { + // Per default the name of the VirtualMachine should be equal to the Machine name (this is the same as "{{ .machine.name }}") + if namingStrategy == nil || namingStrategy.Template == nil { + // Note: No need to trim to max length in this case as valid Machine names will also be valid VirtualMachine names. + return machineName, nil + } + + nameTemplate := *namingStrategy.Template + data := map[string]interface{}{ + "machine": map[string]interface{}{ + "name": machineName, + }, + } + + tpl, err := nameTpl.Parse(nameTemplate) + if err != nil { + return "", errors.Wrapf(err, "failed to generate name for VirtualMachine: failed to parse namingStrategy.template %q", nameTemplate) + } + + var buf bytes.Buffer + if err := tpl.Execute(&buf, data); err != nil { + return "", errors.Wrap(err, "failed to generate name for VirtualMachine") + } + + name := buf.String() + + // If the name exceeds the maxNameLength, trim to maxNameLength. + // Note: we're not adding a random suffix as the name has to be deterministic. + if len(name) > maxNameLength { + name = name[:maxNameLength] + } + + return name, nil +} + // GetHostInfo returns the hostname or IP address of the infrastructure host for the VM Operator VM. func (v *VmopMachineService) GetHostInfo(ctx context.Context, machineCtx capvcontext.MachineContext) (string, error) { supervisorMachineCtx, ok := machineCtx.(*vmware.SupervisorMachineContext) @@ -278,25 +361,17 @@ func (v *VmopMachineService) GetHostInfo(ctx context.Context, machineCtx capvcon } vmOperatorVM := &vmoprv1.VirtualMachine{} - if err := v.Client.Get(ctx, client.ObjectKey{ - Name: supervisorMachineCtx.Machine.Name, - Namespace: supervisorMachineCtx.Machine.Namespace, - }, vmOperatorVM); err != nil { + key, err := virtualMachineObjectKey(supervisorMachineCtx.Machine.Name, supervisorMachineCtx.Machine.Namespace, supervisorMachineCtx.VSphereMachine.Spec.NamingStrategy) + if err != nil { + return "", err + } + if err := v.Client.Get(ctx, *key, vmOperatorVM); err != nil { return "", err } return vmOperatorVM.Status.Host, nil } -func (v *VmopMachineService) newVMOperatorVM(supervisorMachineCtx *vmware.SupervisorMachineContext) *vmoprv1.VirtualMachine { - return &vmoprv1.VirtualMachine{ - ObjectMeta: metav1.ObjectMeta{ - Name: supervisorMachineCtx.Machine.Name, - Namespace: supervisorMachineCtx.Machine.Namespace, - }, - } -} - func (v *VmopMachineService) reconcileVMOperatorVM(ctx context.Context, supervisorMachineCtx *vmware.SupervisorMachineContext, vmOperatorVM *vmoprv1.VirtualMachine) error { // All Machine resources should define the version of Kubernetes to use. if supervisorMachineCtx.Machine.Spec.Version == nil || *supervisorMachineCtx.Machine.Spec.Version == "" { diff --git a/pkg/services/vmoperator/vmopmachine_test.go b/pkg/services/vmoperator/vmopmachine_test.go index 7e3f5f6437..1d4aec4adf 100644 --- a/pkg/services/vmoperator/vmopmachine_test.go +++ b/pkg/services/vmoperator/vmopmachine_test.go @@ -18,10 +18,12 @@ package vmoperator import ( "context" + "testing" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + gomegatypes "github.com/onsi/gomega/types" vmoprv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha2" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -29,6 +31,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util/conditions" "sigs.k8s.io/controller-runtime/pkg/client" @@ -651,3 +654,73 @@ var _ = Describe("GetMachinesInCluster", func() { Expect(objs[0].GetName()).To(Equal(machineName)) }) }) + +func Test_virtualMachineObjectKey(t *testing.T) { + tests := []struct { + name string + machineName string + template *string + want []gomegatypes.GomegaMatcher + wantErr bool + }{ + { + name: "default template", + machineName: "quick-start-d34gt4-md-0-wqc85-8nxwc-gfd5v", + template: nil, + want: []gomegatypes.GomegaMatcher{ + Equal("quick-start-d34gt4-md-0-wqc85-8nxwc-gfd5v"), + }, + }, + { + name: "template which doesn't respect max length: trim to max length", + machineName: "quick-start-d34gt4-md-0-wqc85-8nxwc-gfd5v", // 41 characters + template: ptr.To[string]("{{ .machine.name }}-{{ .machine.name }}"), + want: []gomegatypes.GomegaMatcher{ + Equal("quick-start-d34gt4-md-0-wqc85-8nxwc-gfd5v-quick-start-d34gt4-md"), // 63 characters + }, + }, + { + name: "template for 20 characters: keep machine name if name has 20 characters", + machineName: "quick-md-8nxwc-gfd5v", // 20 characters + template: ptr.To[string]("{{ if le (len .machine.name) 20 }}{{ .machine.name }}{{else}}{{ trimSuffix \"-\" (trunc 14 .machine.name) }}-{{ trunc -5 .machine.name }}{{end}}"), + want: []gomegatypes.GomegaMatcher{ + Equal("quick-md-8nxwc-gfd5v"), // 20 characters + }, + }, + { + name: "template for 20 characters: trim to 20 characters if name has more than 20 characters", + machineName: "quick-start-d34gt4-md-0-wqc85-8nxwc-gfd5v", // 41 characters + template: ptr.To[string]("{{ if le (len .machine.name) 20 }}{{ .machine.name }}{{else}}{{ trimSuffix \"-\" (trunc 14 .machine.name) }}-{{ trunc -5 .machine.name }}{{end}}"), + want: []gomegatypes.GomegaMatcher{ + Equal("quick-start-d3-gfd5v"), // 20 characters + }, + }, + { + name: "template for 20 characters: trim to 19 characters if name has more than 20 characters and last character of prefix is -", + machineName: "quick-start-d-34gt4-md-0-wqc85-8nxwc-gfd5v", // 42 characters + template: ptr.To[string]("{{ if le (len .machine.name) 20 }}{{ .machine.name }}{{else}}{{ trimSuffix \"-\" (trunc 14 .machine.name) }}-{{ trunc -5 .machine.name }}{{end}}"), + want: []gomegatypes.GomegaMatcher{ + Equal("quick-start-d-gfd5v"), // 19 characters + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + got, err := virtualMachineObjectKey(tt.machineName, corev1.NamespaceDefault, &vmwarev1.VirtualMachineNamingStrategy{ + Template: tt.template, + }) + if (err != nil) != tt.wantErr { + t.Errorf("virtualMachineObjectKey error = %v, wantErr %v", err, tt.wantErr) + return + } + if len(got.Name) > maxNameLength { + t.Errorf("generated name should never be longer than %d, got %d", maxNameLength, len(got.Name)) + } + for _, matcher := range tt.want { + g.Expect(got.Name).To(matcher) + } + }) + } +} diff --git a/test/e2e/data/infrastructure-vsphere-supervisor/main/clusterclass/kustomization.yaml b/test/e2e/data/infrastructure-vsphere-supervisor/main/clusterclass/kustomization.yaml index bab4a05d78..a0dabb0005 100644 --- a/test/e2e/data/infrastructure-vsphere-supervisor/main/clusterclass/kustomization.yaml +++ b/test/e2e/data/infrastructure-vsphere-supervisor/main/clusterclass/kustomization.yaml @@ -12,3 +12,6 @@ patches: - target: kind: ClusterClass path: ./patch-namingstrategy.yaml + - target: + kind: VSphereMachineTemplate + path: ./patch-vm-namingstrategy.yaml diff --git a/test/e2e/data/infrastructure-vsphere-supervisor/main/clusterclass/patch-vm-namingstrategy.yaml b/test/e2e/data/infrastructure-vsphere-supervisor/main/clusterclass/patch-vm-namingstrategy.yaml new file mode 100644 index 0000000000..0ec61e9df0 --- /dev/null +++ b/test/e2e/data/infrastructure-vsphere-supervisor/main/clusterclass/patch-vm-namingstrategy.yaml @@ -0,0 +1,4 @@ +- op: add + path: /spec/template/spec/namingStrategy + value: + template: '{{ if le (len .machine.name) 20 }}{{ .machine.name }}{{else}}{{ trimSuffix "-" (trunc 14 .machine.name) }}-{{ trunc -5 .machine.name }}{{end}}'