Revert "Adds new condition to expose state of guestinfo"

Revert "Adds new condition to expose state of guestinfo"

api/v1alpha1/condition_consts.go

@@ -11,30 +11,6 @@ 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,32 +10,6 @@ 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,7 +9,6 @@ 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,8 +54,6 @@ 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,8 +4,6 @@
 package constants
 
 import (
-	"github.com/alecthomas/units"
-
 	"github.com/vmware-tanzu/vm-operator/pkg"
 )
 
@@ -74,12 +72,6 @@ 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,7 +17,6 @@ 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"
@@ -115,27 +114,17 @@ 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, *ErrWithReason) {
+	updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, *vimTypes.CustomizationSpec, error) {
 
 	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, &ErrWithReason{
-				error:           fmt.Errorf("decoding cloud-init prep userdata failed %v", err),
-				conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason,
-			}
+			return nil, nil, fmt.Errorf("decoding cloud-init prep userdata failed %v", err)
 		}
 		userdata = plainText
 	}
@@ -158,27 +147,17 @@ func GetCloudInitPrepCustSpec(
 func GetCloudInitGuestInfoCustSpec(
 	cloudInitMetadata string,
 	config *vimTypes.VirtualMachineConfigInfo,
-	updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, *ErrWithReason) {
+	updateArgs VMUpdateArgs) (*vimTypes.VirtualMachineConfigSpec, error) {
 
 	extraConfig := map[string]string{}
 
 	encodedMetadata, err := EncodeGzipBase64(cloudInitMetadata)
 	if err != nil {
-		return nil, &ErrWithReason{
-			error:           fmt.Errorf("encoding cloud-init metadata failed %v", err),
-			conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason,
-		}
+		return nil, fmt.Errorf("encoding cloud-init metadata failed %v", err)
 	}
 	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,
@@ -194,30 +173,16 @@ func GetCloudInitGuestInfoCustSpec(
 		// Ensure the data is normalized first to plain-text.
 		plainText, err := util.TryToDecodeBase64Gzip([]byte(data))
 		if err != nil {
-			return nil, &ErrWithReason{
-				error:           fmt.Errorf("decoding cloud-init userdata failed %v", err),
-				conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason,
-			}
+			return nil, fmt.Errorf("decoding cloud-init userdata failed %v", err)
 		}
 
 		encodedUserdata, err := EncodeGzipBase64(plainText)
 		if err != nil {
-			return nil, &ErrWithReason{
-				error:           fmt.Errorf("encoding cloud-init userdata failed %v", err),
-				conditionReason: vmopv1.VirtualMachineMetadataFormatInvalidReason,
-			}
+			return nil, fmt.Errorf("encoding cloud-init userdata failed %v", err)
 		}
 
 		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{}
@@ -280,11 +245,6 @@ 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
 	}
 
@@ -301,12 +261,6 @@ func customizeCloudInit(
 	}
 
 	if err != nil {
-		errWithReason := err.(ErrWithReason)
-		conditions.MarkFalse(vmCtx.VM,
-			vmopv1.VirtualMachineMetadataReadyCondition,
-			errWithReason.conditionReason,
-			vmopv1.ConditionSeverityError,
-			errWithReason.Error())
 		return nil, nil, err
 	}
 
@@ -353,7 +307,6 @@ 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,7 +7,6 @@ import (
 	goctx "context"
 	"encoding/base64"
 	"fmt"
-	"math/rand"
 	"strings"
 
 	. "github.com/onsi/ginkgo"
@@ -403,59 +402,6 @@ 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

@@ -229,25 +229,14 @@ 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 {
-			conditions.MarkFalse(vmCtx.VM,
-				vmopv1.VirtualMachineMetadataReadyCondition,
-				vmopv1.VirtualMachineMetadataNotFoundReason,
-				vmopv1.ConditionSeverityError,
-				"Unable to get metadata ConfigMap %s/%s", metadata.ConfigMapName, vmCtx.VM.Namespace)
+			// TODO: Condition
 			return vmMD, errors.Wrap(err, "Failed to get VM Metadata ConfigMap")
 		}
 
@@ -257,11 +246,7 @@ func GetVMMetadata(
 		secret := &corev1.Secret{}
 		err := k8sClient.Get(vmCtx, ctrlclient.ObjectKey{Name: metadata.SecretName, Namespace: vmCtx.VM.Namespace}, secret)
 		if err != nil {
-			conditions.MarkFalse(vmCtx.VM,
-				vmopv1.VirtualMachineMetadataReadyCondition,
-				vmopv1.VirtualMachineMetadataNotFoundReason,
-				vmopv1.ConditionSeverityError,
-				"Unable to get metadata Secret %s/%s", metadata.SecretName, vmCtx.VM.Namespace)
+			// TODO: Condition
 			return vmMD, errors.Wrap(err, "Failed to get VM Metadata Secret")
 		}