Merge pull request #11751 from Luap99/net-alias

always add short container id as net alias

libpod/container_internal_linux.go

@@ -1310,7 +1310,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
 	if err == nil && options.Name == "" && (!options.IgnoreStaticIP || !options.IgnoreStaticMAC) {
 		// The file with the network.status does exist. Let's restore the
 		// container with the same networks settings as during checkpointing.
-		aliases, err := c.runtime.state.GetAllNetworkAliases(c)
+		aliases, err := c.GetAllNetworkAliases()
 		if err != nil {
 			return err
 		}

libpod/network/cni/run.go

@@ -186,9 +186,6 @@ outer:
 		}
 		return errors.Errorf("requested static ip %s not in any subnet on network %s", ip.String(), network.libpodNet.Name)
 	}
-	if len(netOpts.Aliases) > 0 && !network.libpodNet.DNSEnabled {
-		return errors.New("cannot set aliases on a network without dns enabled")
-	}
 	return nil
 }
 

libpod/network/cni/run_test.go

@@ -966,6 +966,26 @@ var _ = Describe("run CNI", func() {
 			})
 		})
 
+		It("setup with aliases but dns disabled should work", func() {
+			runTest(func() {
+				defNet := types.DefaultNetworkName
+				intName := "eth0"
+				setupOpts := types.SetupOptions{
+					NetworkOptions: types.NetworkOptions{
+						ContainerID: stringid.GenerateNonCryptoID(),
+						Networks: map[string]types.PerNetworkOptions{
+							defNet: {
+								InterfaceName: intName,
+								Aliases:       []string{"somealias"},
+							},
+						},
+					},
+				}
+				_, err := libpodNet.Setup(netNSContainer.Path(), setupOpts)
+				Expect(err).ToNot(HaveOccurred())
+			})
+		})
+
 	})
 
 	Context("invalid network setup test", func() {
@@ -1052,27 +1072,6 @@ var _ = Describe("run CNI", func() {
 			})
 		})
 
-		It("setup with aliases but dns disabled", func() {
-			runTest(func() {
-				defNet := types.DefaultNetworkName
-				intName := "eth0"
-				setupOpts := types.SetupOptions{
-					NetworkOptions: types.NetworkOptions{
-						ContainerID: stringid.GenerateNonCryptoID(),
-						Networks: map[string]types.PerNetworkOptions{
-							defNet: {
-								InterfaceName: intName,
-								Aliases:       []string{"somealias"},
-							},
-						},
-					},
-				}
-				_, err := libpodNet.Setup(netNSContainer.Path(), setupOpts)
-				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("cannot set aliases on a network without dns enabled"))
-			})
-		})
-
 		It("setup without networks", func() {
 			runTest(func() {
 				setupOpts := types.SetupOptions{

libpod/network/types/network.go

@@ -151,7 +151,9 @@ type PerNetworkOptions struct {
 	// StaticIPv4 for this container. Optional.
 	StaticIPs []net.IP `json:"static_ips,omitempty"`
 	// Aliases contains a list of names which the dns server should resolve
-	// to this container. Can only be set when DNSEnabled is true on the Network.
+	// to this container. Should only be set when DNSEnabled is true on the Network.
+	// If aliases are set but there is no dns support for this network the
+	// network interface implementation should ignore this and NOT error.
 	// Optional.
 	Aliases []string `json:"aliases,omitempty"`
 	// StaticMac for this container. Optional.

libpod/networking_linux.go

@@ -48,6 +48,41 @@ const (
 	persistentCNIDir = "/var/lib/cni"
 )
 
+// GetAllNetworkAliases returns all configured aliases for this container.
+// It also adds the container short ID as alias to match docker.
+func (c *Container) GetAllNetworkAliases() (map[string][]string, error) {
+	allAliases, err := c.runtime.state.GetAllNetworkAliases(c)
+	if err != nil {
+		return nil, err
+	}
+
+	// get the all attached networks, we cannot use GetAllNetworkAliases()
+	// since it returns nil if there are no aliases
+	nets, _, err := c.networks()
+	if err != nil {
+		return nil, err
+	}
+
+	// add container short ID as alias to match docker
+	for _, net := range nets {
+		allAliases[net] = append(allAliases[net], c.config.ID[:12])
+	}
+	return allAliases, nil
+}
+
+// GetNetworkAliases returns configured aliases for this network.
+// It also adds the container short ID as alias to match docker.
+func (c *Container) GetNetworkAliases(netName string) ([]string, error) {
+	aliases, err := c.runtime.state.GetNetworkAliases(c, netName)
+	if err != nil {
+		return nil, err
+	}
+
+	// add container short ID as alias to match docker
+	aliases = append(aliases, c.config.ID[:12])
+	return aliases, nil
+}
+
 func (c *Container) getNetworkOptions() (types.NetworkOptions, error) {
 	opts := types.NetworkOptions{
 		ContainerID:   c.config.ID,
@@ -61,7 +96,7 @@ func (c *Container) getNetworkOptions() (types.NetworkOptions, error) {
 	if err != nil {
 		return opts, err
 	}
-	aliases, err := c.runtime.state.GetAllNetworkAliases(c)
+	aliases, err := c.GetAllNetworkAliases()
 	if err != nil {
 		return opts, err
 	}
@@ -872,7 +907,7 @@ func (r *Runtime) reloadContainerNetwork(ctr *Container) (map[string]types.Statu
 		}
 	}
 
-	aliases, err := ctr.runtime.state.GetAllNetworkAliases(ctr)
+	aliases, err := ctr.GetAllNetworkAliases()
 	if err != nil {
 		return nil, err
 	}
@@ -975,6 +1010,11 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
 			for _, net := range networks {
 				cniNet := new(define.InspectAdditionalNetwork)
 				cniNet.NetworkID = net
+				aliases, err := c.GetNetworkAliases(net)
+				if err != nil {
+					return nil, err
+				}
+				cniNet.Aliases = aliases
 				settings.Networks[net] = cniNet
 			}
 		}
@@ -1009,7 +1049,7 @@ func (c *Container) getContainerNetworkInfo() (*define.InspectNetworkSettings, e
 				return nil, err
 			}
 
-			aliases, err := c.runtime.state.GetNetworkAliases(c, name)
+			aliases, err := c.GetNetworkAliases(name)
 			if err != nil {
 				return nil, err
 			}
@@ -1222,6 +1262,14 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e
 	// get network status before we connect
 	networkStatus := c.getNetworkStatus()
 
+	network, err := c.runtime.network.NetworkInspect(netName)
+	if err != nil {
+		return err
+	}
+	if !network.DNSEnabled && len(aliases) > 0 {
+		return errors.Wrapf(define.ErrInvalidArg, "cannot set network aliases for network %q because dns is disabled", netName)
+	}
+
 	if err := c.runtime.state.NetworkConnect(c, netName, aliases); err != nil {
 		return err
 	}
@@ -1253,6 +1301,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, aliases []string) e
 	if !exists {
 		return errors.Errorf("no network interface name for container %s on network %s", c.config.ID, netName)
 	}
+	aliases = append(aliases, c.config.ID[:12])
 	opts.Networks = map[string]types.PerNetworkOptions{
 		netName: {
 			Aliases:       aliases,

libpod/runtime_ctr.go

@@ -234,13 +234,6 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options ..
 }
 
 func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Container, retErr error) {
-	// Validate the container
-	if err := ctr.validate(); err != nil {
-		return nil, err
-	}
-	if ctr.config.IsInfra {
-		ctr.config.StopTimeout = 10
-	}
 	// normalize the networks to names
 	// ocicni only knows about cni names so we have to make
 	// sure we do not use ids internally
@@ -265,11 +258,26 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai
 			if err != nil {
 				return nil, err
 			}
+			network, err := r.network.NetworkInspect(netName)
+			if err != nil {
+				return nil, err
+			}
+			if !network.DNSEnabled {
+				return nil, errors.Wrapf(define.ErrInvalidArg, "cannot set network aliases for network %q because dns is disabled", netName)
+			}
 			netAliases[netName] = aliases
 		}
 		ctr.config.NetworkAliases = netAliases
 	}
 
+	// Validate the container
+	if err := ctr.validate(); err != nil {
+		return nil, err
+	}
+	if ctr.config.IsInfra {
+		ctr.config.StopTimeout = 10
+	}
+
 	// Inhibit shutdown until creation succeeds
 	shutdown.Inhibit()
 	defer shutdown.Uninhibit()

pkg/specgen/generate/container_create.go

@@ -498,6 +498,7 @@ func CreateExitCommandArgs(storageConfig types.StoreOptions, config *config.Conf
 		"--log-level", logrus.GetLevel().String(),
 		"--cgroup-manager", config.Engine.CgroupManager,
 		"--tmpdir", config.Engine.TmpDir,
+		"--cni-config-dir", config.Network.NetworkConfigDir,
 	}
 	if config.Engine.OCIRuntime != "" {
 		command = append(command, []string{"--runtime", config.Engine.OCIRuntime}...)

test/compose/test-compose

@@ -247,6 +247,7 @@ function podman() {
 	--storage-driver=vfs \
         --root    $WORKDIR/root    \
         --runroot $WORKDIR/runroot \
+        --cni-config-dir $WORKDIR/cni \
         "$@")
     echo -n "$output" >>$WORKDIR/output.log
 }

test/e2e/network_connect_disconnect_test.go

@@ -52,7 +52,6 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("network disconnect with net mode slirp4netns should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootful")
 		netName := "slirp" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName})
 		session.WaitWithDefaultTimeout()
@@ -118,7 +117,6 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("network connect with net mode slirp4netns should result in error", func() {
-		SkipIfRootless("network connect and disconnect are only rootful")
 		netName := "slirp" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName})
 		session.WaitWithDefaultTimeout()
@@ -146,6 +144,13 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		ctr := podmanTest.Podman([]string{"create", "--name", "test", "--network", netName, ALPINE, "top"})
 		ctr.WaitWithDefaultTimeout()
 		Expect(ctr).Should(Exit(0))
+		cid := ctr.OutputToString()
+
+		// network alias container short id is always added and shown in inspect
+		inspect := podmanTest.Podman([]string{"container", "inspect", "test", "--format", "{{(index .NetworkSettings.Networks \"" + netName + "\").Aliases}}"})
+		inspect.WaitWithDefaultTimeout()
+		Expect(inspect).Should(Exit(0))
+		Expect(inspect.OutputToString()).To(Equal("[" + cid[0:12] + "]"))
 
 		con := podmanTest.Podman([]string{"network", "connect", netName, "test"})
 		con.WaitWithDefaultTimeout()
@@ -153,7 +158,6 @@ var _ = Describe("Podman network connect and disconnect", func() {
 	})
 
 	It("podman network connect", func() {
-		SkipIfRemote("This requires a pending PR to be merged before it will work")
 		netName := "aliasTest" + stringid.GenerateNonCryptoID()
 		session := podmanTest.Podman([]string{"network", "create", netName})
 		session.WaitWithDefaultTimeout()
@@ -163,6 +167,7 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		ctr := podmanTest.Podman([]string{"run", "-dt", "--name", "test", "--network", netName, ALPINE, "top"})
 		ctr.WaitWithDefaultTimeout()
 		Expect(ctr).Should(Exit(0))
+		cid := ctr.OutputToString()
 
 		exec := podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth0"})
 		exec.WaitWithDefaultTimeout()
@@ -184,6 +189,12 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		Expect(inspect).Should(Exit(0))
 		Expect(inspect.OutputToString()).To(Equal("2"))
 
+		// network alias container short id is always added and shown in inspect
+		inspect = podmanTest.Podman([]string{"container", "inspect", "test", "--format", "{{(index .NetworkSettings.Networks \"" + newNetName + "\").Aliases}}"})
+		inspect.WaitWithDefaultTimeout()
+		Expect(inspect).Should(Exit(0))
+		Expect(inspect.OutputToString()).To(Equal("[" + cid[0:12] + "]"))
+
 		exec = podmanTest.Podman([]string{"exec", "-it", "test", "ip", "addr", "show", "eth1"})
 		exec.WaitWithDefaultTimeout()
 		Expect(exec).Should(Exit(0))
@@ -193,7 +204,6 @@ var _ = Describe("Podman network connect and disconnect", func() {
 		rm.WaitWithDefaultTimeout()
 		Expect(rm).Should(Exit(0))
 		Expect(rm.ErrorToString()).To(Equal(""))
-
 	})
 
 	It("podman network connect when not running", func() {

test/system/500-networking.bats

@@ -415,6 +415,10 @@ load helpers
     run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname\").MacAddress}}"
     mac="$output"
 
+    # check network alias for container short id
+    run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname\").Aliases}}"
+    is "$output" "\[${cid:0:12}\]" "short container id in network aliases"
+
     run_podman network disconnect $netname $cid
 
     # check that we cannot curl (timeout after 3 sec)
@@ -443,6 +447,10 @@ load helpers
     # connect a second network
     run_podman network connect $netname2 $cid
 
+    # check network2 alias for container short id
+    run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname2\").Aliases}}"
+    is "$output" "\[${cid:0:12}\]" "short container id in network aliases"
+
     # curl should work
     run curl --max-time 3 -s $SERVER/index.txt
     is "$output" "$random_1" "curl 127.0.0.1:/index.txt should work"