Skip to content

Commit

Permalink
Merge pull request #47196 from akerouanton/25.0-fix-multiple-rename-e…
Browse files Browse the repository at this point in the history
…rror

[25.0] daemon: rename: don't reload endpoint from datastore
  • Loading branch information
thaJeztah committed Jan 23, 2024
2 parents 6eef840 + 5295e88 commit 71fa3ab
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 11 deletions.
7 changes: 4 additions & 3 deletions daemon/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package daemon // import "github.com/docker/docker/daemon"

import (
"context"
"fmt"
"strings"

"github.com/containerd/log"
Expand Down Expand Up @@ -127,9 +128,9 @@ func (daemon *Daemon) ContainerRename(oldName, newName string) (retErr error) {
return err
}

ep, err := nw.EndpointByID(epConfig.EndpointID)
if err != nil {
return err
ep := sb.GetEndpoint(epConfig.EndpointID)
if ep == nil {
return fmt.Errorf("no endpoint attached to network %s found", nw.Name())
}

oldDNSNames := make([]string, len(epConfig.DNSNames))
Expand Down
22 changes: 22 additions & 0 deletions integration/container/rename_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,25 @@ func TestRenameContainerWithLinkedContainer(t *testing.T) {
assert.NilError(t, err)
assert.Check(t, is.Equal(db1ID, inspect.ID))
}

// Regression test for https://github.com/moby/moby/issues/47186
func TestRenameContainerTwice(t *testing.T) {
ctx := setupTest(t)
apiClient := testEnv.APIClient()

ctrName := "c0"
container.Run(ctx, t, apiClient, container.WithName("c0"))
defer func() {
container.Remove(ctx, t, apiClient, ctrName, containertypes.RemoveOptions{
Force: true,
})
}()

err := apiClient.ContainerRename(ctx, "c0", "c1")
assert.NilError(t, err)
ctrName = "c1"

err = apiClient.ContainerRename(ctx, "c1", "c2")
assert.NilError(t, err)
ctrName = "c2"
}
4 changes: 2 additions & 2 deletions libnetwork/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ func (ep *Endpoint) addServiceInfoToCluster(sb *Sandbox) error {
// In case the deleteServiceInfoToCluster arrives first, this one is happening after the endpoint is
// removed from the list, in this situation the delete will bail out not finding any data to cleanup
// and the add will bail out not finding the endpoint on the sandbox.
if err := sb.getEndpoint(ep.ID()); err == nil {
if err := sb.GetEndpoint(ep.ID()); err == nil {
log.G(context.TODO()).Warnf("addServiceInfoToCluster suppressing service resolution ep is not anymore in the sandbox %s", ep.ID())
return nil
}
Expand Down Expand Up @@ -692,7 +692,7 @@ func (ep *Endpoint) deleteServiceInfoFromCluster(sb *Sandbox, fullRemove bool, m
// get caught in disableServceInNetworkDB, but we check here to make the
// nature of the condition more clear.
// See comment in addServiceInfoToCluster()
if err := sb.getEndpoint(ep.ID()); err == nil {
if err := sb.GetEndpoint(ep.ID()); err == nil {
log.G(context.TODO()).Warnf("deleteServiceInfoFromCluster suppressing service resolution ep is not anymore in the sandbox %s", ep.ID())
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion libnetwork/endpoint_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (ep *Endpoint) Info() EndpointInfo {
return ep
}

return sb.getEndpoint(ep.ID())
return sb.GetEndpoint(ep.ID())
}

// Iface returns information about the interface which was assigned to
Expand Down
4 changes: 2 additions & 2 deletions libnetwork/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -1276,8 +1276,8 @@ func (n *Network) EndpointByName(name string) (*Endpoint, error) {
return e, nil
}

// EndpointByID returns the Endpoint which has the passed id. If not found,
// the error ErrNoSuchEndpoint is returned.
// EndpointByID should *never* be called as it's going to create a 2nd instance of an Endpoint. The first one lives in
// the Sandbox the endpoint is attached to. Instead, the endpoint should be retrieved by calling [Sandbox.Endpoints()].
func (n *Network) EndpointByID(id string) (*Endpoint, error) {
if id == "" {
return nil, ErrInvalidID(id)
Expand Down
4 changes: 2 additions & 2 deletions libnetwork/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ func (sb *Sandbox) removeEndpointRaw(ep *Endpoint) {
}
}

func (sb *Sandbox) getEndpoint(id string) *Endpoint {
func (sb *Sandbox) GetEndpoint(id string) *Endpoint {
sb.mu.Lock()
defer sb.mu.Unlock()

Expand Down Expand Up @@ -587,7 +587,7 @@ func (sb *Sandbox) DisableService() (err error) {
}

func (sb *Sandbox) clearNetworkResources(origEp *Endpoint) error {
ep := sb.getEndpoint(origEp.id)
ep := sb.GetEndpoint(origEp.id)
if ep == nil {
return fmt.Errorf("could not find the sandbox endpoint data for endpoint %s",
origEp.id)
Expand Down
2 changes: 1 addition & 1 deletion libnetwork/service_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (n *Network) findLBEndpointSandbox() (*Endpoint, *Sandbox, error) {
if !ok {
return nil, nil, fmt.Errorf("Unable to get sandbox for %s(%s) in for %s", ep.Name(), ep.ID(), n.ID())
}
sep := sb.getEndpoint(ep.ID())
sep := sb.GetEndpoint(ep.ID())
if sep == nil {
return nil, nil, fmt.Errorf("Load balancing endpoint %s(%s) removed from %s", ep.Name(), ep.ID(), n.ID())
}
Expand Down

0 comments on commit 71fa3ab

Please sign in to comment.