-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Use logical AND on chained label! filters #20280
Conversation
Fixes the filter matching for chained negated labels (label!) on containers and volumes. When pruning with chained label! filters, the filters will be ANDed instead of ORed. Fixes containers#17413 Signed-off-by: Jake Correnti <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jakecorrenti 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 |
Probably need to update manpages to reflect this, it's not fully intuitive it would be different |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this how docker behaves? Seems like the linked bug links to a open docker bug so I assume our current version is compatible with docker with I think is more describable.
I am not sure this has an easy answer but a change like this can easily break users and as far as I can tell there is exactly one user asking for this so I would vote for keeping the current behaviour and not break.
@rhatdan what are your thoughts on this? |
I think we should match what Docker does. I would figure users want to see CONTENT which did not container Labels (ABC and XYZ) |
Is the best course of action here to close in favor of maintaining docker compatibility? |
What is the difference? What are you trying to achieve? |
I just wanted to confirm that is what the consensus was. Looking back at your previous comment I suppose I don't really follow when you say the user wants to see content which doesn't contain the labels |
Fixes the filter matching for chained negated labels (label!) on containers and volumes. When pruning with chained label! filters, the filters will be ANDed instead of ORed.
Fixes #17413
Does this PR introduce a user-facing change?