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

Allow filtering containers by command #24791

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arsenalzp
Copy link

This PR is intended to implement feature requested in issue #24664

I decided to implement filtering of commands to all kind of containers: for external and for managed by podman, I guess, this approach is better than just implementing this feature for a single sort of container.

If you don't want to accept this approach then I will put it away and implement filtering for external container only.

Does this PR introduce a user-facing change?

None

@giuseppe
Copy link
Member

giuseppe commented Dec 9, 2024

the change LGTM, but you'll need to update the man pages too

test/e2e/ps_test.go Outdated Show resolved Hide resolved
test/e2e/ps_test.go Outdated Show resolved Hide resolved
@arsenalzp
Copy link
Author

arsenalzp commented Dec 9, 2024

the change LGTM, but you'll need to update the man pages too

Thank you!
All related man pages have been updated, however I have the following issue on the validation stage:

hack/xref-helpmsgs-manpages
xref-helpmsgs-manpages: 'podman container list --format ' lists '.Commands', which is not in docs/source/markdown/podman-ps.1.md
xref-helpmsgs-manpages: 'podman container ps --format ' lists '.Commands', which is not in docs/source/markdown/podman-ps.1.md
xref-helpmsgs-manpages: 'podman ps --format ' lists '.Commands', which is not in docs/source/markdown/podman-ps.1.md
make[1]: *** [Makefile:608: xref-helpmsgs-manpages] Error 1

Could you please me an advice how to fix it?

libpod/runtime_ctr.go Outdated Show resolved Hide resolved
libpod/runtime_ctr.go Outdated Show resolved Hide resolved
pkg/ps/ps.go Show resolved Hide resolved
docs/source/markdown/podman-ps.1.md Outdated Show resolved Hide resolved
Copy link
Contributor

openshift-ci bot commented Dec 18, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: arsenalzp
Once this PR has been reviewed and has the lgtm label, please assign vrothberg for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@arsenalzp arsenalzp force-pushed the issue_24664 branch 4 times, most recently from 52b9241 to e299759 Compare December 24, 2024 18:00
Signed-off-by: Oleksandr Krutko <[email protected]>

add a test, improve logic of command filter

Signed-off-by: Oleksandr Krutko <[email protected]>

improve a test

Signed-off-by: Oleksandr Krutko <[email protected]>

improve test, update a man page

Signed-off-by: Oleksandr Krutko <[email protected]>

improve man page, runtime functions

Signed-off-by: Oleksandr Krutko <[email protected]>

move ExternalContainerFilter type to entities package

Signed-off-by: Oleksandr Krutko <[email protected]>

add external filters

Signed-off-by: Oleksandr Krutko <[email protected]>

add tests for external containers

Signed-off-by: Oleksandr Krutko <[email protected]>

add test for ps external id, ancestor

Signed-off-by: Oleksandr Krutko <[email protected]>

add tests for ps external filters of since, before

Signed-off-by: Oleksandr Krutko <[email protected]>

fix linter warnings, add completion for the name filter

Signed-off-by: Oleksandr Krutko <[email protected]>
@arsenalzp
Copy link
Author

All objections were removed, all applicable filters were applied for external containers with corresponding tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants