Repository: threefoldtech/zos
Pull request #546 (2020-02-25 10:28:28)

provisiond: ensure the ip address assigned to the container are returns in the provision results

codecov[bot]: # [Codecov](https://codecov.io/gh/threefoldtech/zos/pull/546?src=pr&el=h1) Report
> Merging [#546](https://codecov.io/gh/threefoldtech/zos/pull/546?src=pr&el=desc) into [master](https://codecov.io/gh/threefoldtech/zos/commit/f5a7f45b630931556ae9da94283924c0c11b1bc1&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/threefoldtech/zos/pull/546/graphs/tree.svg?width=650&height=150&src=pr&token=f7Vmq1OAEm)](https://codecov.io/gh/threefoldtech/zos/pull/546?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #546      +/-   ##
==========================================
+ Coverage   36.26%   36.27%   +0.01%     
==========================================
  Files          75       75              
  Lines        5066     5067       +1     
==========================================
+ Hits         1837     1838       +1     
  Misses       2963     2963              
  Partials      266      266              
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/threefoldtech/zos/pull/546?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/threefoldtech/zos/pull/546?src=pr&el=footer). Last update [f5a7f45...e44fa37](https://codecov.io/gh/threefoldtech/zos/pull/546?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/network/nr/container.go b/pkg/network/nr/container.go
index 389f47df..38765877 100644
--- a/pkg/network/nr/container.go
+++ b/pkg/network/nr/container.go
@@ -78,6 +78,7 @@ func (nr *NetResource) Join(containerID string, addrs []net.IP, publicIP6 bool)
 			}}); err != nil && !os.IsExist(err) {
 				return err
 			}
+			join.IPv4 = addr
 		}
 
 		if !publicIP6 {
@@ -92,6 +93,7 @@ func (nr *NetResource) Join(containerID string, addrs []net.IP, publicIP6 bool)
 			}}); err != nil && !os.IsExist(err) {
 				return err
 			}
+			join.IPv6 = ipv6
 		}
 
 		ipnet := nr.resource.Subnet
diff --git a/pkg/provision/container.go b/pkg/provision/container.go
index 467b4e33..bd88b0a4 100644
--- a/pkg/provision/container.go
+++ b/pkg/provision/container.go
@@ -265,8 +265,10 @@ func containerDecommission(ctx context.Context, reservation *Reservation) error
 	}
 
 	netID := networkID(reservation.User, string(config.Network.NetworkID))
-	if err := networkMgr.Leave(netID, string(containerID)); err != nil {
-		return errors.Wrap(err, "failed to delete container network namespace")
+	if _, err := networkMgr.GetSubnet(netID); err == nil { // simple check to make sure the network still exists on the node
+		if err := networkMgr.Leave(netID, string(containerID)); err != nil {
+			return errors.Wrap(err, "failed to delete container network namespace")
+		}
 	}
 
 	return nil


Test patch:
diff --git a/pkg/provision/container_test.go b/pkg/provision/container_test.go
index 6e19d42d..9a8a08ff 100644
--- a/pkg/provision/container_test.go
+++ b/pkg/provision/container_test.go
@@ -288,7 +288,15 @@ func TestContainerDecomissionExists(t *testing.T) {
 		Return(nil)
 
 	netID := networkID(reservation.User, string(container.Network.NetworkID))
-	// fourth, leave the container network namespace
+
+	// fourth, check if the network still exists
+	client.On("Request",
+		"network", zbus.ObjectID{Name: "network", Version: "0.0.1"},
+		"GetSubnet",
+		netID).
+		Return(net.IPNet{}, nil) // return a nil error, the network still exists
+
+	// fifths, leave the container network namespace
 	client.On("Request",
 		"network", zbus.ObjectID{Name: "network", Version: "0.0.1"},
 		"Leave",
@@ -335,8 +343,15 @@ func TestContainerDecomissionContainerGone(t *testing.T) {
 		pkg.ContainerID(reservation.ID)).
 		Return(nil, fmt.Errorf("container not found"))
 
+	// second, check if the network still exists
 	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"},
+		"GetSubnet",
+		netID).
+		Return(net.IPNet{}, nil) // return a nil error, the network still exists
+
+	// third,  leave the container network namespace
 	client.On("Request",
 		"network", zbus.ObjectID{Name: "network", Version: "0.0.1"},
 		"Leave",
@@ -347,3 +362,66 @@ func TestContainerDecomissionContainerGone(t *testing.T) {
 	err := containerDecommission(ctx, &reservation)
 	require.NoError(err)
 }
+
+func TestContainerDecomissionNetworkGone(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),
+	}
+	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, check if the network still exists
+	client.On("Request",
+		"network", zbus.ObjectID{Name: "network", Version: "0.0.1"},
+		"GetSubnet",
+		netID).
+		Return(net.IPNet{}, fmt.Errorf("network not found"))
+
+	err := containerDecommission(ctx, &reservation)
+	require.NoError(err)
+}