Repository: threefoldtech/zos (go)

Issue:
failed to decommission reservation
In the logs I see a lot of error like
```
[-] provisiond: {"level":"error","error":"decommissioning of reservation failed: failed to inspect container 112-1: container \"112-1\" in namespace \"nsFuy6CatZGmmtWby13MESN2dYZUJi1ThjPR493w9TyWz1\": not found","time":"2019-11-25T10:53:46Z","message":"failed to decommission reservation 112-1"}
```
I'm not sure of why exactly this happens, but it seems this already shows a problem. The decommission should not fail if the container is already not existing.

Pull request:
provisiond: container decomission handle already deleted container
Handle the case where a container decomission failed halfway though.
The code now will still try to cleanup network even if the container is
already gone.

fixes #403

Patch:
diff --git a/pkg/provision/container.go b/pkg/provision/container.go
index 9bb0dac0..320841ea 100644
--- a/pkg/provision/container.go
+++ b/pkg/provision/container.go
@@ -220,24 +220,25 @@ func containerDecommission(ctx context.Context, reservation *Reservation) error
 	}
 
 	info, err := container.Inspect(tenantNS, containerID)
-	if err != nil {
-		return errors.Wrapf(err, "failed to inspect container %s", containerID)
-	}
+	if err == nil {
+		if err := container.Delete(tenantNS, containerID); err != nil {
+			return errors.Wrapf(err, "failed to delete container %s", containerID)
+		}
 
-	if err := container.Delete(tenantNS, containerID); err != nil {
-		return errors.Wrapf(err, "failed to delete container %s", containerID)
-	}
+		rootFS := info.RootFS
+		if info.Interactive {
+			rootFS, err = findRootFS(info.Mounts)
+			if err != nil {
+				return err
+			}
+		}
 
-	rootFS := info.RootFS
-	if info.Interactive {
-		rootFS, err = findRootFS(info.Mounts)
-		if err != nil {
-			return err
+		if err := flist.Umount(rootFS); err != nil {
+			return errors.Wrapf(err, "failed to unmount flist at %s", rootFS)
 		}
-	}
 
-	if err := flist.Umount(rootFS); err != nil {
-		return errors.Wrapf(err, "failed to unmount flist at %s", rootFS)
+	} else {
+		log.Error().Err(err).Str("container", string(containerID)).Msg("failed to inspect container for decomission")
 	}
 
 	netID := networkID(reservation.User, string(config.Network.NetworkID))


Test patch:
diff --git a/pkg/provision/container_test.go b/pkg/provision/container_test.go
index 5c864da4..aa6afe27 100644
--- a/pkg/provision/container_test.go
+++ b/pkg/provision/container_test.go
@@ -233,3 +233,118 @@ func TestContainerProvisionWithMounts(t *testing.T) {
 	require.Equal("reservation-id", result.ID)
 	require.Equal("192.168.1.1", result.IPv4)
 }
+
+func TestContainerDecomissionExists(t *testing.T) {
+	require := require.New(t)
+
+	var client TestClient
+	var cache TestOwnerCache
+
+	ctx := context.Background()
+	ctx = WithZBus(ctx, &client)
+	ctx = WithOwnerCache(ctx, &cache)
+
+	// const module = "network"
+	// version := zbus.ObjectID{Name: "network", Version: "0.0.1"}
+
+	container := Container{
+		FList: "https://hub.grid.tf/thabet/redis.flist",
+		Network: Network{
+			NetworkID: pkg.NetID("net1"),
+			IPs: []net.IP{
+				net.ParseIP("192.168.1.1"),
+			},
+		},
+	}
+
+	reservation := Reservation{
+		ID:   "reservation-id",
+		User: "user",
+		Type: ContainerReservation,
+		Data: MustMarshal(t, container),
+	}
+	rootFS := "/mnt/flist_root"
+
+	// first, the decomission will inspect the container
+	client.On("Request",
+		"container", zbus.ObjectID{Name: "container", Version: "0.0.1"},
+		"Inspect",
+		fmt.Sprintf("ns%s", reservation.User),
+		pkg.ContainerID(reservation.ID)).
+		Return(pkg.Container{Name: reservation.ID, RootFS: rootFS}, nil)
+
+	// second, the decomission will delete the container
+	client.On("Request",
+		"container", zbus.ObjectID{Name: "container", Version: "0.0.1"},
+		"Delete",
+		fmt.Sprintf("ns%s", reservation.User),
+		pkg.ContainerID(reservation.ID)).
+		Return(nil)
+
+	// third, unmount the flist of the container
+	client.On("Request",
+		"flist", zbus.ObjectID{Name: "flist", Version: "0.0.1"},
+		"Umount",
+		rootFS).
+		Return(nil)
+
+	netID := networkID(reservation.User, string(container.Network.NetworkID))
+	// fourth, leave the container network namespace
+	client.On("Request",
+		"network", zbus.ObjectID{Name: "network", Version: "0.0.1"},
+		"Leave",
+		netID,
+		reservation.ID).
+		Return(nil)
+
+	err := containerDecommission(ctx, &reservation)
+	require.NoError(err)
+}
+
+func TestContainerDecomissionContainerGone(t *testing.T) {
+	require := require.New(t)
+
+	var client TestClient
+	var cache TestOwnerCache
+
+	ctx := context.Background()
+	ctx = WithZBus(ctx, &client)
+	ctx = WithOwnerCache(ctx, &cache)
+
+	container := Container{
+		FList: "https://hub.grid.tf/thabet/redis.flist",
+		Network: Network{
+			NetworkID: pkg.NetID("net1"),
+			IPs: []net.IP{
+				net.ParseIP("192.168.1.1"),
+			},
+		},
+	}
+
+	reservation := Reservation{
+		ID:   "reservation-id",
+		User: "user",
+		Type: ContainerReservation,
+		Data: MustMarshal(t, container),
+	}
+
+	// first, the decomission will inspect the container
+	client.On("Request",
+		"container", zbus.ObjectID{Name: "container", Version: "0.0.1"},
+		"Inspect",
+		fmt.Sprintf("ns%s", reservation.User),
+		pkg.ContainerID(reservation.ID)).
+		Return(nil, fmt.Errorf("container not found"))
+
+	netID := networkID(reservation.User, string(container.Network.NetworkID))
+	// fourth, leave the container network namespace
+	client.On("Request",
+		"network", zbus.ObjectID{Name: "network", Version: "0.0.1"},
+		"Leave",
+		netID,
+		reservation.ID).
+		Return(nil)
+
+	err := containerDecommission(ctx, &reservation)
+	require.NoError(err)
+}