Adds new condition to expose state of guestinfo

Signed-off-by: Sagar Muchhal <[email protected]>

Author: srm09

api/v1alpha1/condition_consts.go

@@ -11,6 +11,30 @@ const (
 
 // Conditions and condition Reasons for the VirtualMachine object.
 
+const (
+	// VirtualMachineMetadataReadyCondition documents the state of the metadata passed to the VirtualMachine.
+	VirtualMachineMetadataReadyCondition ConditionType = "VirtualMachineMetadataReady"
+
+	// VirtualMachineMetadataDuplicatedReason (Severity=Error) documents that the mutually exclusive Secret and ConfigMap are
+	// specified in the VirtualMachineMetadata.
+	//
+	// This validation is performed by the validation webhook, defensively adding the reason.
+	VirtualMachineMetadataDuplicatedReason = "VirtualMachineMetadataDuplicated"
+
+	// VirtualMachineMetadataNotFoundReason (Severity=Error) documents that the Secret or ConfigMap specified in the VirtualMachineSpec
+	// cannot be found.
+	VirtualMachineMetadataNotFoundReason = "VirtualMachineMetadataNotFound"
+
+	// VirtualMachineMetadataFormatInvalidReason (Severity=Error) documents that the format of the supplied metadata is invalid.
+	//
+	// It is used to indicate issues with the formatting of the metadata irrespective of the Transport in use.
+	VirtualMachineMetadataFormatInvalidReason = "VirtualMachineMetadataFormatInvalid"
+
+	// VirtualMachineGuestInfoSizeExceededReason (Severity=Error) documents that the supplied Cloud Init metadata exceeds the
+	// maximum allowed capacity.
+	VirtualMachineGuestInfoSizeExceededReason = "VirtualMachineGuestInfoSizeExceeded"
+)
+
 const (
 	// VirtualMachinePrereqReadyCondition documents that all of a VirtualMachine's prerequisites declared in the spec
 	// (e.g. VirtualMachineClass) are satisfied.

api/v1alpha2/virtualmachine_types.go

@@ -10,6 +10,32 @@ import (
 	"github.com/vmware-tanzu/vm-operator/api/v1alpha2/common"
 )
 
+// TODO: Since this API version replaces the concept of metadata to BootstrapSpec, rename the types once
+// these reasons before using them in Conditions. Keeping the reasons the same for the sake of consistency.
+const (
+	// VirtualMachineMetadataReadyCondition documents the state of the metadata passed to the VirtualMachine.
+	VirtualMachineMetadataReadyCondition = "VirtualMachineMetadataReady"
+
+	// VirtualMachineMetadataDuplicatedReason (Severity=Error) documents that the mutually exclusive Secret and ConfigMap are
+	// specified in the VirtualMachineMetadata.
+	//
+	// This validation is performed by the validation webhook, defensively adding the reason.
+	VirtualMachineMetadataDuplicatedReason = "VirtualMachineMetadataDuplicated"
+
+	// VirtualMachineMetadataNotFoundReason (Severity=Error) documents that the Secret or ConfigMap specified in the VirtualMachineSpec
+	// cannot be found.
+	VirtualMachineMetadataNotFoundReason = "VirtualMachineMetadataNotFound"
+
+	// VirtualMachineMetadataFormatInvalidReason (Severity=Error) documents that the format of the supplied metadata is invalid.
+	//
+	// It is used to indicate issues with the formatting of the metadata irrespective of the Transport in use.
+	VirtualMachineMetadataFormatInvalidReason = "VirtualMachineMetadataFormatInvalid"
+
+	// VirtualMachineGuestInfoSizeExceededReason (Severity=Error) documents that the supplied Cloud Init metadata exceeds the
+	// maximum allowed capacity.
+	VirtualMachineGuestInfoSizeExceededReason = "VirtualMachineGuestInfoSizeExceeded"
+)
+
 const (
 	// VirtualMachineConditionClassReady indicates that a referenced
 	// VirtualMachineClass is ready.

go.mod

@@ -9,6 +9,7 @@ replace (
 )
 
 require (
+	github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137
 	github.com/davecgh/go-spew v1.1.1
 	github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1
 	github.com/go-logr/logr v1.2.3

go.sum

@@ -54,6 +54,8 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho=
+github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137 h1:s6gZFSlWYmbqAuRjVTiNNhvNRfY2Wxp9nhfyel4rklc=
+github.com/alecthomas/units v0.0.0-20211218093645-b94a6e3cc137/go.mod h1:OMCwj8VM1Kc9e19TLln2VL61YJF0x1XFtfdL4JdbSyE=
 github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o=
 github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY=
 github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=

pkg/vmprovider/providers/vsphere/constants/constants.go

@@ -4,6 +4,8 @@
 package constants
 
 import (
+	"github.com/alecthomas/units"
+
 	"github.com/vmware-tanzu/vm-operator/pkg"
 )
 
@@ -72,6 +74,12 @@ const (
 	CloudInitGuestInfoUserdata         = "guestinfo.userdata"
 	CloudInitGuestInfoUserdataEncoding = "guestinfo.userdata.encoding"
 
+	// CloudInitGuestInfoMaxSize the maximum allowed size for CloudInit metadata.
+	// As defined in the https://github.com/vmware/open-vm-tools/blob/42ce53f39be45b7795a9f1adf892af84629b4a19/open-vm-tools/lib/include/guest_msg_def.h#L100-L104
+	//
+	// The human-readable value for the constant equates to 64 Kibibytes, or 64KiB.
+	CloudInitGuestInfoMaxSize = 64 * units.KiB
+
 	// InstanceStoragePVCNamePrefix prefix of auto-generated PVC names.
 	InstanceStoragePVCNamePrefix = "instance-pvc-"
 	// InstanceStorageLabelKey identifies resources related to instance storage.

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

@@ -17,6 +17,7 @@ import (
 	apiEquality "k8s.io/apimachinery/pkg/api/equality"
 
 	vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1"
+	"github.com/vmware-tanzu/vm-operator/pkg/conditions"
 	"github.com/vmware-tanzu/vm-operator/pkg/context"
 	"github.com/vmware-tanzu/vm-operator/pkg/lib"
 	"github.com/vmware-tanzu/vm-operator/pkg/util"
@@ -114,17 +115,27 @@ func GetCloudInitMetadata(vm *vmopv1.VirtualMachine,
 	return string(metadataBytes), nil
 }
 
+// ErrWithReason encapsulates the error type as well as a human-readable reason that will be
+// used to set the Reason field on the Condition set on the object.
+type ErrWithReason struct {
+	error
+	conditionReason string
+}
+
 func GetCloudInitPrepCustSpec(
 	cloudInitMetadata string,
-	updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, *vimTypes.CustomizationSpec, error) {
+	updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, *vimTypes.CustomizationSpec, *ErrWithReason) {
 
 	userdata := updateArgs.VMMetadata.Data["user-data"]
 
 	if userdata != "" {
 		// Ensure the data is normalized first to plain-text.
 		plainText, err := util.TryToDecodeBase64Gzip([]byte(userdata))
 		if err != nil {
-			return nil, nil, fmt.Errorf("decoding cloud-init prep userdata failed %v", err)
+			return nil, nil, &ErrWithReason{
+				error:           fmt.Errorf("decoding cloud-init prep userdata failed %v", err),
+				conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason,
+			}
 		}
 		userdata = plainText
 	}
@@ -147,17 +158,27 @@ func GetCloudInitPrepCustSpec(
 func GetCloudInitGuestInfoCustSpec(
 	cloudInitMetadata string,
 	config *vimTypes.VirtualMachineConfigInfo,
-	updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, error) {
+	updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, *ErrWithReason) {
 
 	extraConfig := map[string]string{}
 
 	encodedMetadata, err := EncodeGzipBase64(cloudInitMetadata)
 	if err != nil {
-		return nil, fmt.Errorf("encoding cloud-init metadata failed %v", err)
+		return nil, &ErrWithReason{
+			error:           fmt.Errorf("encoding cloud-init metadata failed %v", err),
+			conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason,
+		}
 	}
 	extraConfig[constants.CloudInitGuestInfoMetadata] = encodedMetadata
 	extraConfig[constants.CloudInitGuestInfoMetadataEncoding] = "gzip+base64"
 
+	if len(encodedMetadata) > int(constants.CloudInitGuestInfoMaxSize) {
+		return nil, &ErrWithReason{
+			error:           fmt.Errorf("size of cloud-init guest info exceeds the maximum allowed size of %s", constants.CloudInitGuestInfoMaxSize.String()),
+			conditionReason: vmopv1.VirtualMachineGuestInfoSizeExceededReason,
+		}
+	}
+
 	var data string
 	// Check for the 'user-data' key as per official contract and API documentation.
 	// Additionally, To support the cluster bootstrap data supplied by CAPBK's secret,
@@ -173,16 +194,30 @@ func GetCloudInitGuestInfoCustSpec(
 		// Ensure the data is normalized first to plain-text.
 		plainText, err := util.TryToDecodeBase64Gzip([]byte(data))
 		if err != nil {
-			return nil, fmt.Errorf("decoding cloud-init userdata failed %v", err)
+			return nil, &ErrWithReason{
+				error:           fmt.Errorf("decoding cloud-init userdata failed %v", err),
+				conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason,
+			}
 		}
 
 		encodedUserdata, err := EncodeGzipBase64(plainText)
 		if err != nil {
-			return nil, fmt.Errorf("encoding cloud-init userdata failed %v", err)
+			return nil, &ErrWithReason{
+				error:           fmt.Errorf("encoding cloud-init userdata failed %v", err),
+				conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason,
+			}
 		}
 
 		extraConfig[constants.CloudInitGuestInfoUserdata] = encodedUserdata
 		extraConfig[constants.CloudInitGuestInfoUserdataEncoding] = "gzip+base64"
+
+		if len(encodedUserdata) > int(constants.CloudInitGuestInfoMaxSize) ||
+			len(encodedUserdata)+len(encodedMetadata) > int(constants.CloudInitGuestInfoMaxSize) {
+			return nil, &ErrWithReason{
+				error:           fmt.Errorf("size of cloud-init guest info exceeds the maximum allowed size of %s", constants.CloudInitGuestInfoMaxSize.String()),
+				conditionReason: vmopv1.VirtualMachineGuestInfoSizeExceededReason,
+			}
+		}
 	}
 
 	configSpec := &vimTypes.VirtualMachineConfigSpec{}
@@ -245,6 +280,11 @@ func customizeCloudInit(
 
 	cloudInitMetadata, err := GetCloudInitMetadata(vmCtx.VM, netplan, updateArgs.VMMetadata.Data)
 	if err != nil {
+		conditions.MarkFalse(vmCtx.VM,
+			vmopv1.VirtualMachineMetadataReadyCondition,
+			vmopv1.VirtualMachineMetadataFormatInvalidReason,
+			vmopv1.ConditionSeverityError,
+			err.Error())
 		return nil, nil, err
 	}
 
@@ -261,6 +301,12 @@ func customizeCloudInit(
 	}
 
 	if err != nil {
+		errWithReason := err.(ErrWithReason)
+		conditions.MarkFalse(vmCtx.VM,
+			vmopv1.VirtualMachineMetadataReadyCondition,
+			errWithReason.conditionReason,
+			vmopv1.ConditionSeverityError,
+			errWithReason.Error())
 		return nil, nil, err
 	}
 
@@ -307,6 +353,7 @@ func (s *Session) customize(
 	if err != nil {
 		return err
 	}
+	conditions.MarkTrue(vmCtx.VM, vmopv1.VirtualMachineMetadataReadyCondition)
 
 	if configSpec != nil {
 		defaultConfigSpec := &vimTypes.VirtualMachineConfigSpec{}

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

@@ -7,6 +7,7 @@ import (
 	goctx "context"
 	"encoding/base64"
 	"fmt"
+	"math/rand"
 	"strings"
 
 	. "github.com/onsi/ginkgo"
@@ -402,6 +403,59 @@ var _ = Describe("Cloud-Init Customization", func() {
 			})
 		})
 
+		Context("With cloud init guest info exceeding the maximum size", func() {
+			generateDataFunc := func(length int) string {
+				charset := "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
+
+				sb := strings.Builder{}
+				sb.Grow(length)
+				for i := 0; i < length; i++ {
+					sb.WriteByte(charset[rand.Intn(len(charset))]) //nolint:gosec
+				}
+				return sb.String()
+			}
+
+			Context("With cloud init user data exceeding the maximum size", func() {
+				BeforeEach(func() {
+					// Doubling the input to the max allowed size to make sure the compressed and encoded
+					// output is still above the allowed limit.
+					data, err := session.EncodeGzipBase64(generateDataFunc(128 * 1000))
+					Expect(err).ToNot(HaveOccurred())
+					updateArgs.VMMetadata.Data["user-data"] = data
+				})
+				It("will return a limit exceeded error", func() {
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("exceeds the maximum allowed size of 64KiB"))
+				})
+			})
+
+			Context("With cloud init metadata exceeding the maximum size", func() {
+				BeforeEach(func() {
+					// Doubling the input to the max allowed size to make sure the compressed and encoded
+					// output is still above the allowed limit.
+					data, err := session.EncodeGzipBase64(generateDataFunc(128 * 1000))
+					Expect(err).ToNot(HaveOccurred())
+					cloudInitMetadata = data
+				})
+				It("will return a limit exceeded error", func() {
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("exceeds the maximum allowed size of 64KiB"))
+				})
+			})
+
+			Context("With cloud init metadata + userdata exceeding the maximum size", func() {
+				BeforeEach(func() {
+					data, err := session.EncodeGzipBase64(generateDataFunc(64 * 1000))
+					Expect(err).ToNot(HaveOccurred())
+					cloudInitMetadata = data
+					updateArgs.VMMetadata.Data["user-data"] = data
+				})
+				It("will return a limit exceeded error", func() {
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(ContainSubstring("exceeds the maximum allowed size of 64KiB"))
+				})
+			})
+		})
 	})
 
 	Context("GetCloudInitPrepCustSpec", func() {

pkg/vmprovider/providers/vsphere/vmprovider_vm_utils.go

@@ -228,14 +228,25 @@ func GetVMMetadata(
 	// The VM's MetaData ConfigMapName and SecretName are mutually exclusive. Validation webhook checks
 	// this during create/update but double check it here.
 	if metadata.ConfigMapName != "" && metadata.SecretName != "" {
+		conditions.MarkFalse(vmCtx.VM,
+			vmopv1.VirtualMachineMetadataReadyCondition,
+			vmopv1.VirtualMachineMetadataDuplicatedReason,
+			vmopv1.ConditionSeverityError,
+			"Both ConfigMapName %s/%s and SecretName %s/%s are specified",
+			metadata.ConfigMapName, vmCtx.VM.Namespace,
+			metadata.SecretName, vmCtx.VM.Namespace)
 		return vmMD, errors.New("invalid VM Metadata: both ConfigMapName and SecretName are specified")
 	}
 
 	if metadata.ConfigMapName != "" {
 		cm := &corev1.ConfigMap{}
 		err := k8sClient.Get(vmCtx, ctrlclient.ObjectKey{Name: metadata.ConfigMapName, Namespace: vmCtx.VM.Namespace}, cm)
 		if err != nil {
-			// TODO: Condition
+			conditions.MarkFalse(vmCtx.VM,
+				vmopv1.VirtualMachineMetadataReadyCondition,
+				vmopv1.VirtualMachineMetadataNotFoundReason,
+				vmopv1.ConditionSeverityError,
+				"Unable to get metadata ConfigMap %s/%s", metadata.ConfigMapName, vmCtx.VM.Namespace)
 			return vmMD, errors.Wrap(err, "Failed to get VM Metadata ConfigMap")
 		}
 
@@ -245,7 +256,11 @@ func GetVMMetadata(
 		secret := &corev1.Secret{}
 		err := k8sClient.Get(vmCtx, ctrlclient.ObjectKey{Name: metadata.SecretName, Namespace: vmCtx.VM.Namespace}, secret)
 		if err != nil {
-			// TODO: Condition
+			conditions.MarkFalse(vmCtx.VM,
+				vmopv1.VirtualMachineMetadataReadyCondition,
+				vmopv1.VirtualMachineMetadataNotFoundReason,
+				vmopv1.ConditionSeverityError,
+				"Unable to get metadata Secret %s/%s", metadata.SecretName, vmCtx.VM.Namespace)
 			return vmMD, errors.Wrap(err, "Failed to get VM Metadata Secret")
 		}