[specific ci=Group11-Upgrade] Add UpgradeInProgress flag for VCH upgrade

cmd/vic-machine/inspect/inspect.go

@@ -168,13 +168,13 @@ func (i *Inspect) upgradeStatusMessage(ctx context.Context, vch *vm.VirtualMachi
 		return
 	}
 
-	upgrading, _, err := vch.UpgradeInProgress(ctx, management.UpgradePrefix)
+	upgrading, err := vch.VCHUpdateStatus(ctx)
 	if err != nil {
-		log.Errorf("Unable to determine if upgrade is in progress: %s", err)
+		log.Errorf("Unable to determine if upgrade/configure is in progress: %s", err)
 		return
 	}
 	if upgrading {
-		log.Info("Upgrade in progress")
+		log.Info("Upgrade/configure in progress")
 		return
 	}
 

cmd/vic-machine/list/list.go

@@ -192,7 +192,7 @@ func (l *List) upgradeStatusMessage(ctx context.Context, vch *vm.VirtualMachine,
 		return "Up to date"
 	}
 
-	upgrading, _, err := vch.UpgradeInProgress(ctx, management.UpgradePrefix)
+	upgrading, err := vch.VCHUpdateStatus(ctx)
 	if err != nil {
 		return fmt.Sprintf("Unknown: %s", err)
 	}

cmd/vic-machine/upgrade/upgrade.go

@@ -64,6 +64,11 @@ func (u *Upgrade) Flags() []cli.Flag {
 			Usage:       "Roll back VCH version to before the previous upgrade",
 			Destination: &u.Rollback,
 		},
+		cli.BoolFlag{
+			Name:        "resetInProgressFlag",
+			Usage:       "Reset the UpdateInProgress flag. Warning: Do not reset this flag if another upgrade/configure process is running",
+			Destination: &u.ResetInProgressFlag,
+		},
 	}
 
 	target := u.TargetFlags()
@@ -148,6 +153,41 @@ func (u *Upgrade) Run(clic *cli.Context) (err error) {
 	log.Infof("")
 	log.Infof("VCH ID: %s", vch.Reference().String())
 
+	if u.ResetInProgressFlag {
+		if err = vch.SetVCHUpdateStatus(ctx, false); err != nil {
+			log.Error("Failed to reset UpdateInProgress flag")
+			log.Error(err)
+			return errors.New("upgrade failed")
+		}
+		log.Infof("Reset UpdateInProgress flag successfully")
+		return nil
+	}
+
+	upgrading, err := vch.VCHUpdateStatus(ctx)
+	if err != nil {
+		log.Error("Unable to determine if upgrade/configure is in progress")
+		log.Error(err)
+		return errors.New("upgrade failed")
+	}
+	if upgrading {
+		log.Error("Upgrade failed: another upgrade/configure operation is in progress")
+		log.Error("If no other upgrade/configure process is running, use --resetInProgressFlag to reset the VCH upgrade/configure status")
+		return errors.New("upgrade failed")
+	}
+
+	if err = vch.SetVCHUpdateStatus(ctx, true); err != nil {
+		log.Error("Failed to set UpdateInProgress flag to true")
+		log.Error(err)
+		return errors.New("upgrade failed")
+	}
+
+	defer func() {
+		if err = vch.SetVCHUpdateStatus(ctx, false); err != nil {
+			log.Error("Failed to reset UpdateInProgress")
+			log.Error(err)
+		}
+	}()
+
 	vchConfig, err := executor.FetchAndMigrateVCHConfig(vch)
 	if err != nil {
 		log.Error("Failed to get Virtual Container Host configuration")

lib/install/data/data.go

@@ -76,8 +76,9 @@ type Data struct {
 
 	Timeout time.Duration
 
-	Force bool
-	UseRP bool
+	Force               bool
+	UseRP               bool
+	ResetInProgressFlag bool
 
 	AsymmetricRouting bool
 

pkg/vsphere/vm/vm.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"net/url"
 	"path"
+	"strconv"
 	"strings"
 	"sync/atomic"
 
@@ -32,10 +33,13 @@ import (
 	"github.com/vmware/govmomi/vim25/mo"
 	"github.com/vmware/govmomi/vim25/types"
 
+	"github.com/vmware/vic/pkg/vsphere/extraconfig/vmomi"
 	"github.com/vmware/vic/pkg/vsphere/session"
 	"github.com/vmware/vic/pkg/vsphere/tasks"
 )
 
+const UpdateStatus = "UpdateInProgress"
+
 type InvalidState struct {
 	r types.ManagedObjectReference
 }
@@ -359,20 +363,6 @@ func IsUpgradeSnapshot(node *types.VirtualMachineSnapshotTree, upgradePrefix str
 	return node != nil && strings.HasPrefix(node.Name, upgradePrefix)
 }
 
-// UpgradeInProgress tells if an upgrade has already been started based on snapshot name beginning with upgradePrefix
-func (vm *VirtualMachine) UpgradeInProgress(ctx context.Context, upgradePrefix string) (bool, string, error) {
-	node, err := vm.GetCurrentSnapshotTree(ctx)
-	if err != nil {
-		return false, "", fmt.Errorf("Failed to check upgrade snapshot status: %s", err)
-	}
-
-	if IsUpgradeSnapshot(node, upgradePrefix) {
-		return true, node.Name, nil
-	}
-
-	return false, "", nil
-}
-
 func (vm *VirtualMachine) registerVM(ctx context.Context, path, name string,
 	vapp, pool, host *types.ManagedObjectReference, vmfolder *object.Folder) (*object.Task, error) {
 	log.Debugf("Register VM %s", name)
@@ -560,3 +550,41 @@ func (vm *VirtualMachine) DatastoreReference(ctx context.Context) ([]types.Manag
 	}
 	return mvm.Datastore, nil
 }
+
+// VCHUpdateStatus tells if an upgrade/configure has already been started based on the UpdateInProgress flag in ExtraConfig
+// It returns the error if the vm operation does not succeed
+func (vm *VirtualMachine) VCHUpdateStatus(ctx context.Context) (bool, error) {
+	info, err := vm.FetchExtraConfig(ctx)
+	if err != nil {
+		log.Errorf("Unable to get vm ExtraConfig: %s", err)
+		return false, err
+	}
+
+	if v, ok := info[UpdateStatus]; ok {
+		status, err := strconv.ParseBool(v)
+		if err != nil {
+			//  If error occurs, the bool return value does not matter for the caller.
+			return false, fmt.Errorf("failed to parse %s to bool: %s", v, err)
+		}
+		return status, nil
+	}
+
+	// If UpdateStatus is not found, it might be the case that no upgrade/configure has been done to this VCH before
+	return false, nil
+}
+
+// SetVCHUpdateStatus sets the VCH update status in ExtraConfig
+func (vm *VirtualMachine) SetVCHUpdateStatus(ctx context.Context, status bool) error {
+	info := make(map[string]string)
+	info[UpdateStatus] = strconv.FormatBool(status)
+
+	s := &types.VirtualMachineConfigSpec{
+		ExtraConfig: vmomi.OptionValueFromMap(info),
+	}
+
+	_, err := vm.WaitForResult(ctx, func(ctx context.Context) (tasks.Task, error) {
+		return vm.Reconfigure(ctx, *s)
+	})
+
+	return err
+}

pkg/vsphere/vm/vm_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/vmware/govmomi/vim25/mo"
 	"github.com/vmware/govmomi/vim25/types"
 	"github.com/vmware/vic/lib/guest"
+	"github.com/vmware/vic/pkg/vsphere/extraconfig/vmomi"
 	"github.com/vmware/vic/pkg/vsphere/session"
 	"github.com/vmware/vic/pkg/vsphere/simulator"
 	"github.com/vmware/vic/pkg/vsphere/sys"
@@ -603,3 +604,178 @@ func TestWaitForResult(t *testing.T) {
 	assert.True(t, called == 2, "task should be retried once")
 	assert.True(t, !vmm.IsInvalidState(ctx), "vm state should be fixed")
 }
+
+// SetUpdateStatus sets the VCH upgrade/configure status.
+func SetUpdateStatus(ctx context.Context, updateStatus string, vm *VirtualMachine) error {
+	info := make(map[string]string)
+	info[UpdateStatus] = updateStatus
+
+	s := &types.VirtualMachineConfigSpec{
+		ExtraConfig: vmomi.OptionValueFromMap(info),
+	}
+
+	_, err := vm.WaitForResult(ctx, func(ctx context.Context) (tasks.Task, error) {
+		return vm.Reconfigure(ctx, *s)
+	})
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// TestVCHUpdateStatus tests if VCHUpdateStatus() could obtain the correct VCH upgrade/configure status
+func TestVCHUpdateStatus(t *testing.T) {
+	ctx := context.Background()
+
+	// Nothing VC specific in this test, so we use the simpler ESX model
+	model := simulator.ESX()
+	defer model.Remove()
+	err := model.Create()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	server := model.Service.NewServer()
+	defer server.Close()
+	client, err := govmomi.NewClient(ctx, server.URL, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Any VM will do
+	finder := find.NewFinder(client.Client, false)
+	vmo, err := finder.VirtualMachine(ctx, "/ha-datacenter/vm/*_VM0")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	config := &session.Config{
+		Service:        server.URL.String(),
+		Insecure:       true,
+		Keepalive:      time.Duration(5) * time.Minute,
+		DatacenterPath: "",
+		DatastorePath:  "/ha-datacenter/datastore/*",
+		HostPath:       "/ha-datacenter/host/*/*",
+		PoolPath:       "/ha-datacenter/host/*/Resources",
+	}
+
+	s, err := session.NewSession(config).Connect(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s.Populate(ctx)
+	vmm := NewVirtualMachine(ctx, s, vmo.Reference())
+
+	updateStatus, err := vmm.VCHUpdateStatus(ctx)
+	if err != nil {
+		t.Fatalf("ERROR: %s", err)
+	}
+	assert.False(t, updateStatus, "updateStatus should be false if UpdateInProgress is not set in the VCH's ExtraConfig")
+
+	// Set UpdateInProgress to false
+	SetUpdateStatus(ctx, "false", vmm)
+
+	updateStatus, err = vmm.VCHUpdateStatus(ctx)
+	if err != nil {
+		t.Fatalf("ERROR: %s", err)
+	}
+	assert.False(t, updateStatus, "updateStatus should be false since UpdateInProgress is set to false")
+
+	// Set UpdateInProgress to true
+	SetUpdateStatus(ctx, "true", vmm)
+
+	updateStatus, err = vmm.VCHUpdateStatus(ctx)
+	if err != nil {
+		t.Fatalf("ERROR: %s", err)
+	}
+	assert.True(t, updateStatus, "updateStatus should be true since UpdateInProgress is set to true")
+
+	// Set UpdateInProgress to NonBool
+	SetUpdateStatus(ctx, "NonBool", vmm)
+
+	updateStatus, err = vmm.VCHUpdateStatus(ctx)
+	if assert.Error(t, err, "An error was expected") {
+		assert.Contains(t, err.Error(), "failed to parse", "Error msg should contain 'failed to parse' since UpdateInProgress is set to NonBool")
+	}
+}
+
+// TestSetVCHUpdateStatus tests if SetVCHUpdateStatus() could set the VCH upgrade/configure status correctly
+func TestSetVCHUpdateStatus(t *testing.T) {
+	ctx := context.Background()
+
+	// Nothing VC specific in this test, so we use the simpler ESX model
+	model := simulator.ESX()
+	defer model.Remove()
+	err := model.Create()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	server := model.Service.NewServer()
+	defer server.Close()
+	client, err := govmomi.NewClient(ctx, server.URL, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Any VM will do
+	finder := find.NewFinder(client.Client, false)
+	vmo, err := finder.VirtualMachine(ctx, "/ha-datacenter/vm/*_VM0")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	config := &session.Config{
+		Service:        server.URL.String(),
+		Insecure:       true,
+		Keepalive:      time.Duration(5) * time.Minute,
+		DatacenterPath: "",
+		DatastorePath:  "/ha-datacenter/datastore/*",
+		HostPath:       "/ha-datacenter/host/*/*",
+		PoolPath:       "/ha-datacenter/host/*/Resources",
+	}
+
+	s, err := session.NewSession(config).Connect(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	s.Populate(ctx)
+	vmm := NewVirtualMachine(ctx, s, vmo.Reference())
+
+	// Set UpdateInProgress to true and then check status
+	err = vmm.SetVCHUpdateStatus(ctx, true)
+	if err != nil {
+		t.Fatalf("ERROR: %s", err)
+	}
+
+	info, err := vmm.FetchExtraConfig(ctx)
+	if err != nil {
+		t.Fatalf("ERROR: %s", err)
+	}
+
+	v, ok := info[UpdateStatus]
+	if ok {
+		assert.Equal(t, "true", v, "UpdateInProgress should be true")
+	} else {
+		t.Fatal("ERROR: UpdateInProgress does not exist in ExtraConfig")
+	}
+
+	// Set UpdateInProgress to false and then check status
+	err = vmm.SetVCHUpdateStatus(ctx, false)
+	if err != nil {
+		t.Fatalf("ERROR: %s", err)
+	}
+
+	info, err = vmm.FetchExtraConfig(ctx)
+	if err != nil {
+		t.Fatalf("ERROR: %s", err)
+	}
+
+	v, ok = info[UpdateStatus]
+	if ok {
+		assert.Equal(t, "false", v, "UpdateInProgress should be false")
+	} else {
+		t.Fatal("ERROR: UpdateInProgress does not exist in ExtraConfig")
+	}
+}

tests/resources/VCH-Util.robot

@@ -206,6 +206,18 @@ Run VIC Machine Inspect Command
     ${rc}  ${output}=  Run Secret VIC Machine Inspect Command  %{VCH-NAME}
     Get Docker Params  ${output}  ${true}
 
+Inspect VCH
+    [Arguments]  ${expected}
+    ${rc}  ${output}=  Run And Return Rc And Output  bin/vic-machine-linux inspect --name=%{VCH-NAME} --target=%{TEST_URL} --thumbprint=%{TEST_THUMBPRINT} --user=%{TEST_USERNAME} --password=%{TEST_PASSWORD} --compute-resource=%{TEST_RESOURCE}
+    Should Be Equal As Integers  ${rc}  0
+    Should Contain  ${output}  ${expected}
+
+Check UpdateInProgress
+    [Arguments]  ${expected}
+    ${rc}  ${output}=  Run And Return Rc And Output  govc vm.info -e %{VCH-NAME} | grep UpdateInProgress
+    Should Be Equal As Integers  ${rc}  0
+    Should Contain  ${output}  ${expected}
+
 Gather Logs From Test Server
     [Tags]  secret
     Run Keyword And Continue On Failure  Run  zip %{VCH-NAME}-certs -r %{VCH-NAME}