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 /etc/hosts when running with host network #4869

Merged
merged 2 commits into from Jun 21, 2023

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Jun 15, 2023

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

Some tools depend on the hostname being present in /etc/hosts. I would
argue they are broken but its not like we can do anything about that.

This adds the hostname with the local host ip when the host network is
used. For private networking we already add the hostname.

We also now correctly force host networking in chroot mode, it was
silently ignored before thus causing extra confusion here.

How to verify it

Which issue(s) this PR fixes:

Fixes #4446

Special notes for your reviewer:

Does this PR introduce a user-facing change?

buildah now adds the hostname to /etc/hosts when running with host networking.

@TomSweeneyRedHat
Copy link
Member

A test blipped not being able to see google.com, I've restarted.

@Luap99
Copy link
Member Author

Luap99 commented Jun 16, 2023

Tests are failing because of the return error with chroot and --network change, looks like a lot of tests are testing functionality that never worked.
Given how many test fails and that commit d6b43c3 can be considered a breaking change even when it never did what the users asked for I think it is just best to drop that commit and document it better. Basically every container in container workload that uses BUILDAH_ISOLATION=chroot would start blowing up when they have --network set and it is not set to host.
@TomSweeneyRedHat @flouthoc @nalind @rhatdan WDYT?

@nalind
Copy link
Member

nalind commented Jun 16, 2023

host should always have been the default for chroot. LGTM.

@TomSweeneyRedHat
Copy link
Member

Backing out d6b43c3 sounds like a good plan to me @Luap99

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Some tools depend on the hostname being present in /etc/hosts. I would
argue they are broken but its not like we can do anything about that.

This adds the hostname with the local host ip when the host network is
used. For private networking we already add the hostname.

We also now correctly force host networking in chroot mode, it was
silently ignored before thus causing extra confusion here.

Fixes containers#4446

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented Jun 20, 2023

/approve
/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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 Jun 21, 2023

/hold cancel

@rhatdan rhatdan merged commit f4e8be4 into containers:main Jun 21, 2023
32 checks passed
@Luap99 Luap99 deleted the hosts-host branch June 21, 2023 12:37
@SaurabhAhuja1983
Copy link

@Luap99 Thank You for working on it.
I really appreciate it.
I see there are couple of things failing in main after this merge.
Once that's done, can you please release the new version of buildah, so that i can pick it up instead of using main.

@Luap99
Copy link
Member Author

Luap99 commented Jun 21, 2023

A new release will be cut in the next few weeks.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved kind/bug Categorizes issue or PR as related to a bug. lgtm locked - please file new issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Buildah intermediate container(Id) is not resolvable
5 participants