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

add hostname to network alias #19193

Merged
merged 2 commits into from
Jul 11, 2023

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Jul 11, 2023

We use the name as alias but using the hostname makes also sense and
this is what docker does. We have to keep the short id as well for
docker compat.

While adding some tests I removed some duplicated tests that were
executed twice for nv for no reason.

Fixes #17370

Does this PR introduce a user-facing change?

Add the hostname by default as network alias so containers can resolve other hostnames and not only their names.

Luap99 added 2 commits July 11, 2023 15:16
Since we have sqlite there is no point in duplicating this acroos two db
backends. Just set earlier when we validate the networks anyway.

Signed-off-by: Paul Holzinger <[email protected]>
We use the name as alias but using the hostname makes also sense and
this is what docker does. We have to keep the short id as well for
docker compat.

While adding some tests I removed some duplicated tests that were
executed twice for nv for no reason.

Fixes containers#17370

Signed-off-by: Paul Holzinger <[email protected]>
@openshift-ci openshift-ci bot added release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 11, 2023
@@ -1128,40 +1129,8 @@ EXPOSE 2004-2005/tcp`, ALPINE)
session = podmanTest.Podman([]string{"run", "--name", "con4", "--network", net, ALPINE, "nslookup", pod2 + ".dns.podman"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
})

It("podman run check dnsname plugin with Netavark", func() {
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch. I took a moment just now to copypaste each of those tests into tmpfiles, then diff them. They are 100% identical other than test name and the (useless) SkipIfCNI().

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 11, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhatdan
Copy link
Member

rhatdan commented Jul 11, 2023

/lgtm
/hold

@openshift-ci openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Jul 11, 2023
@rhatdan
Copy link
Member

rhatdan commented Jul 11, 2023

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 11, 2023
@openshift-merge-robot openshift-merge-robot merged commit 1be2ec1 into containers:main Jul 11, 2023
@Luap99 Luap99 deleted the hostname-alias branch July 11, 2023 16:20
@Luap99 Luap99 added the 4.6 label Jul 11, 2023
@benoitf
Copy link
Contributor

benoitf commented Jul 11, 2023

Thanks @Luap99

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Oct 10, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: DNS between two pods on the same network not working as expected
6 participants