Refactor compat container create endpoint

* Make endpoint compatibile with docker-py network expectations
* Update specgen helper when called from compat endpoint
* Update godoc on types
* Add test for network/container create using docker-py method
* Add syslog logging when DEBUG=1 for tests

Fixes #8361

Signed-off-by: Jhon Honce <jhonce@redhat.com>

10 files changed, 151 insertions, 94 deletions

diff --git a/cmd/podman/common/create_opts.go b/cmd/podman/common/create_opts.go
index dc3202c7f..6dc43dbc6 100644
--- a/cmd/podman/common/create_opts.go
+++ b/cmd/podman/common/create_opts.go
@@ -134,7 +134,6 @@ func stringMaptoArray(m map[string]string) []string {
 // a specgen spec.
 func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroupsManager string) (*ContainerCLIOpts, []string, error) {
 	var (
-		aliases    []string
 		capAdd     []string
 		cappDrop   []string
 		entrypoint *string
@@ -212,7 +211,7 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
 		mounts = append(mounts, mount)
 	}
 
-	//volumes
+	// volumes
 	volumes := make([]string, 0, len(cc.Config.Volumes))
 	for v := range cc.Config.Volumes {
 		volumes = append(volumes, v)
@@ -242,16 +241,6 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
 		}
 	}
 
-	// network names
-	endpointsConfig := cc.NetworkingConfig.EndpointsConfig
-	cniNetworks := make([]string, 0, len(endpointsConfig))
-	for netName, endpoint := range endpointsConfig {
-		cniNetworks = append(cniNetworks, netName)
-		if len(endpoint.Aliases) > 0 {
-			aliases = append(aliases, endpoint.Aliases...)
-		}
-	}
-
 	// netMode
 	nsmode, _, err := specgen.ParseNetworkNamespace(cc.HostConfig.NetworkMode.NetworkName())
 	if err != nil {
@@ -268,8 +257,6 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
 	// defined when there is only one network.
 	netInfo := entities.NetOptions{
 		AddHosts:     cc.HostConfig.ExtraHosts,
-		Aliases:      aliases,
-		CNINetworks:  cniNetworks,
 		DNSOptions:   cc.HostConfig.DNSOptions,
 		DNSSearch:    cc.HostConfig.DNSSearch,
 		DNSServers:   dns,
@@ -277,31 +264,58 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
 		PublishPorts: specPorts,
 	}
 
-	// static IP and MAC
-	if len(endpointsConfig) == 1 {
-		for _, ep := range endpointsConfig {
-			// if IP address is provided
-			if len(ep.IPAddress) > 0 {
-				staticIP := net.ParseIP(ep.IPAddress)
-				netInfo.StaticIP = &staticIP
+	// network names
+	switch {
+	case len(cc.NetworkingConfig.EndpointsConfig) > 0:
+		var aliases []string
+
+		endpointsConfig := cc.NetworkingConfig.EndpointsConfig
+		cniNetworks := make([]string, 0, len(endpointsConfig))
+		for netName, endpoint := range endpointsConfig {
+
+			cniNetworks = append(cniNetworks, netName)
+
+			if endpoint == nil {
+				continue
+			}
+			if len(endpoint.Aliases) > 0 {
+				aliases = append(aliases, endpoint.Aliases...)
 			}
-			// If MAC address is provided
-			if len(ep.MacAddress) > 0 {
-				staticMac, err := net.ParseMAC(ep.MacAddress)
-				if err != nil {
-					return nil, nil, err
+		}
+
+		// static IP and MAC
+		if len(endpointsConfig) == 1 {
+			for _, ep := range endpointsConfig {
+				if ep == nil {
+					continue
+				}
+				// if IP address is provided
+				if len(ep.IPAddress) > 0 {
+					staticIP := net.ParseIP(ep.IPAddress)
+					netInfo.StaticIP = &staticIP
+				}
+				// If MAC address is provided
+				if len(ep.MacAddress) > 0 {
+					staticMac, err := net.ParseMAC(ep.MacAddress)
+					if err != nil {
+						return nil, nil, err
+					}
+					netInfo.StaticMAC = &staticMac
 				}
-				netInfo.StaticMAC = &staticMac
+				break
 			}
-			break
 		}
+		netInfo.Aliases = aliases
+		netInfo.CNINetworks = cniNetworks
+	case len(cc.HostConfig.NetworkMode) > 0:
+		netInfo.CNINetworks = []string{string(cc.HostConfig.NetworkMode)}
 	}
 
 	// Note: several options here are marked as "don't need". this is based
 	// on speculation by Matt and I. We think that these come into play later
 	// like with start. We believe this is just a difference in podman/compat
 	cliOpts := ContainerCLIOpts{
-		//Attach:            nil, // dont need?
+		// Attach:            nil, // dont need?
 		Authfile:     "",
 		CapAdd:       append(capAdd, cc.HostConfig.CapAdd...),
 		CapDrop:      append(cappDrop, cc.HostConfig.CapDrop...),
@@ -312,11 +326,11 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
 		CPURTPeriod:  uint64(cc.HostConfig.CPURealtimePeriod),
 		CPURTRuntime: cc.HostConfig.CPURealtimeRuntime,
 		CPUShares:    uint64(cc.HostConfig.CPUShares),
-		//CPUS:              0, // dont need?
+		// CPUS:              0, // dont need?
 		CPUSetCPUs: cc.HostConfig.CpusetCpus,
 		CPUSetMems: cc.HostConfig.CpusetMems,
-		//Detach:            false, // dont need
-		//DetachKeys:        "",    // dont need
+		// Detach:            false, // dont need
+		// DetachKeys:        "",    // dont need
 		Devices:          devices,
 		DeviceCGroupRule: nil,
 		DeviceReadBPs:    readBps,
@@ -438,7 +452,7 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, cgroup
 	}
 
 	// specgen assumes the image name is arg[0]
-	cmd := []string{cc.Image}
+	cmd := []string{cc.Config.Image}
 	cmd = append(cmd, cc.Config.Cmd...)
 	return &cliOpts, cmd, nil
 }
diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index 7a0bebd95..baa11935d 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -900,7 +900,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
 	// If we have CNI networks - handle that here
 	if len(networks) > 0 && !isDefault {
 		if len(networks) != len(c.state.NetworkStatus) {
-			return nil, errors.Wrapf(define.ErrInternal, "network inspection mismatch: asked to join %d CNI networks but have information on %d networks", len(networks), len(c.state.NetworkStatus))
+			return nil, errors.Wrapf(define.ErrInternal, "network inspection mismatch: asked to join %d CNI network(s) %v, but have information on %d network(s)", len(networks), networks, len(c.state.NetworkStatus))
 		}
 
 		settings.Networks = make(map[string]*define.InspectAdditionalNetwork)
diff --git a/pkg/api/handlers/compat/containers_create.go b/pkg/api/handlers/compat/containers_create.go
index 4efe770b3..729639928 100644
--- a/pkg/api/handlers/compat/containers_create.go
+++ b/pkg/api/handlers/compat/containers_create.go
@@ -19,7 +19,6 @@ import (
 func CreateContainer(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value("runtime").(*libpod.Runtime)
 	decoder := r.Context().Value("decoder").(*schema.Decoder)
-	input := handlers.CreateContainerConfig{}
 	query := struct {
 		Name string `schema:"name"`
 	}{
@@ -30,11 +29,15 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 			errors.Wrapf(err, "failed to parse parameters for %s", r.URL.String()))
 		return
 	}
-	if err := json.NewDecoder(r.Body).Decode(&input); err != nil {
+
+	// compatible configuration
+	body := handlers.CreateContainerConfig{}
+	if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "Decode()"))
 		return
 	}
-	if len(input.HostConfig.Links) > 0 {
+
+	if len(body.HostConfig.Links) > 0 {
 		utils.Error(w, utils.ErrLinkNotSupport.Error(), http.StatusBadRequest, errors.Wrapf(utils.ErrLinkNotSupport, "bad parameter"))
 		return
 	}
@@ -43,7 +46,7 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 		utils.Error(w, "unable to obtain runtime config", http.StatusInternalServerError, errors.Wrap(err, "unable to get runtime config"))
 	}
 
-	newImage, err := runtime.ImageRuntime().NewFromLocal(input.Image)
+	newImage, err := runtime.ImageRuntime().NewFromLocal(body.Config.Image)
 	if err != nil {
 		if errors.Cause(err) == define.ErrNoSuchImage {
 			utils.Error(w, "No such image", http.StatusNotFound, err)
@@ -54,11 +57,8 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// Add the container name to the input struct
-	input.Name = query.Name
-
-	// Take input structure and convert to cliopts
-	cliOpts, args, err := common.ContainerCreateToContainerCLIOpts(input, rtc.Engine.CgroupManager)
+	// Take body structure and convert to cliopts
+	cliOpts, args, err := common.ContainerCreateToContainerCLIOpts(body, rtc.Engine.CgroupManager)
 	if err != nil {
 		utils.Error(w, "Something went wrong.", http.StatusInternalServerError, errors.Wrap(err, "make cli opts()"))
 		return
@@ -69,6 +69,9 @@ func CreateContainer(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Override the container name in the body struct
+	body.Name = query.Name
+
 	ic := abi.ContainerEngine{Libpod: runtime}
 	report, err := ic.ContainerCreate(r.Context(), sg)
 	if err != nil {
diff --git a/pkg/api/handlers/types.go b/pkg/api/handlers/types.go
index 6bb5f5101..40cf16807 100644
--- a/pkg/api/handlers/types.go
+++ b/pkg/api/handlers/types.go
@@ -110,11 +110,12 @@ type ContainerWaitOKBody struct {
 	}
 }
 
+// CreateContainerConfig used when compatible endpoint creates a container
 type CreateContainerConfig struct {
-	Name string
-	dockerContainer.Config
-	HostConfig       dockerContainer.HostConfig
-	NetworkingConfig dockerNetwork.NetworkingConfig
+	Name                   string                         // container name
+	dockerContainer.Config                                // desired container configuration
+	HostConfig             dockerContainer.HostConfig     // host dependent configuration for container
+	NetworkingConfig       dockerNetwork.NetworkingConfig // network configuration for container
 }
 
 // swagger:model IDResponse
@@ -253,7 +254,7 @@ func ImageDataToImageInspect(ctx context.Context, l *libpodImage.Image) (*ImageI
 		//	StdinOnce:       false,
 		Env: info.Config.Env,
 		Cmd: info.Config.Cmd,
-		//Healthcheck: l.ImageData.HealthCheck,
+		// Healthcheck: l.ImageData.HealthCheck,
 		//	ArgsEscaped:     false,
 		//	Image:           "",
 		Volumes:    info.Config.Volumes,
@@ -261,7 +262,7 @@ func ImageDataToImageInspect(ctx context.Context, l *libpodImage.Image) (*ImageI
 		Entrypoint: info.Config.Entrypoint,
 		//	NetworkDisabled: false,
 		//	MacAddress:      "",
-		//OnBuild:    info.Config.OnBuild,
+		// OnBuild:    info.Config.OnBuild,
 		Labels:     info.Labels,
 		StopSignal: info.Config.StopSignal,
 		//	StopTimeout:     nil,
diff --git a/test/apiv2/rest_api/__init__.py b/test/apiv2/rest_api/__init__.py
index 8100a4df5..db0257f03 100644
--- a/test/apiv2/rest_api/__init__.py
+++ b/test/apiv2/rest_api/__init__.py
@@ -16,19 +16,18 @@ class Podman(object):
         binary = os.getenv("PODMAN", "bin/podman")
         self.cmd = [binary, "--storage-driver=vfs"]
 
-        cgroupfs = os.getenv("CGROUP_MANAGER", "cgroupfs")
+        cgroupfs = os.getenv("CGROUP_MANAGER", "systemd")
         self.cmd.append(f"--cgroup-manager={cgroupfs}")
 
         if os.getenv("DEBUG"):
             self.cmd.append("--log-level=debug")
+            self.cmd.append("--syslog=true")
 
         self.anchor_directory = tempfile.mkdtemp(prefix="podman_restapi_")
         self.cmd.append("--root=" + os.path.join(self.anchor_directory, "crio"))
         self.cmd.append("--runroot=" + os.path.join(self.anchor_directory, "crio-run"))
 
-        os.environ["REGISTRIES_CONFIG_PATH"] = os.path.join(
-            self.anchor_directory, "registry.conf"
-        )
+        os.environ["REGISTRIES_CONFIG_PATH"] = os.path.join(self.anchor_directory, "registry.conf")
         p = configparser.ConfigParser()
         p.read_dict(
             {
@@ -40,14 +39,10 @@ class Podman(object):
         with open(os.environ["REGISTRIES_CONFIG_PATH"], "w") as w:
             p.write(w)
 
-        os.environ["CNI_CONFIG_PATH"] = os.path.join(
-            self.anchor_directory, "cni", "net.d"
-        )
+        os.environ["CNI_CONFIG_PATH"] = os.path.join(self.anchor_directory, "cni", "net.d")
         os.makedirs(os.environ["CNI_CONFIG_PATH"], exist_ok=True)
         self.cmd.append("--cni-config-dir=" + os.environ["CNI_CONFIG_PATH"])
-        cni_cfg = os.path.join(
-            os.environ["CNI_CONFIG_PATH"], "87-podman-bridge.conflist"
-        )
+        cni_cfg = os.path.join(os.environ["CNI_CONFIG_PATH"], "87-podman-bridge.conflist")
         # json decoded and encoded to ensure legal json
         buf = json.loads(
             """
diff --git a/test/apiv2/rest_api/test_rest_v2_0_0.py b/test/apiv2/rest_api/test_rest_v2_0_0.py
index 49e18f063..52348d4f4 100644
--- a/test/apiv2/rest_api/test_rest_v2_0_0.py
+++ b/test/apiv2/rest_api/test_rest_v2_0_0.py
@@ -61,9 +61,7 @@ class TestApi(unittest.TestCase):
         super().setUpClass()
 
         TestApi.podman = Podman()
-        TestApi.service = TestApi.podman.open(
-            "system", "service", "tcp:localhost:8080", "--time=0"
-        )
+        TestApi.service = TestApi.podman.open("system", "service", "tcp:localhost:8080", "--time=0")
         # give the service some time to be ready...
         time.sleep(2)
 
@@ -165,11 +163,71 @@ class TestApi(unittest.TestCase):
         r = requests.get(_url(ctnr("/containers/{}/logs?stdout=true")))
         self.assertEqual(r.status_code, 200, r.text)
 
-    def test_post_create_compat(self):
+    # TODO Need to support Docker-py order of network/container creates
+    def test_post_create_compat_connect(self):
         """Create network and container then connect to network"""
-        net = requests.post(
-            PODMAN_URL + "/v1.40/networks/create", json={"Name": "TestNetwork"}
+        net_default = requests.post(
+            PODMAN_URL + "/v1.40/networks/create", json={"Name": "TestDefaultNetwork"}
+        )
+        self.assertEqual(net_default.status_code, 201, net_default.text)
+
+        create = requests.post(
+            PODMAN_URL + "/v1.40/containers/create?name=postCreate",
+            json={
+                "Cmd": ["top"],
+                "Image": "alpine:latest",
+                "NetworkDisabled": False,
+                # FIXME adding these 2 lines cause: (This is sampled from docker-py)
+                #   "network already exists","message":"container
+                #  01306e499df5441560d70071a54342611e422a94de20865add50a9565fd79fb9 is already connected to CNI network \"TestDefaultNetwork\": network already exists"
+                # "HostConfig": {"NetworkMode": "TestDefaultNetwork"},
+                # "NetworkingConfig": {"EndpointsConfig": {"TestDefaultNetwork": None}},
+                # FIXME These two lines cause:
+                # CNI network \"TestNetwork\" not found","message":"error configuring network namespace for container 369ddfa7d3211ebf1fbd5ddbff91bd33fa948858cea2985c133d6b6507546dff: CNI network \"TestNetwork\" not found"
+                # "HostConfig": {"NetworkMode": "TestNetwork"},
+                # "NetworkingConfig": {"EndpointsConfig": {"TestNetwork": None}},
+                # FIXME no networking defined cause: (note this error is from the container inspect below)
+                # "internal libpod error","message":"network inspection mismatch: asked to join 2 CNI network(s) [TestDefaultNetwork podman], but have information on 1 network(s): internal libpod error"
+            },
+        )
+        self.assertEqual(create.status_code, 201, create.text)
+        payload = json.loads(create.text)
+        self.assertIsNotNone(payload["Id"])
+
+        start = requests.post(PODMAN_URL + f"/v1.40/containers/{payload['Id']}/start")
+        self.assertEqual(start.status_code, 204, start.text)
+
+        connect = requests.post(
+            PODMAN_URL + "/v1.40/networks/TestDefaultNetwork/connect",
+            json={"Container": payload["Id"]},
+        )
+        self.assertEqual(connect.status_code, 200, connect.text)
+        self.assertEqual(connect.text, "OK\n")
+
+        inspect = requests.get(f"{PODMAN_URL}/v1.40/containers/{payload['Id']}/json")
+        self.assertEqual(inspect.status_code, 200, inspect.text)
+
+        payload = json.loads(inspect.text)
+        self.assertFalse(payload["Config"].get("NetworkDisabled", False))
+
+        self.assertEqual(
+            "TestDefaultNetwork",
+            payload["NetworkSettings"]["Networks"]["TestDefaultNetwork"]["NetworkID"],
         )
+        # TODO restore this to test, when joining multiple networks possible
+        # self.assertEqual(
+        #     "TestNetwork",
+        #     payload["NetworkSettings"]["Networks"]["TestNetwork"]["NetworkID"],
+        # )
+        # TODO Need to support network aliases
+        # self.assertIn(
+        #     "test_post_create",
+        #     payload["NetworkSettings"]["Networks"]["TestNetwork"]["Aliases"],
+        # )
+
+    def test_post_create_compat(self):
+        """Create network and connect container during create"""
+        net = requests.post(PODMAN_URL + "/v1.40/networks/create", json={"Name": "TestNetwork"})
         self.assertEqual(net.status_code, 201, net.text)
 
         create = requests.post(
@@ -178,23 +236,21 @@ class TestApi(unittest.TestCase):
                 "Cmd": ["date"],
                 "Image": "alpine:latest",
                 "NetworkDisabled": False,
-                "NetworkConfig": {
-                    "EndpointConfig": {"TestNetwork": {"Aliases": ["test_post_create"]}}
-                },
+                "HostConfig": {"NetworkMode": "TestNetwork"},
             },
         )
         self.assertEqual(create.status_code, 201, create.text)
         payload = json.loads(create.text)
         self.assertIsNotNone(payload["Id"])
 
-        # This cannot be done until full completion of the network connect
-        # stack and network disconnect stack are complete
-        # connect = requests.post(
-        #     PODMAN_URL + "/v1.40/networks/TestNetwork/connect",
-        #     json={"Container": payload["Id"]},
-        # )
-        # self.assertEqual(connect.status_code, 200, connect.text)
-        # self.assertEqual(connect.text, "OK\n")
+        inspect = requests.get(f"{PODMAN_URL}/v1.40/containers/{payload['Id']}/json")
+        self.assertEqual(inspect.status_code, 200, inspect.text)
+        payload = json.loads(inspect.text)
+        self.assertFalse(payload["Config"].get("NetworkDisabled", False))
+        self.assertEqual(
+            "TestNetwork",
+            payload["NetworkSettings"]["Networks"]["TestNetwork"]["NetworkID"],
+        )
 
     def test_commit(self):
         r = requests.post(_url(ctnr("/commit?container={}")))
diff --git a/test/apiv2/rest_api/v1_test_rest_v1_0_0.py b/test/apiv2/rest_api/v1_test_rest_v1_0_0.py
index acd6273ef..23528a246 100644
--- a/test/apiv2/rest_api/v1_test_rest_v1_0_0.py
+++ b/test/apiv2/rest_api/v1_test_rest_v1_0_0.py
@@ -84,9 +84,7 @@ class TestApi(unittest.TestCase):
             print("\nService Stderr:\n" + stderr.decode("utf-8"))
 
         if TestApi.podman.returncode > 0:
-            sys.stderr.write(
-                "podman exited with error code {}\n".format(TestApi.podman.returncode)
-            )
+            sys.stderr.write("podman exited with error code {}\n".format(TestApi.podman.returncode))
             sys.exit(2)
 
         return super().tearDownClass()
diff --git a/test/python/docker/__init__.py b/test/python/docker/__init__.py
index 316b102f4..a52f0742c 100644
--- a/test/python/docker/__init__.py
+++ b/test/python/docker/__init__.py
@@ -39,9 +39,7 @@ class Podman(object):
         self.cmd.append("--root=" + os.path.join(self.anchor_directory, "crio"))
         self.cmd.append("--runroot=" + os.path.join(self.anchor_directory, "crio-run"))
 
-        os.environ["REGISTRIES_CONFIG_PATH"] = os.path.join(
-            self.anchor_directory, "registry.conf"
-        )
+        os.environ["REGISTRIES_CONFIG_PATH"] = os.path.join(self.anchor_directory, "registry.conf")
         p = configparser.ConfigParser()
         p.read_dict(
             {
@@ -53,14 +51,10 @@ class Podman(object):
         with open(os.environ["REGISTRIES_CONFIG_PATH"], "w") as w:
             p.write(w)
 
-        os.environ["CNI_CONFIG_PATH"] = os.path.join(
-            self.anchor_directory, "cni", "net.d"
-        )
+        os.environ["CNI_CONFIG_PATH"] = os.path.join(self.anchor_directory, "cni", "net.d")
         os.makedirs(os.environ["CNI_CONFIG_PATH"], exist_ok=True)
         self.cmd.append("--cni-config-dir=" + os.environ["CNI_CONFIG_PATH"])
-        cni_cfg = os.path.join(
-            os.environ["CNI_CONFIG_PATH"], "87-podman-bridge.conflist"
-        )
+        cni_cfg = os.path.join(os.environ["CNI_CONFIG_PATH"], "87-podman-bridge.conflist")
         # json decoded and encoded to ensure legal json
         buf = json.loads(
             """
diff --git a/test/python/docker/common.py b/test/python/docker/common.py
index e79d64a9b..11f512495 100644
--- a/test/python/docker/common.py
+++ b/test/python/docker/common.py
@@ -4,9 +4,7 @@ from test.python.docker import constant
 
 
 def run_top_container(client: DockerClient):
-    c = client.containers.create(
-        constant.ALPINE, command="top", detach=True, tty=True, name="top"
-    )
+    c = client.containers.create(constant.ALPINE, command="top", detach=True, tty=True, name="top")
     c.start()
     return c.id
 
diff --git a/test/python/docker/test_images.py b/test/python/docker/test_images.py
index 7ef3d708b..1fa4aade9 100644
--- a/test/python/docker/test_images.py
+++ b/test/python/docker/test_images.py
@@ -78,9 +78,7 @@ class TestImages(unittest.TestCase):
         self.assertEqual(len(self.client.images.list()), 2)
 
         # List images with filter
-        self.assertEqual(
-            len(self.client.images.list(filters={"reference": "alpine"})), 1
-        )
+        self.assertEqual(len(self.client.images.list(filters={"reference": "alpine"})), 1)
 
     def test_search_image(self):
         """Search for image"""