Skip to content

Commit

Permalink
Brownfield fix to pass instanceID to CloudInit
Browse files Browse the repository at this point in the history
  • Loading branch information
@zyiyi11
zyiyi11 committed Sep 14, 2023
1 parent d6a76ed commit 4c2198d
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 17 deletions.
11 changes: 10 additions & 1 deletion api/v1alpha1/virtualmachine_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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. This annotation is used in upgrade script.
InstanceIDAnnotation = GroupName + "/cloud-init-instance-id"
)

// VirtualMachinePort is unused and can be considered deprecated.
Expand Down
9 changes: 9 additions & 0 deletions api/v1alpha2/virtualmachine_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. This annotation is used in upgrade script.
InstanceIDAnnotation = GroupName + "/cloud-init-instance-id"
)

// VirtualMachinePowerState defines a VM's desired and observed power states.
Expand Down
6 changes: 6 additions & 0 deletions pkg/vmprovider/providers/vsphere/session/session_vm_customization.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ func GetCloudInitMetadata(vm *vmopv1.VirtualMachine,
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 in brownfield scenario.
if value, ok := vm.Annotations[vmopv1.InstanceIDAnnotation]; ok {
metadataObj.InstanceID = value
}

metadataBytes, err := yaml.Marshal(metadataObj)
if err != nil {
Expand Down
18 changes: 2 additions & 16 deletions pkg/vmprovider/providers/vsphere/vmprovider_vm_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1498,7 +1498,7 @@ func vmTests() {
Context("Cloudinint Transport without userdata", func() {
var ec map[string]interface{}

JustBeforeEach(func() {
It("Metadata data is included in ExtraConfig", func() {
vm.Spec.VmMetadata = &vmopv1.VirtualMachineMetadata{
Transport: vmopv1.VirtualMachineMetadataCloudInitTransport,
}
Expand All @@ -1514,13 +1514,6 @@ func vmTests() {
ec[val.Key] = val.Value.(string)
}
}
})

AfterEach(func() {
ec = nil
})

It("Metadata data is included in ExtraConfig", func() {
By("Should include default keys and values", func() {
Expect(ec).To(HaveKeyWithValue("disk.enableUUID", "TRUE"))
Expect(ec).To(HaveKeyWithValue("vmware.tools.gosc.ignoretoolscheck", "TRUE"))
Expand All @@ -1537,7 +1530,7 @@ func vmTests() {
Context("Cloudinint Transport with userdata", func() {
var ec map[string]interface{}

JustBeforeEach(func() {
It("Metadata data is included in ExtraConfig", func() {
configMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "md-configmap-",
Expand Down Expand Up @@ -1566,13 +1559,6 @@ func vmTests() {
ec[val.Key] = val.Value.(string)
}
}
})

AfterEach(func() {
ec = nil
})

It("Metadata data is included in ExtraConfig", func() {
By("Should include default keys and values", func() {
Expect(ec).To(HaveKeyWithValue("disk.enableUUID", "TRUE"))
Expect(ec).To(HaveKeyWithValue("vmware.tools.gosc.ignoretoolscheck", "TRUE"))
Expand Down
19 changes: 19 additions & 0 deletions webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
settingAnnotationNotAllowed = "adding this 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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -738,3 +741,19 @@ 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

if ctx.IsPrivilegedAccount || vm.Annotations == nil {
return allErrs
}

annotationPath := field.NewPath("metadata", "annotations")

if _, exists := vm.Annotations[vmopv1.InstanceIDAnnotation]; exists {
allErrs = append(allErrs, field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), settingAnnotationNotAllowed))
}

return allErrs
}
17 changes: 17 additions & 0 deletions webhooks/virtualmachine/v1alpha1/validation/virtualmachine_validator_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -506,6 +512,9 @@ 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 cloud-init-instance-id annotation set by SSO user", createArgs{isForbiddenAnnotation: true}, false,
field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "adding this annotation is not allowed").Error(), nil),
Entry("should allow cloud-init-instance-id annotation set by service user", createArgs{isServiceUser: true, isForbiddenAnnotation: true}, true, nil, nil),
)
}

Expand Down Expand Up @@ -533,6 +542,7 @@ func unitTestsValidateUpdate() {
newPowerStateEmptyAllowed bool
nextRestartTime string
lastRestartTime string
isForbiddenAnnotation bool
}

validateUpdate := func(args updateArgs, expectedAllowed bool, expectedReason string, expectedErr error) {
Expand Down Expand Up @@ -581,6 +591,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,
Expand Down Expand Up @@ -621,6 +634,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
Expand Down Expand Up @@ -672,6 +686,9 @@ 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 cloud-init-instance-id annotation set by SSO user", updateArgs{isForbiddenAnnotation: true}, false,
field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "adding this annotation is not allowed").Error(), nil),
Entry("should allow cloud-init-instance-id annotation set by service user", updateArgs{isServiceUser: true, isForbiddenAnnotation: true}, true, nil, nil),
)

When("the update is performed while object deletion", func() {
Expand Down
19 changes: 19 additions & 0 deletions webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
settingAnnotationNotAllowed = "adding this 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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -679,3 +682,19 @@ 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

if ctx.IsPrivilegedAccount || vm.Annotations == nil {
return allErrs
}

annotationPath := field.NewPath("metadata", "annotations")

if _, exists := vm.Annotations[vmopv1.InstanceIDAnnotation]; exists {
allErrs = append(allErrs, field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), settingAnnotationNotAllowed))
}

return allErrs
}
18 changes: 18 additions & 0 deletions webhooks/virtualmachine/v1alpha2/validation/virtualmachine_validator_unit_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -362,6 +368,9 @@ 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 cloud-init-instance-id annotation set by user", createArgs{isForbiddenAnnotation: true}, false,
field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "adding this annotation is not allowed").Error(), nil),
Entry("should allow cloud-init-instance-id annotation set by service user", createArgs{isServiceUser: true, isForbiddenAnnotation: true}, true, nil, nil),
)
}

Expand All @@ -387,6 +396,7 @@ func unitTestsValidateUpdate() {
newPowerStateEmptyAllowed bool
nextRestartTime string
lastRestartTime string
isForbiddenAnnotation bool
}

validateUpdate := func(args updateArgs, expectedAllowed bool, expectedReason string, expectedErr error) {
Expand Down Expand Up @@ -440,6 +450,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

Expand Down Expand Up @@ -473,6 +487,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),
Expand Down Expand Up @@ -517,6 +532,9 @@ 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 cloud-init-instance-id annotation set by user", updateArgs{isForbiddenAnnotation: true}, false,
field.Forbidden(annotationPath.Child(vmopv1.InstanceIDAnnotation), "adding this annotation is not allowed").Error(), nil),
Entry("should allow cloud-init-instance-id annotation set by service user", updateArgs{isServiceUser: true, isForbiddenAnnotation: true}, true, nil, nil),
)

When("the update is performed while object deletion", func() {
Expand Down

0 comments on commit 4c2198d

Please sign in to comment.