Skip to content

Commit

Permalink
Fix #15499 already connected network
Browse files Browse the repository at this point in the history
Compat: Treat already attached networks as a no-op
Applies only to containers in created state. Maintain error in running state.

Co-authored-by: Alessandro Rossi <[email protected]>
Co-authored-by: Brent Baude <[email protected]>
Co-authored-by: Jason T. Greene <[email protected]>
Signed-off-by: Alessandro Rossi <[email protected]>
Signed-off-by: Jason T. Greene <[email protected]>
  • Loading branch information
3 people committed Aug 28, 2022
1 parent 03e51a0 commit 78aec21
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 3 deletions.
2 changes: 1 addition & 1 deletion libpod/boltdb_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,7 @@ func (s *BoltState) NetworkConnect(ctr *Container, network string, opts types.Pe
}
netConnected := ctrNetworksBkt.Get([]byte(network))
if netConnected != nil {
return fmt.Errorf("container %s is already connected to network %q: %w", ctr.ID(), network, define.ErrNetworkExists)
return fmt.Errorf("container %s is already connected to network %q: %w", ctr.ID(), network, define.ErrNetworkConnected)
}

// Add the network
Expand Down
3 changes: 3 additions & 0 deletions libpod/define/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ var (
// ErrNetworkInUse indicates the requested operation failed because the network was in use
ErrNetworkInUse = errors.New("network is being used")

// ErrNetworkConnected indicates that the required operation failed because the container is already a network endpoint
ErrNetworkConnected = errors.New("network is already connected")

// ErrStoreNotInitialized indicates that the container storage was never
// initialized.
ErrStoreNotInitialized = errors.New("the container storage was never initialized")
Expand Down
5 changes: 5 additions & 0 deletions libpod/networking_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,11 @@ func (c *Container) NetworkConnect(nameOrID, netName string, netOpts types.PerNe
}

if err := c.runtime.state.NetworkConnect(c, netName, netOpts); err != nil {
// Docker compat: treat requests to attach already attached networks as a no-op, ignoring opts
if errors.Is(err, define.ErrNetworkConnected) && c.ensureState(define.ContainerStateConfigured) {
return nil
}

return err
}
c.newNetworkEvent(events.NetworkConnect, netName)
Expand Down
12 changes: 10 additions & 2 deletions test/e2e/network_connect_disconnect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ var _ = Describe("Podman network connect and disconnect", func() {
Expect(con.ErrorToString()).To(ContainSubstring(`"slirp4netns" is not supported: invalid network mode`))
})

It("podman connect on a container that already is connected to the network should error", func() {
It("podman connect on a container that already is connected to the network should error after init", func() {
netName := "aliasTest" + stringid.GenerateNonCryptoID()
session := podmanTest.Podman([]string{"network", "create", netName})
session.WaitWithDefaultTimeout()
Expand All @@ -177,7 +177,15 @@ var _ = Describe("Podman network connect and disconnect", func() {

con := podmanTest.Podman([]string{"network", "connect", netName, "test"})
con.WaitWithDefaultTimeout()
Expect(con).Should(ExitWithError())
Expect(con).Should(Exit(0))

init := podmanTest.Podman([]string{"init", "test"})
init.WaitWithDefaultTimeout()
Expect(init).Should(Exit(0))

con2 := podmanTest.Podman([]string{"network", "connect", netName, "test"})
con2.WaitWithDefaultTimeout()
Expect(con2).Should(ExitWithError())
})

It("podman network connect", func() {
Expand Down

0 comments on commit 78aec21

Please sign in to comment.