docker driver: handle already deleted containers

don't expect that all the containers of a pod are available. Some could be
already be removed. Improve the logic of sorting containers by their index.

internal/services/executor/driver/docker.go

@@ -23,6 +23,7 @@ import (
 	"io/ioutil"
 	"os"
 	"runtime"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -181,10 +182,9 @@ func (d *DockerDriver) NewPod(ctx context.Context, podConfig *PodConfig, out io.
 		id:         podConfig.ID,
 		client:     d.client,
 		executorID: d.executorID,
-		containers: make([]types.Container, len(containers)),
+		containers: []*DockerContainer{},
 	}
 
-	// Put the containers in the right order based on their containerIndexKey label value
 	count := 0
 	seenIndexes := map[int]struct{}{}
 	for _, container := range containers {
@@ -201,7 +201,11 @@ func (d *DockerDriver) NewPod(ctx context.Context, podConfig *PodConfig, out io.
 		if _, ok := seenIndexes[cIndex]; ok {
 			return nil, errors.Errorf("duplicate container with index %d", cIndex)
 		}
-		pod.containers[cIndex] = container
+		dContainer := &DockerContainer{
+			Index:     cIndex,
+			Container: container,
+		}
+		pod.containers = append(pod.containers, dContainer)
 
 		seenIndexes[cIndex] = struct{}{}
 		count++
@@ -209,6 +213,8 @@ func (d *DockerDriver) NewPod(ctx context.Context, podConfig *PodConfig, out io.
 	if count != len(containers) {
 		return nil, errors.Errorf("expected %d containers but got %d", len(containers), count)
 	}
+	// put the containers in the right order based on their container index
+	sort.Sort(ContainerSlice(pod.containers))
 
 	return pod, nil
 }
@@ -312,21 +318,17 @@ func (d *DockerDriver) GetPods(ctx context.Context, all bool) ([]Pod, error) {
 			// skip container
 			continue
 		}
-		if pod, ok := podsMap[podID]; !ok {
+		if _, ok := podsMap[podID]; !ok {
 			pod := &DockerPod{
 				id:         podID,
 				client:     d.client,
-				containers: []types.Container{container},
 				executorID: d.executorID,
+				containers: []*DockerContainer{},
 			}
 			podsMap[podID] = pod
-
-		} else {
-			pod.containers = append(pod.containers, container)
 		}
 	}
 
-	// Put the containers in the right order based on their containerIndexKey label value
 	for _, container := range containers {
 		podID, ok := container.Labels[podIDKey]
 		if !ok {
@@ -343,8 +345,13 @@ func (d *DockerDriver) GetPods(ctx context.Context, all bool) ([]Pod, error) {
 			// remove pod since some of its containers don't have the right labels
 			delete(podsMap, podID)
 		}
+
 		pod := podsMap[podID]
-		pod.containers[cIndex] = container
+		dContainer := &DockerContainer{
+			Index:     cIndex,
+			Container: container,
+		}
+		pod.containers = append(pod.containers, dContainer)
 
 		// overwrite containers with the right order
 
@@ -363,6 +370,8 @@ func (d *DockerDriver) GetPods(ctx context.Context, all bool) ([]Pod, error) {
 
 	pods := make([]Pod, 0, len(podsMap))
 	for _, pod := range podsMap {
+		// put the containers in the right order based on their container index
+		sort.Sort(ContainerSlice(pod.containers))
 		pods = append(pods, pod)
 	}
 	return pods, nil
@@ -383,10 +392,21 @@ type DockerPod struct {
 	id         string
 	client     *client.Client
 	labels     map[string]string
-	containers []types.Container
+	containers []*DockerContainer
 	executorID string
 }
 
+type DockerContainer struct {
+	Index int
+	types.Container
+}
+
+type ContainerSlice []*DockerContainer
+
+func (p ContainerSlice) Len() int           { return len(p) }
+func (p ContainerSlice) Less(i, j int) bool { return p[i].Index < p[j].Index }
+func (p ContainerSlice) Swap(i, j int)      { p[i], p[j] = p[j], p[i] }
+
 func (dp *DockerPod) ID() string {
 	return dp.id
 }

internal/services/executor/driver/docker_test.go

@@ -27,6 +27,7 @@ import (
 	"time"
 	"unicode"
 
+	"github.com/docker/docker/api/types"
 	uuid "github.com/satori/go.uuid"
 	slog "github.com/sorintlab/agola/internal/log"
 
@@ -294,7 +295,7 @@ func TestDockerPod(t *testing.T) {
 				dp := p.(*DockerPod)
 				for i, c := range dp.containers {
 					if c.ID != ip.containers[i].ID {
-						t.Fatalf("different pod id, want: %s, got: %s", ip.id, dp.id)
+						t.Fatalf("different container id, want: %q, got: %q", c.ID, ip.containers[i].ID)
 					}
 					if diff := cmp.Diff(ip.containers[i], c); diff != "" {
 						t.Error(diff)
@@ -340,7 +341,7 @@ func TestDockerPod(t *testing.T) {
 				dp := p.(*DockerPod)
 				for i, c := range dp.containers {
 					if c.ID != ip.containers[i].ID {
-						t.Fatalf("different pod id, want: %s, got: %s", ip.id, dp.id)
+						t.Fatalf("different container id, want: %q, got: %q", c.ID, ip.containers[i].ID)
 					}
 					if diff := cmp.Diff(ip.containers[i], c); diff != "" {
 						t.Error(diff)
@@ -352,4 +353,57 @@ func TestDockerPod(t *testing.T) {
 			t.Fatalf("pod with id %q not found", pod.ID())
 		}
 	})
+
+	t.Run("test get pods with two containers and the first already deleted", func(t *testing.T) {
+		pod, err := d.NewPod(ctx, &PodConfig{
+			ID:     uuid.NewV4().String(),
+			TaskID: uuid.NewV4().String(),
+			Containers: []*ContainerConfig{
+				&ContainerConfig{
+					Cmd:   []string{"cat"},
+					Image: "busybox",
+				},
+				&ContainerConfig{
+					Image: "nginx:1.16",
+				},
+			},
+			InitVolumeDir: "/tmp/agola",
+		}, ioutil.Discard)
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+		defer pod.Remove(ctx)
+
+		// delete the first container
+		dp := pod.(*DockerPod)
+		if err := dp.client.ContainerRemove(ctx, dp.containers[0].ID, types.ContainerRemoveOptions{Force: true}); err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		pods, err := d.GetPods(ctx, true)
+		if err != nil {
+			t.Fatalf("unexpected err: %v", err)
+		}
+
+		ok := false
+		for _, p := range pods {
+			if p.ID() == pod.ID() {
+				ok = true
+				ip := pod.(*DockerPod)
+				dp := p.(*DockerPod)
+				if len(dp.containers) != 1 {
+					t.Fatalf("expected 1 container, got %d containers", len(dp.containers))
+				}
+				if dp.containers[0].ID != ip.containers[1].ID {
+					t.Fatalf("different container id, want: %q, got: %q", dp.containers[0].ID, ip.containers[1].ID)
+				}
+				if diff := cmp.Diff(ip.containers[1], dp.containers[0]); diff != "" {
+					t.Error(diff)
+				}
+			}
+		}
+		if !ok {
+			t.Fatalf("pod with id %q not found", pod.ID())
+		}
+	})
 }