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

fix /etc/hosts and resolv.conf setup with network configs #5409

Merged
merged 4 commits into from
Mar 18, 2024

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Mar 18, 2024

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:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fixes problems with incorrectly configured hosts and resolv.conf files for containers.

Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

Luap99 added 4 commits March 18, 2024 14:37
Includes new pasta functionality.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
I got confused a bit there so make it clearer to readers that both are
different.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
We should never configure the files before we setup the network as we
are missing a lot of information. This is part 1 of the refactor where
we split the functions between create/add entries part. See the
following commit to actualy see how me make use of this.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Previously buildah may have created a incorrect hosts/resolv.conf file,
when netavark, slirp4netns or pasta are used we have to actually
consider their special setup and use the correct nameservers.

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

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

LGTM , needs rebase though.

@Luap99
Copy link
Member Author

Luap99 commented Mar 18, 2024

LGTM , needs rebase though.

No it doesn't, the only thing merged sine my changes is #5410 which doesn't cause any conflicts.

@flouthoc
Copy link
Collaborator

flouthoc commented Mar 18, 2024

LGTM , needs rebase though.

No it doesn't, the only thing merged sine my changes is #5410 which doesn't cause any conflicts.

@Luap99 Ah its the github UI then, i think it is just asking for a rebase with main branch since your branch is not in sync with main. I have no clue why does it asks for a rebase when it has no conflicts 😄

Anyways PR LGTM

Copy link
Collaborator

@flouthoc flouthoc 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

openshift-ci bot commented Mar 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@Luap99
Copy link
Member Author

Luap99 commented Mar 18, 2024

@mheon @baude PTAL

@mheon
Copy link
Member

mheon commented Mar 18, 2024

LGTM

@rhatdan
Copy link
Member

rhatdan commented Mar 18, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Mar 18, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit e514115 into containers:main Mar 18, 2024
36 checks passed
@Luap99 Luap99 deleted the pasta-result branch March 19, 2024 09:13
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Jun 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants