podman icon indicating copy to clipboard operation
podman copied to clipboard

Allow filtering containers by command

Open arsenalzp opened this issue 11 months ago • 15 comments

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

arsenalzp avatar Dec 07 '24 00:12 arsenalzp

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

giuseppe avatar Dec 09 '24 10:12 giuseppe

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 <TAB>' lists '.Commands', which is not in docs/source/markdown/podman-ps.1.md xref-helpmsgs-manpages: 'podman container ps --format <TAB>' lists '.Commands', which is not in docs/source/markdown/podman-ps.1.md xref-helpmsgs-manpages: 'podman ps --format <TAB>' 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?

arsenalzp avatar Dec 09 '24 23:12 arsenalzp

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

arsenalzp avatar Dec 24 '24 19:12 arsenalzp

Looks like you did not update man pages correctly.

rhatdan avatar Jan 02 '25 16:01 rhatdan

Looks like you did not update man pages correctly.

Hello, I'll work on it once back from vacation.

arsenalzp avatar Jan 08 '25 10:01 arsenalzp

This isn't really what I had in mind since it still duplicates all the filtering functions.

My idea was to define a interface for the filter function type func(container *libpod.Container) bool, i.e. instead of using the Container struct it just accept and interface the implements all the access methods And then we just have to do that for the external container and Container struct.

That way there is no need to duplicate the GenerateFilterFunc... funxtions

Hello,

I absolutely agree with you! However, that improvement might be implemented in separate PR due to its complexity as it affects not only main code but tests as well, I'm ready to support/work on it.

It is more wise in this PR accept changes which affect filtering feature only.

arsenalzp avatar Jan 19 '25 13:01 arsenalzp

Hello colleagues, Any decision regarding this PR?

arsenalzp avatar Feb 06 '25 15:02 arsenalzp

@arsenalzp I don't have time to tackle this right now, I am not strictly opposed to how you have done it so if others are ok with then it is fine for me to. We just need to make sure it doesn't break anything.

Luap99 avatar Feb 12 '25 14:02 Luap99

Build is complaining you need to update the manpages

mheon avatar Mar 04 '25 15:03 mheon

Build is complaining you need to update the manpages

Hello, Thank you for help. I tried, it seems it finished successfully.

$make --debug=b docs
...
 Must remake target 'docs/source/markdown/podmansh.1'.
 Successfully remade target file 'docs/source/markdown/podmansh.1'.
Must remake target 'docs'.
Successfully remade target file 'docs'.
$

However, there are many warnings:

troff:<standard input>:255: warning [p 4, 5.5i]: cannot adjust line
troff:<standard input>:272: warning [p 4, 8.3i]: cannot adjust line
troff:<standard input>:346: warning [p 5, 9.0i]: cannot adjust line
troff:<standard input>:359: warning [p 5, 11.2i]: cannot adjust line
troff:<standard input>:372: warning [p 6, 2.0i]: cannot adjust line
troff:<standard input>:389: warning [p 6, 4.7i]: cannot adjust line
troff:<standard input>:463: warning [p 7, 5.5i]: cannot adjust line
troff:<standard input>:474: warning [p 7, 7.5i]: cannot adjust line
troff:<standard input>:485: warning [p 7, 9.5i]: cannot adjust line
troff:<standard input>:496: warning [p 8, 0.2i]: cannot adjust line
troff:<standard input>:361: warning [p 4, 11.0i, div '3tbd44,1', 0.0i]: cannot adjust line

arsenalzp avatar Mar 08 '25 12:03 arsenalzp

Hello colleagues, Do you have any updates on this PR?

arsenalzp avatar Mar 20 '25 12:03 arsenalzp

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore.

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

Two checks failed due to version mismatch:

go: go.mod requires go >= 1.23.0 (running go 1.22.12; GOTOOLCHAIN=local) error: Bad exit status from /var/tmp/rpm-tmp.QlFQho (%build)

arsenalzp avatar Mar 31 '25 16:03 arsenalzp

Hello colleagues, Do you have any objections/comments?

arsenalzp avatar Apr 02 '25 08:04 arsenalzp

@arsenalzp Mind rebasing on top of latest main to fix CI?

mheon avatar Apr 02 '25 18:04 mheon

Seems OK in initial review, I'll do a final one after rebase

mheon avatar Apr 02 '25 18:04 mheon

@Luap99 PTAL when you get a chance

mheon avatar Apr 07 '25 18:04 mheon

[APPROVALNOTIFIER] This PR is APPROVED

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

openshift-ci[bot] avatar Apr 08 '25 12:04 openshift-ci[bot]