diff --git a/controllers/virtualmachine/v1alpha1/virtualmachine_controller.go b/controllers/virtualmachine/v1alpha1/virtualmachine_controller.go index 1c8f6e79f..a62407a7c 100644 --- a/controllers/virtualmachine/v1alpha1/virtualmachine_controller.go +++ b/controllers/virtualmachine/v1alpha1/virtualmachine_controller.go @@ -474,8 +474,7 @@ func (r *Reconciler) ReconcileNormal(ctx *context.VirtualMachineContext) (reterr // Back up this VM if ReconcileNormal succeeds and the FSS is enabled. if lib.IsVMServiceBackupRestoreFSSEnabled() { - err := r.VMProvider.BackupVirtualMachine(ctx, ctx.VM) - if err != nil { + if err := r.VMProvider.BackupVirtualMachine(ctx, ctx.VM); err != nil { ctx.Logger.Error(err, "Failed to backup VirtualMachine") r.Recorder.EmitEvent(ctx.VM, "Backup", err, false) return err diff --git a/controllers/virtualmachine/v1alpha1/virtualmachine_controller_unit_test.go b/controllers/virtualmachine/v1alpha1/virtualmachine_controller_unit_test.go index 4e8644b6c..363623014 100644 --- a/controllers/virtualmachine/v1alpha1/virtualmachine_controller_unit_test.go +++ b/controllers/virtualmachine/v1alpha1/virtualmachine_controller_unit_test.go @@ -7,6 +7,7 @@ import ( "context" "errors" "strings" + "sync/atomic" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -158,18 +159,24 @@ func unitTestsReconcile() { When("The VM Service Backup and Restore FSS is enabled", func() { var ( - originalIsVMServiceBackupRestoreFSSEnabled func() bool + // This is manipulated atomically to avoid races where the controller + // is trying to read this _while_ the tests are updating it. + vmServiceBackupRestoreFSS uint32 + origVMServiceBackupRestoreFSS uint32 ) + // Modify the helper function to return the custom value of the FSS. + lib.IsVMServiceBackupRestoreFSSEnabled = func() bool { + return atomic.LoadUint32(&vmServiceBackupRestoreFSS) != 0 + } + BeforeEach(func() { - originalIsVMServiceBackupRestoreFSSEnabled = lib.IsVMServiceBackupRestoreFSSEnabled - lib.IsVMServiceBackupRestoreFSSEnabled = func() bool { - return true - } + origVMServiceBackupRestoreFSS = vmServiceBackupRestoreFSS + atomic.StoreUint32(&vmServiceBackupRestoreFSS, 1) }) AfterEach(func() { - lib.IsVMServiceBackupRestoreFSSEnabled = originalIsVMServiceBackupRestoreFSSEnabled + atomic.StoreUint32(&vmServiceBackupRestoreFSS, origVMServiceBackupRestoreFSS) }) It("Should call backup Virtual Machine if ReconcileNormal succeeds", func() { diff --git a/pkg/util/enc.go b/pkg/util/enc.go index d6e8f382c..1098483b4 100644 --- a/pkg/util/enc.go +++ b/pkg/util/enc.go @@ -64,9 +64,6 @@ func EncodeGzipBase64(s string) (string, error) { if _, err := zw.Write([]byte(s)); err != nil { return "", err } - if err := zw.Flush(); err != nil { - return "", err - } if err := zw.Close(); err != nil { return "", err } diff --git a/pkg/util/enc_test.go b/pkg/util/enc_test.go index 3df498148..938b729c0 100644 --- a/pkg/util/enc_test.go +++ b/pkg/util/enc_test.go @@ -7,6 +7,7 @@ import ( "bytes" "compress/gzip" "encoding/base64" + "io/ioutil" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -159,7 +160,18 @@ var _ = Describe("EncodeGzipBase64", func() { It("Encodes a string correctly", func() { input := "HelloWorld" output, err := util.EncodeGzipBase64(input) - Expect(err).ShouldNot(HaveOccurred()) - Expect(output).To(Equal("H4sIAAAAAAAA//JIzcnJD88vykkBAAAA//8BAAD//3kMd3cKAAAA")) + Expect(err).NotTo(HaveOccurred()) + // Decode the base64 output. + decoded, err := base64.StdEncoding.DecodeString(output) + Expect(err).NotTo(HaveOccurred()) + // Ungzip the decoded output. + reader := bytes.NewReader(decoded) + gzipReader, err := gzip.NewReader(reader) + Expect(err).NotTo(HaveOccurred()) + defer Expect(gzipReader.Close()).To(Succeed()) + + ungzipped, err := ioutil.ReadAll(gzipReader) + Expect(err).NotTo(HaveOccurred()) + Expect(input).Should(Equal(string(ungzipped))) }) }) diff --git a/pkg/vmprovider/providers/vsphere/virtualmachine/backup.go b/pkg/vmprovider/providers/vsphere/virtualmachine/backup.go index 6c897e3df..72168310d 100644 --- a/pkg/vmprovider/providers/vsphere/virtualmachine/backup.go +++ b/pkg/vmprovider/providers/vsphere/virtualmachine/backup.go @@ -4,21 +4,22 @@ package virtualmachine import ( - goctx "context" - - "github.com/vmware/govmomi/object" - "github.com/vmware/govmomi/vim25/types" corev1 "k8s.io/api/core/v1" ctrlruntime "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" + "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/vim25/types" + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/util" "github.com/vmware-tanzu/vm-operator/pkg/vmprovider/providers/vsphere/constants" ) -// BackupVirtualMachine backs up the VM's kube data and bootstrap data into the VM's ExtraConfig. +// BackupVirtualMachine backs up the VM's kube data and bootstrap data into the +// vSphere VM's ExtraConfig. The status fields will not be backed up, as we expect +// to recreate and reconcile these resources during the restore process. func BackupVirtualMachine( vmCtx context.VirtualMachineContext, vcVM *object.VirtualMachine, @@ -29,7 +30,7 @@ func BackupVirtualMachine( return err } - vmBootstrapData, err := getEncodedVMBootstrapData(vmCtx.Context, vmCtx.VM, k8sClient) + vmBootstrapData, err := getEncodedVMBootstrapData(vmCtx, k8sClient) if err != nil { vmCtx.Logger.Error(err, "Failed to get VM bootstrap data for backup") return err @@ -59,7 +60,6 @@ func BackupVirtualMachine( func getEncodedVMKubeData(vm *vmopv1.VirtualMachine) (string, error) { backupVM := vm.DeepCopy() - // No need to back up status fields as we expect the VM to be re-created during restore. backupVM.Status = vmopv1.VirtualMachineStatus{} backupYaml, err := yaml.Marshal(backupVM) if err != nil { @@ -70,20 +70,22 @@ func getEncodedVMKubeData(vm *vmopv1.VirtualMachine) (string, error) { } func getEncodedVMBootstrapData( - ctx goctx.Context, - vm *vmopv1.VirtualMachine, + vmCtx context.VirtualMachineContext, k8sClient ctrlruntime.Client) (string, error) { - // Return early if the VM bootstrap data is not set. - if vm.Spec.VmMetadata == nil { + k8sVM := vmCtx.VM + if k8sVM.Spec.VmMetadata == nil { return "", nil } var bootstrapObj ctrlruntime.Object - if cmName := vm.Spec.VmMetadata.ConfigMapName; cmName != "" { + if cmName := k8sVM.Spec.VmMetadata.ConfigMapName; cmName != "" { cm := &corev1.ConfigMap{} - cmKey := ctrlruntime.ObjectKey{Name: cmName, Namespace: vm.Namespace} - if err := k8sClient.Get(ctx, cmKey, cm); err != nil { + cmKey := ctrlruntime.ObjectKey{ + Name: cmName, + Namespace: k8sVM.Namespace, + } + if err := k8sClient.Get(vmCtx.Context, cmKey, cm); err != nil { return "", err } cm.APIVersion = "v1" @@ -91,10 +93,13 @@ func getEncodedVMBootstrapData( bootstrapObj = cm } - if secretName := vm.Spec.VmMetadata.SecretName; secretName != "" { + if secretName := k8sVM.Spec.VmMetadata.SecretName; secretName != "" { secret := &corev1.Secret{} - secretKey := ctrlruntime.ObjectKey{Name: secretName, Namespace: vm.Namespace} - if err := k8sClient.Get(ctx, secretKey, secret); err != nil { + secretKey := ctrlruntime.ObjectKey{ + Name: secretName, + Namespace: k8sVM.Namespace, + } + if err := k8sClient.Get(vmCtx.Context, secretKey, secret); err != nil { return "", err } secret.APIVersion = "v1" diff --git a/pkg/vmprovider/providers/vsphere/virtualmachine/backup_test.go b/pkg/vmprovider/providers/vsphere/virtualmachine/backup_test.go index fea89a784..51a5fffd7 100644 --- a/pkg/vmprovider/providers/vsphere/virtualmachine/backup_test.go +++ b/pkg/vmprovider/providers/vsphere/virtualmachine/backup_test.go @@ -4,15 +4,16 @@ package virtualmachine_test import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - "github.com/vmware/govmomi/object" - "github.com/vmware/govmomi/vim25/mo" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrlruntime "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/vim25/mo" + vmopv1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" "github.com/vmware-tanzu/vm-operator/pkg/context" "github.com/vmware-tanzu/vm-operator/pkg/util"