Merge pull request #787 from darkowlzz/ps-outdated

cmd/ignite/run/ps.go

@@ -6,12 +6,22 @@ import (
 	"strings"
 	"text/template"
 
+	"github.com/pkg/errors"
+	log "github.com/sirupsen/logrus"
 	api "github.com/weaveworks/ignite/pkg/apis/ignite"
 	"github.com/weaveworks/ignite/pkg/filter"
 	"github.com/weaveworks/ignite/pkg/providers"
+	"github.com/weaveworks/ignite/pkg/runtime"
+	containerdruntime "github.com/weaveworks/ignite/pkg/runtime/containerd"
+	dockerruntime "github.com/weaveworks/ignite/pkg/runtime/docker"
 	"github.com/weaveworks/ignite/pkg/util"
 )
 
+// runtimeRunningStatus is the status returned from the container runtimes when
+// the VM container is in running state.
+const runtimeRunningStatus = "running"
+const oldManifestIndicator = "*"
+
 // PsFlags contains the flags supported by ps.
 type PsFlags struct {
 	All            bool
@@ -67,6 +77,24 @@ func Ps(po *PsOptions) error {
 		}
 	}
 
+	endWarnings := []error{}
+	outdatedVMs, errList := fetchLatestStatus(filteredVMs)
+	if len(outdatedVMs) > 0 {
+		endWarnings = append(
+			endWarnings,
+			fmt.Errorf("The symbol %s on the VM status indicates that the VM manifest on disk is not up-to-date with the actual VM status from the container runtime", oldManifestIndicator),
+		)
+	}
+	if len(errList) > 0 {
+		endWarnings = append(endWarnings, errList...)
+	}
+	defer func() {
+		// Add a note at the bottom about the old manifest indicator in the status.
+		for _, err := range endWarnings {
+			log.Warn(err)
+		}
+	}()
+
 	// If template format is specified, render the template.
 	if po.PsFlags.TemplateFormat != "" {
 		// Parse the template format.
@@ -92,7 +120,7 @@ func Ps(po *PsOptions) error {
 	o.Write("VM ID", "IMAGE", "KERNEL", "SIZE", "CPUS", "MEMORY", "CREATED", "STATUS", "IPS", "PORTS", "NAME")
 	for _, vm := range filteredVMs {
 		o.Write(vm.GetUID(), vm.Spec.Image.OCI, vm.Spec.Kernel.OCI,
-			vm.Spec.DiskSize, vm.Spec.CPUs, vm.Spec.Memory, formatCreated(vm), formatStatus(vm), vm.Status.Network.IPAddresses,
+			vm.Spec.DiskSize, vm.Spec.CPUs, vm.Spec.Memory, formatCreated(vm), formatStatus(vm, outdatedVMs), vm.Status.Network.IPAddresses,
 			vm.Spec.Network.Ports, vm.GetName())
 	}
 
@@ -110,10 +138,86 @@ func formatCreated(vm *api.VM) string {
 	return fmt.Sprint(created, suffix)
 }
 
-func formatStatus(vm *api.VM) string {
+func formatStatus(vm *api.VM, outdatedVMs map[string]bool) string {
+	isOld := ""
+	if _, ok := outdatedVMs[vm.Name]; ok {
+		isOld = oldManifestIndicator
+	}
+
 	if vm.Running() {
-		return fmt.Sprintf("Up %s", vm.Status.StartTime)
+		return fmt.Sprintf("%sUp %s", isOld, vm.Status.StartTime)
+	}
+
+	return isOld + "Stopped"
+}
+
+// fetchLatestStatus fetches the current status the VMs, updates the VM status
+// in memory and returns a list of outdated VMs.
+func fetchLatestStatus(vms []*api.VM) (outdatedVMs map[string]bool, errList []error) {
+	outdatedVMs = map[string]bool{}
+	errList = []error{}
+
+	// Container runtime clients. These clients are lazy initialized based on
+	// the VM's runtime.
+	var containerdClient, dockerClient runtime.Interface
+
+	// Iterate through the VMs, fetching the actual status from the runtime.
+	for _, vm := range vms {
+		// Skip VMs with no runtime info or no runtime ID.
+		if vm.Status.Runtime == nil || vm.Status.Runtime.ID == "" {
+			continue
+		}
+		containerID := vm.Status.Runtime.ID
+		currentRunning := false
+
+		// Runtime client of the VM.
+		var vmRuntime runtime.Interface
+
+		// Set the appropriate runtime client based on the VM runtime info.
+		switch vm.Status.Runtime.Name {
+		case runtime.RuntimeContainerd:
+			if containerdClient == nil {
+				var err error
+				containerdClient, err = containerdruntime.GetContainerdClient()
+				if err != nil {
+					errList = append(errList, err)
+					return
+				}
+			}
+			vmRuntime = containerdClient
+		case runtime.RuntimeDocker:
+			if dockerClient == nil {
+				var err error
+				dockerClient, err = dockerruntime.GetDockerClient()
+				if err != nil {
+					errList = append(errList, err)
+					return
+				}
+			}
+			vmRuntime = dockerClient
+		}
+
+		// Inspect the VM container using the runtime client.
+		ir, inspectErr := vmRuntime.InspectContainer(containerID)
+		if inspectErr != nil {
+			errList = append(errList, errors.Wrapf(inspectErr, "failed to inspect container for VM %s", containerID))
+			continue
+		}
+
+		// Set current running based on the container status result.
+		if ir.Status == runtimeRunningStatus {
+			currentRunning = true
+		}
+
+		// If current running status and the VM object status don't match, mark
+		// it as an outdated VM and update the VM object staus in memory.
+		// NOTE: Avoid updating the VM manifest on disk here. That'll be
+		// indicated in the ps output.
+		if currentRunning != vm.Status.Running {
+			vm.Status.Running = currentRunning
+			outdatedVMs[vm.Name] = true
+		}
 	}
 
-	return "Stopped"
+	return
 }

e2e/vm_ps_test.go

@@ -0,0 +1,79 @@
+package e2e
+
+import (
+	"fmt"
+	"io/ioutil"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"gotest.tools/assert"
+
+	"github.com/weaveworks/ignite/e2e/util"
+	api "github.com/weaveworks/ignite/pkg/apis/ignite"
+	"github.com/weaveworks/ignite/pkg/apis/ignite/scheme"
+	"github.com/weaveworks/ignite/pkg/constants"
+)
+
+// TestVMpsWithOutdatedStatus checks if outdated status indicators are printed
+// in ps output when the VM manifest status on disk don't match with actual
+// status.
+func TestVMpsWithOutdatedStatus(t *testing.T) {
+	assert.Assert(t, e2eHome != "", "IGNITE_E2E_HOME should be set")
+
+	vmName := "e2e-test-ignite-ps-outdated"
+
+	igniteCmd := util.NewCommand(t, igniteBin)
+
+	defer igniteCmd.New().
+		With("rm", "-f", vmName).
+		Run()
+
+	igniteCmd.New().
+		With("run", "--name="+vmName).
+		With(util.DefaultVMImage).
+		With("--ssh").
+		Run()
+
+	// Filter the VM and obtain the UID.
+	nameFilter := fmt.Sprintf("{{.ObjectMeta.Name}}=%s", vmName)
+	psUIDCmd := igniteCmd.New().
+		With("ps", "-a").
+		With("-f", nameFilter).
+		With("-t", "{{.ObjectMeta.UID}}")
+	psUIDOut, psUIDErr := psUIDCmd.Cmd.CombinedOutput()
+	assert.NilError(t, psUIDErr, fmt.Sprintf("ps: \n%q\n%s", psUIDCmd.Cmd, psUIDOut))
+
+	uid := strings.TrimSpace(string(psUIDOut))
+
+	// Update the VM manifest with false info.
+	metadata_path := filepath.Join(constants.VM_DIR, uid, "metadata.json")
+	vm := &api.VM{}
+	assert.NilError(t, scheme.Serializer.DecodeFileInto(metadata_path, vm))
+	vm.Status.Running = false
+	vmBytes, err := scheme.Serializer.EncodeJSON(vm)
+	assert.NilError(t, err)
+	assert.NilError(t, ioutil.WriteFile(metadata_path, vmBytes, 0644))
+
+	// Revert the false data for proper cleanup.
+	// NOTE: This is needed because ignite rm believes the VM manifest status
+	// instead of checking for actual status itself.
+	defer func() {
+		vm.Status.Running = true
+		vmBytes, err = scheme.Serializer.EncodeJSON(vm)
+		assert.NilError(t, err)
+		assert.NilError(t, ioutil.WriteFile(metadata_path, vmBytes, 0644))
+	}()
+
+	// Run ps -a and look for the outdated status info.
+	psCmd := igniteCmd.New().
+		With("ps", "-a")
+
+	psOut, psErr := psCmd.Cmd.CombinedOutput()
+	assert.NilError(t, psErr, fmt.Sprintf("ps: \n%q\n%s", psCmd.Cmd, psOut))
+
+	psOutString := string(psOut)
+	// Check for the outdated status and the note about it.
+	assert.Check(t, strings.Contains(psOutString, "*Up"))
+	assert.Check(t, strings.Contains(psOutString, "The symbol *"))
+}