Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lsm5 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Ephemeral COPR build failed. @containers/packit-build please check. |
1 similar comment
|
Ephemeral COPR build failed. @containers/packit-build please check. |
lsm5
force-pushed
the
podman6-no-cni
branch
4 times, most recently
from
32e105a to
79be198
Compare
|
A friendly reminder that this PR had no activity for 30 days. |
lsm5
force-pushed
the
podman6-no-cni
branch
3 times, most recently
from
3e0adc8 to
dcab45a
Compare
nalind
left a comment
nalind
left a comment
There was a problem hiding this comment.
Please avoid breaking the public API when possible.
|
@nalind good for another look. The testing-farm job failures are a Fedora infra issue, already notified them. |
|
This will stay in draft until containers/container-libs#412 is merged. |
Luap99
left a comment
Luap99
left a comment
There was a problem hiding this comment.
LGTM overall minus the one extra file
lsm5
force-pushed
the
podman6-no-cni
branch
2 times, most recently
from
0ba7a4b to
e00419f
Compare
Luap99
left a comment
Luap99
left a comment
There was a problem hiding this comment.
LGTM
One more thing I found in the CI test scrip we have an rm -rf on the cni dir which can be dropped as well
buildah/contrib/cirrus/test.sh
Lines 11 to 14 in 09917c4
Anyhow non blocking, tests look good here and this can be fixed easily later so I think it is better to merge now to continue the dance into podman
dosubot
bot
added
the
size:L
|
@Luap99 Thanks. Will fix the test script in a followup. @containers/buildah-maintainers draft status removed. PTAL. |
|
@nalind PTAL, updating your review status is required for merging. |
FYI in the github UI allows us to dissmis it, you need to click on "1 requested chnage" to drop down then you see nalind and on the right side there is a pop up menu when you click on the tree dots. |
I didn’t know that, thanks! |
nalind
left a comment
nalind
left a comment
There was a problem hiding this comment.
LGTM once the merge conflict is resolved.
Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
Buildah no longer uses CNI for networking, so remove CNI spec version and libcni version from the version command output, build-time linker flags, and RPM spec. Also drop the FreeBSD CNI build tag from the Makefile since netavark is now the only supported network backend. Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
Mark CNIPluginPath and CNIConfigDir fields as deprecated in Builder, BuilderInfo, BuilderOptions, RunOptions, and BuildOptions structs. The fields are retained to avoid breaking the public API but are no longer used and are expected to be empty. Remove --cni-config-dir and --cni-plugin-path CLI flags. Simplify getNetworkInterface() to no longer accept CNI-specific parameters since netavark is now the only supported network backend. Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
Remove the docs/cni-examples/ directory, CNI installation section from install.md, and CNI fields from version man page. Remove the "from cni config test" from tests/from.bats and clean up --cni-config-dir usage from tests/namespaces.bats. Update comments that referenced CNI to use generic network terminology. Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
Remove the github.com/containernetworking/cni module from go.mod, go.sum, and vendor since it is no longer used. Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
4 participants
What type of PR is this?
What this PR does / why we need it:
Removes CNI for podman6
How to verify it
check if any CNI stuff still exists
Which issue(s) this PR fixes:
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?