From 0b6afc91a05390113d940d65b0ebb7e85ff0e850 Mon Sep 17 00:00:00 2001
From: Djalal Harouni <djalal@endocode.com>
Date: Mon, 11 Apr 2016 09:12:46 +0100
Subject: [PATCH 1/2] registry: remove extra loops when fetching Jobs list from
 etcd

Improve Units() call when we fetch and process Jobs and Units from
etcd. Remove extra unnecessary loops.

We also change the code logic a little bit, since it was always storing
the last matched name with the new Unit, but since Job keys do not expect to
have two units with the same name, this should be ok.
---
 registry/job.go | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/registry/job.go b/registry/job.go
index 8eeda2280..ca16282eb 100644
--- a/registry/job.go
+++ b/registry/job.go
@@ -90,6 +90,7 @@ func (r *EtcdRegistry) Schedule() ([]job.ScheduledUnit, error) {
 func (r *EtcdRegistry) Units() ([]job.Unit, error) {
 	key := r.prefixed(jobPrefix)
 	opts := &etcd.GetOptions{
+		// We need Job Units to be sorted
 		Sort:      true,
 		Recursive: true,
 	}
@@ -117,7 +118,7 @@ func (r *EtcdRegistry) Units() ([]job.Unit, error) {
 		return unit
 	}
 
-	uMap := make(map[string]*job.Unit)
+	units := make([]job.Unit, 0)
 	for _, dir := range res.Node.Nodes {
 		u, err := r.dirToUnit(dir, unitHashLookupFunc)
 		if err != nil {
@@ -127,18 +128,8 @@ func (r *EtcdRegistry) Units() ([]job.Unit, error) {
 		if u == nil {
 			continue
 		}
-		uMap[u.Name] = u
-	}
-
-	var sortable sort.StringSlice
-	for name, _ := range uMap {
-		sortable = append(sortable, name)
-	}
-	sortable.Sort()
 
-	units := make([]job.Unit, 0, len(sortable))
-	for _, name := range sortable {
-		units = append(units, *uMap[name])
+		units = append(units, *u)
 	}
 
 	return units, nil

From 9963e8ba89a777bfec47a10e1bdb01d2589f3ad4 Mon Sep 17 00:00:00 2001
From: Djalal Harouni <djalal@endocode.com>
Date: Mon, 18 Apr 2016 11:58:24 +0200
Subject: [PATCH 2/2] functional: add TestListUnitFilesOrder to assert the
 order of returned units

This test makes sure that it fleetd returned an ordered list of units
from Units() call through list-unit-files command.
---
 functional/unit_action_test.go | 63 ++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/functional/unit_action_test.go b/functional/unit_action_test.go
index 8d7dab729..7850db5bc 100644
--- a/functional/unit_action_test.go
+++ b/functional/unit_action_test.go
@@ -15,8 +15,11 @@
 package functional
 
 import (
+	"fmt"
 	"io/ioutil"
 	"path"
+	"reflect"
+	"sort"
 	"strings"
 	"testing"
 
@@ -404,3 +407,63 @@ func TestUnitStatus(t *testing.T) {
 			stdout, stderr)
 	}
 }
+
+// TestListUnitFilesOrder simply checks if "fleetctl list-unit-files" returns
+// an ordered list of units
+func TestListUnitFilesOrder(t *testing.T) {
+	cluster, err := platform.NewNspawnCluster("smoke")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cluster.Destroy()
+
+	m, err := cluster.CreateMember()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = cluster.WaitForNMachines(m, 1)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Combine units
+	var units []string
+	for i := 1; i <= 20; i++ {
+		unit := fmt.Sprintf("fixtures/units/hello@%02d.service", i)
+		stdout, stderr, err := cluster.Fleetctl(m, "submit", unit)
+		if err != nil {
+			t.Fatalf("Failed to submit a batch of units: \nstdout: %s\nstder: %s\nerr: %v", stdout, stderr, err)
+		}
+		units = append(units, unit)
+	}
+
+	// make sure that all unit files will show up
+	_, err = cluster.WaitForNUnitFiles(m, 20)
+	if err != nil {
+		t.Fatal("Failed to run list-unit-files: %v", err)
+	}
+
+	stdout, _, err := cluster.Fleetctl(m, "list-unit-files", "--no-legend", "--fields", "unit")
+	if err != nil {
+		t.Fatal("Failed to run list-unit-files: %v", err)
+	}
+
+	outUnits := strings.Split(strings.TrimSpace(stdout), "\n")
+
+	var sortable sort.StringSlice
+	for _, name := range units {
+		n := path.Base(name)
+		sortable = append(sortable, n)
+	}
+	sortable.Sort()
+
+	var inUnits []string
+	for _, name := range sortable {
+		inUnits = append(inUnits, name)
+	}
+
+	if !reflect.DeepEqual(inUnits, outUnits) {
+		t.Fatalf("Failed to get a sorted list of units from list-unit-files")
+	}
+}