Add --hosts-file flag; Add nohosts to remote build APIs

[gavinkflam]

I added a container level base-hosts-file configuration in #21013. This PR allows user to apply this configuration using a CLI flag. cc @Luap99

This is intended to be a container / pod level override of base_hosts_file in containers.conf. https://github.com/containers/common/blob/main/docs/containers.conf.5.md#containers-table

Closes #13277

Does this PR introduce a user-facing change?

Add --hosts-file flag to container create, container run, and pod create
Add nohosts option to /build and /libpod/build

[gavinkflam]

Hi @rhatdan, thank you for reviewing the changes.

My idea is to provide a container and pod level override in base_hosts_file of containers.conf, which is currently defaulted to the host's /etc/hosts. I agree --no-hosts seems redundant with the introduction of this new flag. I can hide it if you think that is appropriate.

Flag	Container hosts file
--base-hosts-file=""	Entries in base_hosts_file of containers.conf + Podman managed entries
--base-hosts-file="/path/to/hosts"	Entries in /path/to/hosts + Podman managed entries
--base-hosts-file="image"	Entries in the image's /etc/hosts + Podman managed entries
--base-hosts-file="none"	Use only Podman managed entries
--no-hosts	Use the image /etc/hosts without adding any entries

The current --no-hosts flag is similar to --base-hosts-file="image" with a subtle difference that --no-hosts doesn't add Podman managed entries. They include container name, add_hosts in containers.conf, network host entries, pasta IP, etc.

podman/libpod/container_internal_common.go

Lines 2282 to 2288 in 7dc7cbf

	return etchosts.New(&etchosts.Params{
	BaseFile: baseHostFile,
	ExtraHosts: c.config.HostAdd,
	ContainerIPs: containerIPsEntries,
	HostContainersInternalIP: etchosts.GetHostContainersInternalIP(c.runtime.config, c.state.NetworkStatus, c.runtime.network),
	TargetFile: targetFile,
	})

Do you think this fits into the wider Podman design and roadmap? I am open to changing anything or even starting from scratch.

[rhatdan]

I am fine with this, but I do not think base- adds anything to my understanding. If we can find a way to eliminate --no-host at all and do

--hosts-file="null" Use only Podman managed entries
--hosts-file="none" Use the image /etc/hosts without adding any entries (--no-hosts)

Bottom line for me is discoverability for these. Having them all named and documented in the man page together would help users figure out which is best for them.

Having a random --no-hosts someone else in the man page, does not help with this.

Better names/descriptions are always welcome.

[Luap99]

--no-hosts is a well known docker option so we should not remove it from the docs and this option does not replace this option at all.

We should make the connection between the two options clear int he man page, i.e. --no-hosts should link to --hosts-file and the other way around.

[gavinkflam]

@rhatdan @Luap99 Sounds like we have two directions here.

1. Bring together --no-hosts and this new flag, and introduce a new mode to use only Podman managed entries.
2. Keep it as an override of base_hosts_file in containers.conf.

I'm leaning towards (2) since we have a configuration item in containers.conf already. What do you think?

[Luap99]

I say we should do 2

[rhatdan]

Since docker supports the --no-host, then go with 2

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[rhatdan]

@gavinkflam Still working on this?

[gavinkflam]

Yes. I'll find some time to wrap up the changes. Thanks.

[gavinkflam]

@rhatdan @Luap99 This is now complete. Apologies for the delay - it's been an eventful year, and I lost track of this open PR.

[Luap99]

Thanks mostly looks good but I think we can do a bit better on the tests

[gavinkflam]

Thanks mostly looks good but I think we can do a bit better on the tests

Thank you for the suggestions, @Luap99! I’ve enhanced the tests and implemented the flag for pod create. The pod create command now uses this flag to configure the infra-container appropriately. Additionally, I’ve renamed the container BaseHostsFile to HostsFile for better naming consistency.

During testing, I discovered that image builds using the --no-hosts flag were failing in remote tests. I realized that this flag was never supported in the remote build APIs. To address this, I’ve added a nohosts option to the build APIs.

[gavinkflam]

Reverted rename and rebased.

[Luap99]

LGTM
Thank you

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gavinkflam, 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:

• OWNERS [Luap99,rhatdan]

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

[gavinkflam]

/packit retest-failed

[rhatdan]

/lgtm