Skip to content

Commit

Permalink
always add short container id as net alias
Browse files Browse the repository at this point in the history
This matches what docker does. Also make sure the net aliases are also
shown when the container is stopped.

docker-compose uses this special alias entry to check if it is already
correctly connected to the network. [1]
Because we do not support static ips on network connect at the moment
calling disconnect && connect will loose the static ip.

Fixes containers#11748

[1] https://github.com/docker/compose/blob/0bea52b18dda3de8c28fcfb0c80cc08b8950645e/compose/service.py#L663-L667

Signed-off-by: Paul Holzinger <[email protected]>
  • Loading branch information
Luap99 committed Sep 28, 2021
1 parent e27470b commit 05614ee
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 9 deletions.
2 changes: 1 addition & 1 deletion libpod/container_internal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion libpod/network/types/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
47 changes: 44 additions & 3 deletions libpod/networking_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -1253,6 +1293,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,
Expand Down
1 change: 1 addition & 0 deletions test/compose/test-compose
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
18 changes: 14 additions & 4 deletions test/e2e/network_connect_disconnect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -146,14 +144,20 @@ 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()
Expect(con).Should(ExitWithError())
})

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()
Expand All @@ -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()
Expand All @@ -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))
Expand All @@ -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() {
Expand Down
8 changes: 8 additions & 0 deletions test/system/500-networking.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 05614ee

Please sign in to comment.