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

feat: Add ability to filter by resource labels and annotations #9572

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

avanish23
Copy link
Contributor

@avanish23 avanish23 commented Oct 23, 2024

Closes: #7547
Currently, the filtering is allowed by name only, for any resource.
With this PR intends to introduce support for filtering via various parameters

  • name
  • status
  • label
  • annotations
    and more.

How can this be done?
This will be done by comma ',' separated values. If the input does not have a comma-separated value, it will be filtered by name by default.
If a comma-separated string is provided then the convention followed should be property,value for example status,Running label,app=dashboard annotations,sidecar.istio.io/inject=true
For example, if the user input is:

  • dashboard the pods filtered will be those that will contain "dashboard" in their name.
  • name,dashboard,label,app=dashboard,status,Running the pods filtered will be have "dashboard" in their name, a label with key value pair "{app:dashboard}" and status is "Running"

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 23, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: avanish23
Once this PR has been reviewed and has the lgtm label, please assign maciaszczykm for approval. For more information see the Kubernetes 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

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 23, 2024
@avanish23 avanish23 changed the title Add ability to filter by pod labels and annotations feat: Add ability to filter by pod labels and annotations Oct 28, 2024
@avanish23 avanish23 changed the title feat: Add ability to filter by pod labels and annotations feat: Add ability to filter by resource labels and annotations Nov 1, 2024
@floreks
Copy link
Member

floreks commented Nov 7, 2024

The initial implementation was only meant to work with names. We wanted to take another pass at it. The main flaws of the current approach are:

  • Filtering different fields based on string parsing does not make a good API. We should probably use an encoded array of objects that can define fields like: operation, field/property, value, etc. This way we could create more complex filtering queries by multiple fields with i.e. AND/OR operations.
  • It does not use server-side filtering. The K8S API supports field-level filtering (name, status, labels). Annotations would probably be handled on our side.

I would prefer not to rely on current logic to not encourage anyone to extend it as it should be rewritten and optimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to filter by pod status and by labels
3 participants