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

Network prune filters for http api (compat and libpod) #9710

Conversation

jmguzik
Copy link
Contributor

@jmguzik jmguzik commented Mar 14, 2021

/kind feature
Prune filters are missing not implemented in HTTP API. This PR tackles the topic in compat and libpod API.

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. labels Mar 14, 2021
@jmguzik jmguzik changed the title Network prune filters for http api (compat and libpod) [WIP] Network prune filters for http api (compat and libpod) Mar 14, 2021
@jmguzik jmguzik force-pushed the network-prune-filters-http-api branch 2 times, most recently from 0377f73 to b76e051 Compare March 15, 2021 08:09
@jmguzik
Copy link
Contributor Author

jmguzik commented Mar 15, 2021

It seems there is a timezone issue (maybe due to a time change in the USA):

           Expected
               <string>: '04 EDT'
           to contain substring
               <string>: EST
         

Luap99 pushed a commit to Luap99/libpod that referenced this pull request Mar 15, 2021
The `libpod/network` package should only be used on the backend and not the
client. The client used this package only for two functions so move them
into a new `pkg/network` package.

This is needed so we can put linux only code into `libpod/network`, see containers#9710.

[NO TESTS NEEDED]

Signed-off-by: Paul Holzinger <[email protected]>
Luap99 pushed a commit to Luap99/libpod that referenced this pull request Mar 15, 2021
Some packages used by the remote client imported the libpod package.
This is not wanted because it adds unnecessary bloat to the client and
also causes problems with platform specific code(linux only), see containers#9710.

The solution is to move the used functions/variables into extra packages
which do not import libpod.

This change shrinks the remote client size more than 6MB compared to the
current master.

[NO TESTS NEEDED]
I have no idea how to test this properly but with containers#9710 the cross
compile should fail.

Signed-off-by: Paul Holzinger <[email protected]>
@jmguzik jmguzik force-pushed the network-prune-filters-http-api branch 3 times, most recently from 0828547 to b3d03d0 Compare March 16, 2021 08:40
@jmguzik jmguzik changed the title [WIP] Network prune filters for http api (compat and libpod) Network prune filters for http api (compat and libpod) Mar 16, 2021
@jmguzik jmguzik marked this pull request as ready for review March 16, 2021 10:46
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 16, 2021
@jmguzik jmguzik requested a review from Luap99 March 16, 2021 10:46
@jmguzik
Copy link
Contributor Author

jmguzik commented Mar 16, 2021

sys remote ubuntu-2004 root host fails because of some podman build issues. I think it is unrelated. I would be grateful if somebody with rights could re-run.

Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

Make sure to update the api docs in pkg/api/server/register_networks.go.
Also squash your commits please.

libpod/network/netconflist.go Outdated Show resolved Hide resolved
libpod/network/netconflist.go Show resolved Hide resolved
@jmguzik jmguzik force-pushed the network-prune-filters-http-api branch from b3d03d0 to 8ea02d0 Compare March 17, 2021 23:02
@jmguzik
Copy link
Contributor Author

jmguzik commented Mar 17, 2021

Make sure to update the api docs in pkg/api/server/register_networks.go.
Also squash your commits please.

Commits squashed and API docs updated.

@jmguzik jmguzik requested a review from Luap99 March 18, 2021 07:33
Copy link
Member

@Luap99 Luap99 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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2021
@Luap99
Copy link
Member

Luap99 commented Mar 18, 2021

@containers/podman-maintainers PTAL

@mheon
Copy link
Member

mheon commented Mar 18, 2021

/lgtm
/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jmguzik, Luap99, mheon

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

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2021
@openshift-merge-robot openshift-merge-robot merged commit 629183b into containers:master Mar 18, 2021
@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 Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 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. kind/feature Categorizes issue or PR as related to a new feature. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants