Brownfield fix to pass instanceID to CloudInit

api/v1alpha1/virtualmachine_types.go

@@ -80,7 +80,7 @@ const (
 
 const (
 	// PauseAnnotation is an annotation that can be applied to any VirtualMachine object to prevent VM Operator from
-	// reconciling the object with the vSphere infrastructure.  VM Operator checks the presence of this annotation to
+	// reconciling the object with the vSphere infrastructure. VM Operator checks the presence of this annotation to
 	// skip the reconcile of a VirtualMachine.
 	//
 	// This can be used when a Virtual Machine needs to be modified out-of-band of VM Operator on the infrastructure
@@ -96,6 +96,15 @@ const (
 	// this annotation to skip adding a default nic. VM Operator won't add default NIC to any existing VMs or new VMs
 	// with VirtualMachineNetworkInterfaces specified. This annotation is not required for such VMs.
 	NoDefaultNicAnnotation = GroupName + "/no-default-nic"
+
+	// InstanceIDAnnotation is an annotation that can be applied to set Cloud-Init metadata Instance ID.
+	//
+	// This cannot be set by users. It is for VM Operator to handle corner cases.
+	//
+	// In a corner case where a VM first boot failed to bootstrap with Cloud-Init, VM Operator sets Instance ID
+	// the same with the first boot Instance ID to prevent Cloud-Init from treating this VM as first boot
+	// due to different Instance ID.
+	InstanceIDAnnotation = GroupName + "/instance-id"
 )
 
 // VirtualMachinePort is unused and can be considered deprecated.

api/v1alpha2/virtualmachine_types.go

@@ -93,6 +93,15 @@ const (
 	//
 	// The VM will not be reconciled again until this annotation is removed.
 	PauseAnnotation = GroupName + "/pause-reconcile"
+
+	// InstanceIDAnnotation is an annotation that can be applied to set Cloud-Init metadata Instance ID.
+	//
+	// This cannot be set by users. It is for VM Operator to handle corner cases.
+	//
+	// In a corner case where a VM first boot failed to bootstrap with Cloud-Init, VM Operator sets Instance ID
+	// the same with the first boot Instance ID to prevent Cloud-Init from treating this VM as first boot
+	// due to different Instance ID.
+	InstanceIDAnnotation = GroupName + "/instance-id"
 )
 
 // VirtualMachinePowerState defines a VM's desired and observed power states.

pkg/vmprovider/providers/vsphere/session/session_vm_customization.go

@@ -99,12 +99,19 @@ func GetCloudInitMetadata(vm *vmopv1.VirtualMachine,
 	data map[string]string) (string, error) {
 
 	metadataObj := &CloudInitMetadata{
-		InstanceID:    string(vm.UID),
 		LocalHostname: vm.Name,
 		Hostname:      vm.Name,
 		Network:       netplan,
 		PublicKeys:    data["ssh-public-keys"],
 	}
+	// This is to address a bug when Cloud-Init sans configMap/secret actually got
+	// customized by LinuxPrep. Use annotation as instanceID to avoid
+	// different instanceID triggers CloudInit.
+	if value, ok := vm.Annotations[vmopv1.InstanceIDAnnotation]; ok {
+		metadataObj.InstanceID = value
+	} else {
+		metadataObj.InstanceID = string(vm.UID)
+	}
 
 	metadataBytes, err := yaml.Marshal(metadataObj)
 	if err != nil {

webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go

@@ -64,6 +64,7 @@ const (
 	invalidNextRestartTimeOnCreate            = "cannot restart VM on create"
 	invalidNextRestartTimeOnUpdate            = "must be formatted as RFC3339Nano"
 	invalidNextRestartTimeOnUpdateNow         = "mutation webhooks are required to restart VM"
+	settingInstanceIDAnnotationNotAllowed     = "adding instance-id annotation is not allowed"
 )
 
 // +kubebuilder:webhook:verbs=create;update,path=/default-validate-vmoperator-vmware-com-v1alpha1-virtualmachine,mutating=false,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachines,versions=v1alpha1,name=default.validating.virtualmachine.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1
@@ -119,6 +120,7 @@ func (v validator) ValidateCreate(ctx *context.WebhookRequestContext) admission.
 	fieldErrs = append(fieldErrs, v.validateInstanceStorageVolumes(ctx, vm, nil)...)
 	fieldErrs = append(fieldErrs, v.validatePowerStateOnCreate(ctx, vm)...)
 	fieldErrs = append(fieldErrs, v.validateNextRestartTimeOnCreate(ctx, vm)...)
+	fieldErrs = append(fieldErrs, v.validateAnnotation(ctx, vm)...)
 
 	validationErrs := make([]string, 0, len(fieldErrs))
 	for _, fieldErr := range fieldErrs {
@@ -178,6 +180,7 @@ func (v validator) ValidateUpdate(ctx *context.WebhookRequestContext) admission.
 	fieldErrs = append(fieldErrs, v.validateReadinessProbe(ctx, vm)...)
 	fieldErrs = append(fieldErrs, v.validateInstanceStorageVolumes(ctx, vm, oldVM)...)
 	fieldErrs = append(fieldErrs, v.validateNextRestartTimeOnUpdate(ctx, vm, oldVM)...)
+	fieldErrs = append(fieldErrs, v.validateAnnotation(ctx, vm)...)
 
 	validationErrs := make([]string, 0, len(fieldErrs))
 	for _, fieldErr := range fieldErrs {
@@ -738,3 +741,17 @@ func (v validator) isNetworkRestrictedForReadinessProbe(ctx *context.WebhookRequ
 
 	return configMap.Data[isRestrictedNetworkKey] == "true", nil
 }
+
+func (v validator) validateAnnotation(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList {
+	var allErrs field.ErrorList
+	annotationPath := field.NewPath("metadata", "annotations")
+	forbiddenAnnotationKey := vmopv1.InstanceIDAnnotation
+
+	if vm.Annotations != nil {
+		if _, exists := vm.Annotations[forbiddenAnnotationKey]; exists {
+			allErrs = append(allErrs, field.Forbidden(annotationPath.Child(forbiddenAnnotationKey), settingInstanceIDAnnotationNotAllowed))
+		}
+	}
+
+	return allErrs
+}

webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_unit_test.go

@@ -221,6 +221,7 @@ func unitTestsValidateCreate() {
 		isSysprepTransportUsed            bool
 		powerState                        vmopv1.VirtualMachinePowerState
 		nextRestartTime                   string
+		isForbiddenAnnotation             bool
 	}
 
 	validateCreate := func(args createArgs, expectedAllowed bool, expectedReason string, expectedErr error) {
@@ -368,6 +369,10 @@ func unitTestsValidateCreate() {
 			ctx.vm.Labels[topology.KubernetesTopologyZoneLabelKey] = zoneName
 		}
 
+		if args.isForbiddenAnnotation {
+			ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = "some-value"
+		}
+
 		ctx.vm.Spec.PowerState = args.powerState
 		ctx.vm.Spec.NextRestartTime = args.nextRestartTime
 
@@ -414,6 +419,7 @@ func unitTestsValidateCreate() {
 	volPath := specPath.Child("volumes")
 	nextRestartTimePath := specPath.Child("nextRestartTime")
 	now := time.Now().UTC()
+	annotationPath := field.NewPath("metadata", "annotations")
 
 	DescribeTable("create table", validateCreate,
 		Entry("should allow valid", createArgs{}, true, nil, nil),
@@ -506,6 +512,8 @@ func unitTestsValidateCreate() {
 		Entry("should disallow creating VM with non-empty, invalid nextRestartTime value",
 			createArgs{nextRestartTime: "hello"}, false,
 			field.Invalid(nextRestartTimePath, "hello", "cannot restart VM on create").Error(), nil),
+		Entry("should deny instance-id annotation set by user", createArgs{isForbiddenAnnotation: true}, false,
+			field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "adding instance-id annotation is not allowed").Error(), nil),
 	)
 }
 
@@ -533,6 +541,7 @@ func unitTestsValidateUpdate() {
 		newPowerStateEmptyAllowed       bool
 		nextRestartTime                 string
 		lastRestartTime                 string
+		isForbiddenAnnotation           bool
 	}
 
 	validateUpdate := func(args updateArgs, expectedAllowed bool, expectedReason string, expectedErr error) {
@@ -581,6 +590,9 @@ func unitTestsValidateUpdate() {
 		ctx.oldVM.Spec.NextRestartTime = args.lastRestartTime
 		ctx.vm.Spec.NextRestartTime = args.nextRestartTime
 
+		if args.isForbiddenAnnotation {
+			ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = "some-value"
+		}
 		// Named network provider
 		undoNamedNetProvider := initNamedNetworkProviderConfig(
 			ctx,
@@ -621,6 +633,7 @@ func unitTestsValidateUpdate() {
 	volumesPath := field.NewPath("spec", "volumes")
 	powerStatePath := field.NewPath("spec", "powerState")
 	nextRestartTimePath := field.NewPath("spec", "nextRestartTime")
+	annotationPath := field.NewPath("metadata", "annotations")
 
 	DescribeTable("update table", validateUpdate,
 		// Immutable Fields
@@ -672,6 +685,8 @@ func unitTestsValidateUpdate() {
 		Entry("should disallow updating VM with non-empty, invalid nextRestartTime value ",
 			updateArgs{nextRestartTime: "hello"}, false,
 			field.Invalid(nextRestartTimePath, "hello", "must be formatted as RFC3339Nano").Error(), nil),
+		Entry("should deny instance-id annotation set by user", updateArgs{isForbiddenAnnotation: true}, false,
+			field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "adding instance-id annotation is not allowed").Error(), nil),
 	)
 
 	When("the update is performed while object deletion", func() {

webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go

@@ -59,6 +59,7 @@ const (
 	invalidNextRestartTimeOnCreate           = "cannot restart VM on create"
 	invalidNextRestartTimeOnUpdate           = "must be formatted as RFC3339Nano"
 	invalidNextRestartTimeOnUpdateNow        = "mutation webhooks are required to restart VM"
+	settingInstanceIDAnnotationNotAllowed    = "adding instance-id annotation is not allowed"
 )
 
 // +kubebuilder:webhook:verbs=create;update,path=/default-validate-vmoperator-vmware-com-v1alpha2-virtualmachine,mutating=false,failurePolicy=fail,groups=vmoperator.vmware.com,resources=virtualmachines,versions=v1alpha2,name=default.validating.virtualmachine.v1alpha2.vmoperator.vmware.com,sideEffects=None,admissionReviewVersions=v1;v1beta1
@@ -114,6 +115,7 @@ func (v validator) ValidateCreate(ctx *context.WebhookRequestContext) admission.
 	fieldErrs = append(fieldErrs, v.validateAdvanced(ctx, vm)...)
 	fieldErrs = append(fieldErrs, v.validatePowerStateOnCreate(ctx, vm)...)
 	fieldErrs = append(fieldErrs, v.validateNextRestartTimeOnCreate(ctx, vm)...)
+	fieldErrs = append(fieldErrs, v.validateAnnotation(ctx, vm)...)
 
 	validationErrs := make([]string, 0, len(fieldErrs))
 	for _, fieldErr := range fieldErrs {
@@ -168,6 +170,7 @@ func (v validator) ValidateUpdate(ctx *context.WebhookRequestContext) admission.
 	fieldErrs = append(fieldErrs, v.validateAdvanced(ctx, vm)...)
 	fieldErrs = append(fieldErrs, v.validateInstanceStorageVolumes(ctx, vm, oldVM)...)
 	fieldErrs = append(fieldErrs, v.validateNextRestartTimeOnUpdate(ctx, vm, oldVM)...)
+	fieldErrs = append(fieldErrs, v.validateAnnotation(ctx, vm)...)
 
 	validationErrs := make([]string, 0, len(fieldErrs))
 	for _, fieldErr := range fieldErrs {
@@ -679,3 +682,17 @@ func (v validator) isNetworkRestrictedForReadinessProbe(ctx *context.WebhookRequ
 
 	return configMap.Data[isRestrictedNetworkKey] == "true", nil
 }
+
+func (v validator) validateAnnotation(ctx *context.WebhookRequestContext, vm *vmopv1.VirtualMachine) field.ErrorList {
+	var allErrs field.ErrorList
+	annotationPath := field.NewPath("metadata", "annotations")
+	forbiddenAnnotationKey := vmopv1.InstanceIDAnnotation
+
+	if vm.Annotations != nil {
+		if _, exists := vm.Annotations[forbiddenAnnotationKey]; exists {
+			allErrs = append(allErrs, field.Forbidden(annotationPath.Child(forbiddenAnnotationKey), settingInstanceIDAnnotationNotAllowed))
+		}
+	}
+
+	return allErrs
+}

webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go

@@ -107,6 +107,7 @@ func unitTestsValidateCreate() {
 		isBootstrapVAppConfigInline       bool
 		powerState                        vmopv1.VirtualMachinePowerState
 		nextRestartTime                   string
+		isForbiddenAnnotation             bool
 	}
 
 	validateCreate := func(args createArgs, expectedAllowed bool, expectedReason string, expectedErr error) {
@@ -250,6 +251,10 @@ func unitTestsValidateCreate() {
 			}
 		}
 
+		if args.isForbiddenAnnotation {
+			ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = "some-value"
+		}
+
 		ctx.vm.Spec.PowerState = args.powerState
 		ctx.vm.Spec.NextRestartTime = args.nextRestartTime
 
@@ -281,6 +286,7 @@ func unitTestsValidateCreate() {
 	volPath := specPath.Child("volumes")
 	nextRestartTimePath := specPath.Child("nextRestartTime")
 	now := time.Now().UTC()
+	annotationPath := field.NewPath("metadata", "annotations")
 
 	DescribeTable("create table", validateCreate,
 		Entry("should allow valid", createArgs{}, true, nil, nil),
@@ -362,6 +368,8 @@ func unitTestsValidateCreate() {
 		Entry("should disallow creating VM with non-empty, invalid nextRestartTime value",
 			createArgs{nextRestartTime: "hello"}, false,
 			field.Invalid(nextRestartTimePath, "hello", "cannot restart VM on create").Error(), nil),
+		Entry("should deny instance-id annotation set by user", createArgs{isForbiddenAnnotation: true}, false,
+			field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "adding instance-id annotation is not allowed").Error(), nil),
 	)
 }
 
@@ -387,6 +395,7 @@ func unitTestsValidateUpdate() {
 		newPowerStateEmptyAllowed   bool
 		nextRestartTime             string
 		lastRestartTime             string
+		isForbiddenAnnotation       bool
 	}
 
 	validateUpdate := func(args updateArgs, expectedAllowed bool, expectedReason string, expectedErr error) {
@@ -440,6 +449,10 @@ func unitTestsValidateUpdate() {
 			ctx.vm.Spec.PowerState = args.newPowerState
 		}
 
+		if args.isForbiddenAnnotation {
+			ctx.vm.Annotations[vmopv1.InstanceIDAnnotation] = "some-value"
+		}
+
 		ctx.oldVM.Spec.NextRestartTime = args.lastRestartTime
 		ctx.vm.Spec.NextRestartTime = args.nextRestartTime
 
@@ -473,6 +486,7 @@ func unitTestsValidateUpdate() {
 	volumesPath := field.NewPath("spec", "volumes")
 	powerStatePath := field.NewPath("spec", "powerState")
 	nextRestartTimePath := field.NewPath("spec", "nextRestartTime")
+	annotationPath := field.NewPath("metadata", "annotations")
 
 	DescribeTable("update table", validateUpdate,
 		Entry("should allow", updateArgs{}, true, nil, nil),
@@ -517,6 +531,8 @@ func unitTestsValidateUpdate() {
 		Entry("should disallow updating VM with non-empty, invalid nextRestartTime value ",
 			updateArgs{nextRestartTime: "hello"}, false,
 			field.Invalid(nextRestartTimePath, "hello", "must be formatted as RFC3339Nano").Error(), nil),
+		Entry("should deny instance-id annotation set by user", updateArgs{isForbiddenAnnotation: true}, false,
+			field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "adding instance-id annotation is not allowed").Error(), nil),
 	)
 
 	When("the update is performed while object deletion", func() {