Skip to content

daemon: only add short cid to aliases for custom networks#47181

Merged
thaJeztah merged 1 commit intomoby:masterfrom
akerouanton:fix-aliases-on-default-bridge
Jan 23, 2024
Merged

daemon: only add short cid to aliases for custom networks#47181
thaJeztah merged 1 commit intomoby:masterfrom
akerouanton:fix-aliases-on-default-bridge

Conversation

@akerouanton
Copy link
Member

Related to:

- What I did

Prior to 7a9b680, the container short ID was added to the network aliases only for custom networks. However, this logic wasn't preserved in 6a2542d and now the cid is always added to the list of network aliases.

This commit reintroduces the old logic.

- How to verify it

$ docker run --rm -d --name c0 alpine top
$ docker inspect --format="{{ .NetworkSettings.Networks.bridge.Aliases }}" c0
[]

- Description for the changelog

  • Fix an issue causing containers to have their short ID added to their network alias when inspecting them.

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

@akerouanton akerouanton added this to the 25.0.1 milestone Jan 23, 2024
@akerouanton akerouanton self-assigned this Jan 23, 2024
@thaJeztah thaJeztah modified the milestones: 25.0.1, 26.0.0 Jan 23, 2024
@thaJeztah
Copy link
Member

I changed the milestone for this one to v26.0.0 (as this PR is for master); can you open a cherry-pick for the v25.0 branch (for v25.0.1) ?

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

can we have a test for this?

Copy link
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

@akerouanton akerouanton force-pushed the fix-aliases-on-default-bridge branch from a8001a4 to 9cc48be Compare January 23, 2024 10:49
skip.If(t, testEnv.DaemonInfo.OSType == "windows")

ctx := setupTest(t)
apiClient := request.NewAPIClient(t)
Copy link
Member

@thaJeztah thaJeztah Jan 23, 2024

Choose a reason for hiding this comment

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

double-check; does this test depend on API version, or is this for all versions? Because the code updated is in a branch for specific API versions;

case versions.LessThan(version, "1.45"):

Copy link
Member Author

Choose a reason for hiding this comment

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

Prior to v1.45, we did automatically add short cid to network-scoped aliases. That's not the case anymore, thus short cid should still not appear there so I didn't scope this test to a specific API version.

@akerouanton akerouanton force-pushed the fix-aliases-on-default-bridge branch from 9cc48be to 1f419cb Compare January 23, 2024 12:28
@thaJeztah

This comment was marked as resolved.

@akerouanton akerouanton force-pushed the fix-aliases-on-default-bridge branch from 1f419cb to 0587835 Compare January 23, 2024 15:45
Prior to 7a9b680, the container short ID was added to the network
aliases only for custom networks. However, this logic wasn't preserved
in 6a2542d and now the cid is always added to the list of network
aliases.

This commit reintroduces the old logic.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
@akerouanton akerouanton force-pushed the fix-aliases-on-default-bridge branch from 0587835 to 9f37672 Compare January 23, 2024 16:08
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants