Merge pull request #4210 from krancour/fix-deisctl-journal

fix(deisctl): fix how target names resolve

Author: krancour

deisctl/backend/fleet/unit.go

@@ -32,11 +32,31 @@ func (c *FleetClient) Units(target string) (units []string, err error) {
 	if err != nil {
 		return
 	}
+	// Look for units starting with the given target name first. If the given
+	// name starts with "deis-", this will easily locate platform components,
+	// but we search without canonicalizing the target name FIRST so we have the
+	// opportunity to locate application containers (whose containers do not
+	// adhere to the same naming convention as the platform's own components).
 	for _, u := range allUnits {
-		if strings.Contains(u.Name, target) {
+		if strings.HasPrefix(u.Name, target) {
 			units = append(units, u.Name)
 		}
 	}
+	// If none are found, canonicalize the target string and search again. This
+	// will locate platform components that were referenced by a target string
+	// NOT already beginning with "deis-".
+	if len(units) == 0 {
+		canonTarget := strings.ToLower(target)
+		if !strings.HasPrefix(canonTarget, "deis-") {
+			canonTarget = "deis-" + canonTarget
+		}
+		for _, u := range allUnits {
+			if strings.HasPrefix(u.Name, canonTarget) {
+				units = append(units, u.Name)
+			}
+		}
+	}
+	// If still nothing is found, then we have an error on our hands.
 	if len(units) == 0 {
 		err = fmt.Errorf("could not find unit: %s", target)
 	}

deisctl/backend/fleet/unit_test.go

@@ -4,6 +4,7 @@ import (
 	"io/ioutil"
 	"path"
 	"reflect"
+	"strings"
 	"sync"
 	"testing"
 
@@ -27,18 +28,39 @@ func TestUnits(t *testing.T) {
 	}
 
 	c := &FleetClient{Fleet: &stubFleetClient{testUnits: testUnits, unitsMutex: &sync.Mutex{}}}
+	expected := []string{"deis-router@1.service", "deis-router@2.service", "deis-router@3.service"}
 
+	// Test that the correct units are resolved when providing a target string
+	// that EXCEPT for lacking the "deis-" prefix matches the beginning of one
+	// or more units' name...
 	targets, err := c.Units("router")
-
 	if err != nil {
 		t.Fatal(err)
 	}
+	if !reflect.DeepEqual(targets, expected) {
+		t.Fatalf("Expected %v, Got %v", expected, targets)
+	}
 
-	expected := []string{"deis-router@1.service", "deis-router@2.service", "deis-router@3.service"}
-
+	// Test that the correct units are resolved when providing a target string
+	// INCLUDING the "deis-" prefix that matches the beginning of one or more
+	// units' name...
+	targets, err = c.Units("deis-router")
+	if err != nil {
+		t.Fatal(err)
+	}
 	if !reflect.DeepEqual(targets, expected) {
 		t.Fatalf("Expected %v, Got %v", expected, targets)
 	}
+
+	// Test that no units are resolved and an error is returned when providing
+	// a target string that does not match the BEGINNING of a service unit name,
+	// either with or without the "deis-" prefix. We deliberately test here using
+	// a string that IS a substring (albeit in the wrong position) of service
+	// units in the stub...
+	targets, err = c.Units("outer")
+	if err == nil || !strings.HasPrefix(err.Error(), "could not find unit:") {
+		t.Fatalf("Expected an error beginning with \"could not find unit:\", but did not get one")
+	}
 }
 
 func TestNextUnit(t *testing.T) {