diff --git a/operator/config/crd/bases/forklift.konveyor.io_migrations.yaml b/operator/config/crd/bases/forklift.konveyor.io_migrations.yaml index b5d5771d3..814c94139 100644 --- a/operator/config/crd/bases/forklift.konveyor.io_migrations.yaml +++ b/operator/config/crd/bases/forklift.konveyor.io_migrations.yaml @@ -512,6 +512,20 @@ spec: - progress type: object type: array + pvcNameTemplate: + description: |- + PVCNameTemplate is a template for generating PVC names for VM disks. + It follows Go template syntax and has access to the following variables: + - .VmName: name of the VM + - .PlanName: name of the migration plan + - .DiskIndex: initial volume index of the disk + - .RootDiskIndex: index of the root disk + Note: + This template overrides the plan level template. + Examples: + "{{.VmName}}-disk-{{.DiskIndex}}" + "{{if eq .DiskIndex .RootDiskIndex}}root{{else}}data{{end}}-{{.DiskIndex}}" + type: string restorePowerState: description: Source VM power state before migration. type: string diff --git a/operator/config/crd/bases/forklift.konveyor.io_plans.yaml b/operator/config/crd/bases/forklift.konveyor.io_plans.yaml index 7ddf1136a..ff65d7993 100644 --- a/operator/config/crd/bases/forklift.konveyor.io_plans.yaml +++ b/operator/config/crd/bases/forklift.konveyor.io_plans.yaml @@ -257,6 +257,20 @@ spec: - destination - source type: object + pvcNameTemplate: + description: |- + PVCNameTemplate is a template for generating PVC names for VM disks. + It follows Go template syntax and has access to the following variables: + - .VmName: name of the VM + - .PlanName: name of the migration plan + - .DiskIndex: initial volume index of the disk + - .RootDiskIndex: index of the root disk + Note: + This template can be overridden at the individual VM level. + Examples: + "{{.VmName}}-disk-{{.DiskIndex}}" + "{{if eq .DiskIndex .RootDiskIndex}}root{{else}}data{{end}}-{{.DiskIndex}}" + type: string targetNamespace: description: Target namespace. type: string @@ -432,6 +446,20 @@ spec: The VM Namespace Only relevant for an openshift source. type: string + pvcNameTemplate: + description: |- + PVCNameTemplate is a template for generating PVC names for VM disks. + It follows Go template syntax and has access to the following variables: + - .VmName: name of the VM + - .PlanName: name of the migration plan + - .DiskIndex: initial volume index of the disk + - .RootDiskIndex: index of the root disk + Note: + This template overrides the plan level template. + Examples: + "{{.VmName}}-disk-{{.DiskIndex}}" + "{{if eq .DiskIndex .RootDiskIndex}}root{{else}}data{{end}}-{{.DiskIndex}}" + type: string rootDisk: description: Choose the primary disk the VM boots from type: string @@ -1024,6 +1052,20 @@ spec: - progress type: object type: array + pvcNameTemplate: + description: |- + PVCNameTemplate is a template for generating PVC names for VM disks. + It follows Go template syntax and has access to the following variables: + - .VmName: name of the VM + - .PlanName: name of the migration plan + - .DiskIndex: initial volume index of the disk + - .RootDiskIndex: index of the root disk + Note: + This template overrides the plan level template. + Examples: + "{{.VmName}}-disk-{{.DiskIndex}}" + "{{if eq .DiskIndex .RootDiskIndex}}root{{else}}data{{end}}-{{.DiskIndex}}" + type: string restorePowerState: description: Source VM power state before migration. type: string diff --git a/pkg/apis/forklift/v1beta1/plan.go b/pkg/apis/forklift/v1beta1/plan.go index 17ee8afb2..e7c86adbe 100644 --- a/pkg/apis/forklift/v1beta1/plan.go +++ b/pkg/apis/forklift/v1beta1/plan.go @@ -48,6 +48,19 @@ type PlanSpec struct { PreserveClusterCPUModel bool `json:"preserveClusterCpuModel,omitempty"` // Preserve static IPs of VMs in vSphere PreserveStaticIPs bool `json:"preserveStaticIPs,omitempty"` + // PVCNameTemplate is a template for generating PVC names for VM disks. + // It follows Go template syntax and has access to the following variables: + // - .VmName: name of the VM + // - .PlanName: name of the migration plan + // - .DiskIndex: initial volume index of the disk + // - .RootDiskIndex: index of the root disk + // Note: + // This template can be overridden at the individual VM level. + // Examples: + // "{{.VmName}}-disk-{{.DiskIndex}}" + // "{{if eq .DiskIndex .RootDiskIndex}}root{{else}}data{{end}}-{{.DiskIndex}}" + // +optional + PVCNameTemplate string `json:"pvcNameTemplate,omitempty"` } // Find a planned VM. @@ -141,3 +154,11 @@ func (r *Plan) IsSourceProviderOCP() bool { } func (r *Plan) IsSourceProviderVSphere() bool { return r.Provider.Source.Type() == VSphere } + +// PVCNameTemplateData contains fields used in naming templates. +type PVCNameTemplateData struct { + VmName string `json:"vmName"` + PlanName string `json:"planName"` + DiskIndex int `json:"diskIndex"` + RootDiskIndex int `json:"rootDiskIndex"` +} diff --git a/pkg/apis/forklift/v1beta1/plan/vm.go b/pkg/apis/forklift/v1beta1/plan/vm.go index 8b3fdf000..047a2fcae 100644 --- a/pkg/apis/forklift/v1beta1/plan/vm.go +++ b/pkg/apis/forklift/v1beta1/plan/vm.go @@ -39,6 +39,19 @@ type VM struct { // Selected InstanceType that will override the VM properties. // +optional InstanceType string `json:"instanceType,omitempty"` + // PVCNameTemplate is a template for generating PVC names for VM disks. + // It follows Go template syntax and has access to the following variables: + // - .VmName: name of the VM + // - .PlanName: name of the migration plan + // - .DiskIndex: initial volume index of the disk + // - .RootDiskIndex: index of the root disk + // Note: + // This template overrides the plan level template. + // Examples: + // "{{.VmName}}-disk-{{.DiskIndex}}" + // "{{if eq .DiskIndex .RootDiskIndex}}root{{else}}data{{end}}-{{.DiskIndex}}" + // +optional + PVCNameTemplate string `json:"pvcNameTemplate,omitempty"` } // Find a Hook for the specified step. diff --git a/pkg/apis/forklift/v1beta1/zz_generated.deepcopy.go b/pkg/apis/forklift/v1beta1/zz_generated.deepcopy.go index f078c1050..0aa888f66 100644 --- a/pkg/apis/forklift/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/forklift/v1beta1/zz_generated.deepcopy.go @@ -656,6 +656,21 @@ func (in *OvirtVolumePopulatorStatus) DeepCopy() *OvirtVolumePopulatorStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PVCNameTemplateData) DeepCopyInto(out *PVCNameTemplateData) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PVCNameTemplateData. +func (in *PVCNameTemplateData) DeepCopy() *PVCNameTemplateData { + if in == nil { + return nil + } + out := new(PVCNameTemplateData) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Plan) DeepCopyInto(out *Plan) { *out = *in diff --git a/pkg/controller/plan/adapter/vsphere/builder.go b/pkg/controller/plan/adapter/vsphere/builder.go index 15d4709d2..e188650ea 100644 --- a/pkg/controller/plan/adapter/vsphere/builder.go +++ b/pkg/controller/plan/adapter/vsphere/builder.go @@ -1,6 +1,7 @@ package vsphere import ( + "bytes" "context" "errors" "fmt" @@ -11,6 +12,7 @@ import ( "regexp" "sort" "strings" + "text/template" api "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1" "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1/plan" @@ -390,6 +392,71 @@ func (r *Builder) Secret(vmRef ref.Ref, in, object *core.Secret) (err error) { return } +// Get the plan VM for the given vsphere VM +func (r *Builder) getPlanVM(vm *model.VM) *plan.VM { + for _, planVM := range r.Plan.Spec.VMs { + if planVM.ID == vm.ID { + return &planVM + } + } + + return nil +} + +// GetPVCNameTemplate returns the PVC name template +func (r *Builder) getPVCNameTemplate(vm *model.VM) string { + // Get plan VM + planVM := r.getPlanVM(vm) + if planVM == nil { + return "" + } + + // if vm.PVCNameTemplate is set, use it + if planVM.PVCNameTemplate != "" { + return planVM.PVCNameTemplate + } + + // if planSpec.PVCNameTemplate is set, use it + if r.Plan.Spec.PVCNameTemplate != "" { + return r.Plan.Spec.PVCNameTemplate + } + + return "" +} + +func (r *Builder) getGeneratePVCName(pvcNameTemplate string, vm *model.VM, diskIndex int) (string, error) { + var buf bytes.Buffer + + // Get plan VM + planVM := r.getPlanVM(vm) + if planVM == nil { + return "", errors.New("plan VM not found") + } + rootDisk := planVM.RootDisk + + // Create template data + templateData := api.PVCNameTemplateData{ + VmName: vm.Name, + PlanName: r.Plan.Name, + DiskIndex: diskIndex, + RootDiskIndex: utils.GetDeviceNumber(rootDisk), + } + + // Parse template syntax + tmpl, err := template.New("pvcname").Parse(pvcNameTemplate) + if err != nil { + return "", err + } + + // Execute template + err = tmpl.Execute(&buf, templateData) + if err != nil { + return "", err + } + + return buf.String(), nil +} + // Create DataVolume specs for the VM. func (r *Builder) DataVolumes(vmRef ref.Ref, secret *core.Secret, _ *core.ConfigMap, dvTemplate *cdi.DataVolume) (dvs []cdi.DataVolume, err error) { vm := &model.VM{} @@ -430,7 +497,7 @@ func (r *Builder) DataVolumes(vmRef ref.Ref, secret *core.Secret, _ *core.Config err = fErr return } - for _, disk := range vm.Disks { + for diskIndex, disk := range vm.Disks { if disk.Datastore.ID == ds.ID { storageClass := mapped.Destination.StorageClass var dvSource cdi.DataVolumeSource @@ -483,6 +550,17 @@ func (r *Builder) DataVolumes(vmRef ref.Ref, secret *core.Secret, _ *core.Config dv.ObjectMeta.Annotations = make(map[string]string) } dv.ObjectMeta.Annotations[planbase.AnnDiskSource] = r.baseVolume(disk.File) + + // if exists, get the PVC generate name from the PlanSpec, generate the name + // and update the GenerateName field in the DataVolume object. + pvcNameTemplate := r.getPVCNameTemplate(vm) + if pvcNameTemplate != "" { + generatedName, err := r.getGeneratePVCName(pvcNameTemplate, vm, diskIndex) + if err == nil && generatedName != "" { + dv.ObjectMeta.GenerateName = generatedName + } + } + dvs = append(dvs, *dv) } } diff --git a/pkg/controller/plan/validation.go b/pkg/controller/plan/validation.go index 224859f12..d6e9590f2 100644 --- a/pkg/controller/plan/validation.go +++ b/pkg/controller/plan/validation.go @@ -1,6 +1,7 @@ package plan import ( + "bytes" "context" "crypto/md5" "encoding/hex" @@ -9,6 +10,7 @@ import ( "net" "path" "strconv" + "text/template" k8snet "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" api "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1" @@ -164,6 +166,27 @@ func (r *Reconciler) validate(plan *api.Plan) error { return err } + // Validate PVC name template + if err := r.validatePVCNameTemplate(plan); err != nil { + return err + } + + return nil +} + +func (r *Reconciler) validatePVCNameTemplate(plan *api.Plan) error { + if err := r.IsValidPVCNameTemplate(plan.Spec.PVCNameTemplate); err != nil { + invalidPVCNameTemplate := libcnd.Condition{ + Type: NotValid, + Status: True, + Category: Critical, + Message: "PVC name template is invalid.", + Items: []string{}, + } + + plan.Status.SetCondition(invalidPVCNameTemplate) + } + return nil } @@ -454,6 +477,13 @@ func (r *Reconciler) validateVM(plan *api.Plan) error { Message: "Changed Block Tracking (CBT) has not been enabled on some VM. This feature is a prerequisite for VM warm migration.", Items: []string{}, } + pvcNameInvalid := libcnd.Condition{ + Type: NotValid, + Status: True, + Category: Critical, + Message: "VM PVC name template is invalid.", + Items: []string{}, + } setOf := map[string]bool{} // @@ -589,6 +619,12 @@ func (r *Reconciler) validateVM(plan *api.Plan) error { missingCbtForWarm.Items = append(missingCbtForWarm.Items, ref.String()) } } + // is valid vm pvc name template + if plan.Spec.VMs[i].PVCNameTemplate != "" { + if err := r.IsValidPVCNameTemplate(plan.Spec.VMs[i].PVCNameTemplate); err != nil { + pvcNameInvalid.Items = append(pvcNameInvalid.Items, ref.String()) + } + } } if len(notFound.Items) > 0 { plan.Status.SetCondition(notFound) @@ -626,6 +662,9 @@ func (r *Reconciler) validateVM(plan *api.Plan) error { if len(missingCbtForWarm.Items) > 0 { plan.Status.SetCondition(missingCbtForWarm) } + if len(pvcNameInvalid.Items) > 0 { + plan.Status.SetCondition(pvcNameInvalid) + } return nil } @@ -1136,3 +1175,43 @@ func (r *Reconciler) checkOCPVersion(clientset kubernetes.Interface) error { return nil } + +func (r *Reconciler) IsValidPVCNameTemplate(pvcNameTemplate string) error { + if pvcNameTemplate == "" { + return nil + } + + // Validate golang template syntax + tmpl, err := template.New("pvcname").Parse(pvcNameTemplate) + if err != nil { + return liberr.Wrap(err, "Invalid template syntax") + } + + // Test template with sample data + testData := api.PVCNameTemplateData{ + VmName: "test-vm", + PlanName: "test-plan", + DiskIndex: 0, + RootDiskIndex: 0, + } + + var buf bytes.Buffer + err = tmpl.Execute(&buf, testData) + if err != nil { + return liberr.Wrap(err, "Template execution failed") + } + result := buf.String() + + // Empty output is not valid + if result == "" { + return liberr.New("Template output is empty") + } + + // Validate that template output is a valid k8s label + errs := k8svalidation.IsValidLabelValue(result) + if len(errs) > 0 { + return liberr.New("Template output is not a valid k8s label", "errors", errs) + } + + return nil +} diff --git a/pkg/controller/plan/validation_test.go b/pkg/controller/plan/validation_test.go index 64fa91ff8..cbffa7388 100644 --- a/pkg/controller/plan/validation_test.go +++ b/pkg/controller/plan/validation_test.go @@ -22,6 +22,19 @@ import ( fakeClient "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +const ( + sourceNamespace = "source-namespace" + destNamespace = "destination-namespace" + testNamespace = "test-namespace" + sourceName = "source" + destName = "destination" + sourceSecretName = "source-secret" + testPlanName = "test-plan" + tokenKey = "token" + tokenValue = "token" + insecureSkipKey = "inscureSkipVerify" +) + var ( planValidationLog = logging.WithName("planValidation") ) @@ -65,10 +78,10 @@ var _ = ginkgo.Describe("Plan Validations", func() { ginkgo.Describe("validate", func() { ginkgo.It("Should setup secret when source is not local cluster", func() { - secret := createSecret("source-secret", "source-namespace", false) - source := createProvider("source", "source-namespace", "https://source", v1beta1.OpenShift, &core.ObjectReference{Name: "source-secret", Namespace: "source-namespace"}) - destination := createProvider("destination", "destination-namespace", "", v1beta1.OpenShift, &core.ObjectReference{}) - plan := createPlan("test-plan", "test-namespace", source, destination) + secret := createSecret(sourceSecretName, sourceNamespace, false) + source := createProvider(sourceName, sourceNamespace, "https://source", v1beta1.OpenShift, &core.ObjectReference{Name: sourceSecretName, Namespace: sourceNamespace}) + destination := createProvider(destName, destNamespace, "", v1beta1.OpenShift, &core.ObjectReference{}) + plan := createPlan(testPlanName, testNamespace, source, destination) source.Status.Conditions.SetCondition(condition.Condition{Type: condition.Ready, Status: condition.True}) destination.Status.Conditions.SetCondition(condition.Condition{Type: condition.Ready, Status: condition.True}) @@ -81,10 +94,10 @@ var _ = ginkgo.Describe("Plan Validations", func() { }) ginkgo.It("Should not setup secret when source is local cluster", func() { - secret := createSecret("source-secret", "source-namespace", false) - source := createProvider("source", "source-namespace", "", v1beta1.OpenShift, &core.ObjectReference{Name: "source-secret", Namespace: "source-namespace"}) - destination := createProvider("destination", "destination-namespace", "https://destination", v1beta1.OpenShift, &core.ObjectReference{}) - plan := createPlan("test-plan", "test-namespace", source, destination) + secret := createSecret(sourceSecretName, sourceNamespace, false) + source := createProvider(sourceName, sourceNamespace, "", v1beta1.OpenShift, &core.ObjectReference{Name: sourceSecretName, Namespace: sourceNamespace}) + destination := createProvider(destName, destNamespace, "https://destination", v1beta1.OpenShift, &core.ObjectReference{}) + plan := createPlan(testPlanName, testNamespace, source, destination) source.Status.Conditions.SetCondition(condition.Condition{Type: condition.Ready, Status: condition.True}) destination.Status.Conditions.SetCondition(condition.Condition{Type: condition.Ready, Status: condition.True}) @@ -96,6 +109,78 @@ var _ = ginkgo.Describe("Plan Validations", func() { gomega.Expect(plan.Referenced.Secret).To(gomega.BeNil()) }) }) + + ginkgo.Describe("validatePVCNameTemplate", func() { + var reconciler *Reconciler + + ginkgo.BeforeEach(func() { + reconciler = &Reconciler{} + }) + + source := createProvider(sourceName, sourceNamespace, "", v1beta1.OpenShift, &core.ObjectReference{Name: sourceSecretName, Namespace: sourceNamespace}) + destination := createProvider(destName, destNamespace, "https://destination", v1beta1.OpenShift, &core.ObjectReference{}) + + ginkgo.DescribeTable("should validate a plan correctly", + func(template string, shouldBeValid bool) { + plan := createPlan(testPlanName, testNamespace, source, destination) + plan.Spec.PVCNameTemplate = template + + err := reconciler.validatePVCNameTemplate(plan) + if err != nil { + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } + + hasInvalidCondition := false + for _, cond := range plan.Status.Conditions.List { + if cond.Type == NotValid { + hasInvalidCondition = true + break + } + } + + if shouldBeValid { + gomega.Expect(hasInvalidCondition).To(gomega.BeFalse()) + } else { + + gomega.Expect(hasInvalidCondition).To(gomega.BeTrue()) + } + }, + ginkgo.Entry("empty template is valid", "", true), + ginkgo.Entry("simple valid template", "{{.VmName}}-disk-{{.DiskIndex}}", true), + ginkgo.Entry("complex valid template", "{{.PlanName}}-{{.VmName}}-disk-{{.DiskIndex}}", true), + ginkgo.Entry("valid template with root disk index", "{{if eq .DiskIndex .RootDiskIndex}}root{{else}}data{{end}}-{{.DiskIndex}}", true), + ginkgo.Entry("template with invalid k8s label chars", "disk@{{.DiskIndex}}", false), + ginkgo.Entry("template with undefined variable", "{{.UndefinedVar}}", false), + ginkgo.Entry("template resulting in empty string", "{{if false}}disk{{end}}", false), + ) + }) + + ginkgo.Describe("IsValidPVCNameTemplate", func() { + var reconciler *Reconciler + + ginkgo.BeforeEach(func() { + reconciler = &Reconciler{} + }) + + ginkgo.DescribeTable("should validate PVC name template correctly", + func(template string, shouldBeValid bool) { + err := reconciler.IsValidPVCNameTemplate(template) + if shouldBeValid { + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } else { + gomega.Expect(err).To(gomega.HaveOccurred()) + } + }, + ginkgo.Entry("empty template is valid", "", true), + ginkgo.Entry("simple valid template", "{{.VmName}}-disk-{{.DiskIndex}}", true), + ginkgo.Entry("complex valid template", "{{.PlanName}}-{{.VmName}}-disk-{{.DiskIndex}}", true), + ginkgo.Entry("valid template with root disk index", "{{if eq .DiskIndex .RootDiskIndex}}root{{else}}data{{end}}-{{.DiskIndex}}", true), + ginkgo.Entry("invalid template syntax", "{{.VmName}-disk-{{.DiskIndex}", false), + ginkgo.Entry("template with invalid k8s label chars", "disk@{{.DiskIndex}}", false), + ginkgo.Entry("template with undefined variable", "{{.UndefinedVar}}", false), + ginkgo.Entry("template resulting in empty string", "{{if false}}disk{{end}}", false), + ) + }) }) //nolint:errcheck @@ -141,8 +226,8 @@ func createSecret(name, namespace string, insecure bool) *core.Secret { Namespace: namespace, }, Data: map[string][]byte{ - "inscureSkipVerify": []byte(strconv.FormatBool(insecure)), - "token": []byte("token"), + insecureSkipKey: []byte(strconv.FormatBool(insecure)), + tokenKey: []byte(tokenValue), }, } }