Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

always add short container id as net alias #11751

Merged
merged 3 commits into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
3 changes: 0 additions & 3 deletions libpod/network/cni/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
41 changes: 20 additions & 21 deletions libpod/network/cni/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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{
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
55 changes: 52 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 @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 15 additions & 7 deletions libpod/runtime_ctr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
1 change: 1 addition & 0 deletions pkg/specgen/generate/container_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}...)
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