From 9e86dfc9c5b56eaa91485826bcf3f1f7617d2eb0 Mon Sep 17 00:00:00 2001
From: Zeling Feng <zeling@google.com>
Date: Mon, 22 Mar 2021 14:07:55 -0700
Subject: Fix logs for packetimpact tests cleanup

- Don't cleanup containers in Network.Cleanup, otherwise containers will
  be killed and removed several times.
- Don't set AutoRemove for containers. This will prevent the confusing
  'removal already in progress' messages.

Fixes #3795

PiperOrigin-RevId: 364404414
---
 pkg/test/dockerutil/network.go  |  5 +----
 test/packetimpact/runner/dut.go | 22 +++++++++++++---------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/pkg/test/dockerutil/network.go b/pkg/test/dockerutil/network.go
index 047091e75..dbe17fa5e 100644
--- a/pkg/test/dockerutil/network.go
+++ b/pkg/test/dockerutil/network.go
@@ -102,11 +102,8 @@ func (n *Network) Inspect(ctx context.Context) (types.NetworkResource, error) {
 	return n.client.NetworkInspect(ctx, n.id, types.NetworkInspectOptions{Verbose: true})
 }
 
-// Cleanup cleans up the docker network and all the containers attached to it.
+// Cleanup cleans up the docker network.
 func (n *Network) Cleanup(ctx context.Context) error {
-	for _, c := range n.containers {
-		c.CleanUp(ctx)
-	}
 	n.containers = nil
 
 	return n.client.NetworkRemove(ctx, n.id)
diff --git a/test/packetimpact/runner/dut.go b/test/packetimpact/runner/dut.go
index 1064ca976..b271bd47e 100644
--- a/test/packetimpact/runner/dut.go
+++ b/test/packetimpact/runner/dut.go
@@ -137,7 +137,7 @@ func setUpDUT(ctx context.Context, t *testing.T, id int, mkDevice func(*dockerut
 		dn := dn
 		t.Cleanup(func() {
 			if err := dn.Cleanup(ctx); err != nil {
-				t.Errorf("unable to cleanup container %s: %s", dn.Name, err)
+				t.Errorf("failed to cleanup network %s: %s", dn.Name, err)
 			}
 		})
 		// Sanity check.
@@ -151,13 +151,15 @@ func setUpDUT(ctx context.Context, t *testing.T, id int, mkDevice func(*dockerut
 	info.testNet = testNet
 
 	// Create the Docker container for the DUT.
-	var dut DUT
+	makeContainer := dockerutil.MakeContainer
 	if native {
-		dut = mkDevice(dockerutil.MakeNativeContainer(ctx, logger(fmt.Sprintf("dut-%d", id))))
-	} else {
-		dut = mkDevice(dockerutil.MakeContainer(ctx, logger(fmt.Sprintf("dut-%d", id))))
+		makeContainer = dockerutil.MakeNativeContainer
 	}
-	info.dut = dut
+	dutContainer := makeContainer(ctx, logger(fmt.Sprintf("dut-%d", id)))
+	t.Cleanup(func() {
+		dutContainer.CleanUp(ctx)
+	})
+	info.dut = mkDevice(dutContainer)
 
 	runOpts := dockerutil.RunOpts{
 		Image:  "packetimpact",
@@ -168,7 +170,7 @@ func setUpDUT(ctx context.Context, t *testing.T, id int, mkDevice func(*dockerut
 	}
 
 	ipv4PrefixLength, _ := testNet.Subnet.Mask.Size()
-	remoteIPv6, remoteMAC, dutDeviceID, dutTestNetDev, err := dut.Prepare(ctx, t, runOpts, ctrlNet, testNet)
+	remoteIPv6, remoteMAC, dutDeviceID, dutTestNetDev, err := info.dut.Prepare(ctx, t, runOpts, ctrlNet, testNet)
 	if err != nil {
 		return dutInfo{}, err
 	}
@@ -183,7 +185,7 @@ func setUpDUT(ctx context.Context, t *testing.T, id int, mkDevice func(*dockerut
 		POSIXServerIP:    AddressInSubnet(DUTAddr, *ctrlNet.Subnet),
 		POSIXServerPort:  CtrlPort,
 	}
-	info.uname, err = dut.Uname(ctx)
+	info.uname, err = info.dut.Uname(ctx)
 	if err != nil {
 		return dutInfo{}, fmt.Errorf("failed to get uname information on DUT: %w", err)
 	}
@@ -231,6 +233,9 @@ func TestWithDUT(ctx context.Context, t *testing.T, mkDevice func(*dockerutil.Co
 
 	// Create the Docker container for the testbench.
 	testbenchContainer := dockerutil.MakeNativeContainer(ctx, logger("testbench"))
+	t.Cleanup(func() {
+		testbenchContainer.CleanUp(ctx)
+	})
 
 	runOpts := dockerutil.RunOpts{
 		Image:  "packetimpact",
@@ -598,7 +603,6 @@ func createDockerNetwork(ctx context.Context, n *dockerutil.Network) error {
 func StartContainer(ctx context.Context, runOpts dockerutil.RunOpts, c *dockerutil.Container, containerAddr net.IP, ns []*dockerutil.Network, sysctls map[string]string, cmd ...string) error {
 	conf, hostconf, netconf := c.ConfigsFrom(runOpts, cmd...)
 	_ = netconf
-	hostconf.AutoRemove = true
 	hostconf.Sysctls = map[string]string{"net.ipv6.conf.all.disable_ipv6": "0"}
 	for k, v := range sysctls {
 		hostconf.Sysctls[k] = v
-- 
cgit v1.2.3