Skip to content

libnetwork: remove Network.EndpointByID as it must not be used#49341

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:libnetwork_remove_EndpointByID
Jan 27, 2025
Merged

libnetwork: remove Network.EndpointByID as it must not be used#49341
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:libnetwork_remove_EndpointByID

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

This is a quick-fix; instead of loading endpoints through Network.EndpointByID(), Sandbox.getEndpoint() was made public and is now used. I also added a comment on Network.EndpointByID() saying this method should never be used -- and actually it should be removed in a follow-up.


libnetwork: remove Network.EndpointByID as it must not be used

commit 80c44b4 removed uses of this method and added a comment that it should never be used;

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()].

Given that the only use of this method is in tests, we can remove if altogether.

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

commit 80c44b4 removed uses of this
method and added a comment that it should never be used;

> 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()].

Given that the only use of this method is in tests, we can remove if altogether.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah added status/2-code-review area/networking Networking kind/refactor PR's that refactor, or clean-up code labels Jan 27, 2025
@thaJeztah thaJeztah added this to the 28.0.0 milestone Jan 27, 2025
@thaJeztah thaJeztah self-assigned this Jan 27, 2025
Copy link
Copy Markdown
Member

@akerouanton akerouanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@thaJeztah
Copy link
Copy Markdown
Member Author

😄 Stumbled on this one when cleaning up errors, when I noticed "should never be used!" and .. then found it was indeed never used, so let's remove it! 🧹

Copy link
Copy Markdown
Contributor

@robmry robmry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Unrelated Windows failures ...

=== FAIL: github.com/docker/docker/integration/container TestRenameAnonymousContainer (10.96s)
    rename_test.go:151: assertion failed: 0 (int) != 1 (inspect.State.ExitCode int): container 5768112420fb67457165af681cc2e785c043069535c6bf8dd37636b02b8b8b4a exited with the wrong exitcode: 
=== FAIL: github.com/docker/docker/integration/container TestAttach/with_TTY (60.32s)
    attach_test.go:54: assertion failed: error is not nil: error during connect: Post "http://%2F%2F.%2Fpipe%2Fdocker_engine/v1.48/containers/create": EOF

@thaJeztah thaJeztah merged commit 668cb5a into moby:master Jan 27, 2025
@thaJeztah thaJeztah deleted the libnetwork_remove_EndpointByID branch January 27, 2025 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/networking Networking kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants