argo-workflows
argo-workflows copied to clipboard
there is no name or name prefix filter in workflows ui
Pre-requisites
- [X] I have double-checked my configuration
- [X] I can confirm the issues exists when I tested with
:latest
- [ ] I'd like to contribute the fix myself (see contributing guide)
What happened/what you expected to happen?
v3.4.11 had 'name' and 'name prefix' filters in the 'archived workflows' ui, this is extremely useful to filter on workflows starting with a name or an exact name. particularly if its a sensor generated workflow or you just want to jump to a known workflow name without needing to construct a url in the browser address bar (not user friendly)
v3.5.1 has no such option in the UI!
Version
3.5.1
Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.
n/a
Logs from the workflow controller
kubectl logs -n argo deploy/workflow-controller | grep ${workflow}
Logs from in your workflow's wait container
kubectl logs -n argo -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded
We also need a filter for archive items. We didn't need it because It was a separated view.
I can work on this one. Just to make things clear, here's what I can implement :
- Filtering by workflow name, using
Contains
,Exact
andPrefix
. I'll implement as a dropdown to make it lighter on the UI and because it doesn't make sens to have those three at the same time - Checkbox filtering by state
Active
andArchived
Visual preview proposal :
Edit: I added the Contains
filtering and the dropdown select options
doable now
This is now doable with the SQLite DB from #13021 / #12736. It previously was not possible for non-archived Workflows as k8s/etcd has no such native filter per https://github.com/argoproj/argo-workflows/issues/12025#issuecomment-1914065370, part 3.
implementation details
archived vs. non-archived filter
naming
@Adrien-D "State" is a bit ambiguous naming; that part we did actually discuss this in https://github.com/argoproj/argo-workflows/issues/12025#issuecomment-1865458377 et al, where I suggested "Workflow Storage" and "In-cluster" + "Archived" as the options. "Active" is a bit ambiguous as well, as was the original terminology of "Live" workflows. "Live" has less connotations around things like the phase of a Workflow though. To be fair, you can have an Archive DB in your cluster too, so "In-cluster" is not necessarily fully precise either, but "etcd" is an implementation detail of k8s and so could be both confusing and imprecise on some distros as well. "k8s resources" sounds a bit confusing user-side as well, but is the most precise 😕
Another option is "Unarchived" + "Archived". That might actually be the least confusing / least ambiguous option 🤔
other impl details
Ideally also the checkboxes shouldn't display if you don't have the Workflow Archive enabled. This is easier said than done as the UI does not currently know this information; consider that a stretch goal and possibly separate follow-on PR as such.
Also, this requires adding a new option to the API method, it is not a UI-only change, as I learned in https://github.com/argoproj/argo-workflows/issues/12025#issuecomment-1914065370 part 1 and others in #12237. The API implementation should roughly follow https://github.com/argoproj/argo-workflows/pull/12237#discussion_r1433459929
name filter
That one looks good to me, I like it. Seems more efficient and less confusing than the original in #6801 as well. Although users will have to learn that the name of the field is clickable to access the dropdown.
@agilgur5 I see the confusion with "State" and "Active", I'm not very familiar with all of this wording (and this repo yet) so thanks for giving me some details, helps a lot ! "Workflow Storage" "Unarchived" + "Archived" is good for me, it's indeed the less confusing
For the implementation details I'm trying to understand what you're saying. I did some work here on name filter (Didn't start the archived filtering yet) so maybe it can be a good start to discuss my understanding of the implementation
I'm already facing some issues that I need to understand for consistency :
- Why for workflow templates do we have
NamePattern
as it's own a query parameter instead of a listOptions ? Should I implement it as it's own query parameters as well ? See here - Why for
ListWorkflows
,CountWorkflows
are we forwarding namePrefix as it looks like it's none even used [1] [2] - Why for
CanI
are we forwarding "Name" as we don't seem to use it ?
For the UI I inspired from grafana's interface as they have a good UX on filter in my opinion, hopefully the down arrow will help to understand that the field is clickable
Why for ListWorkflows, CountWorkflows are we forwarding namePrefix as it looks like it's none even used [1] [2]
@Adrien-D Thank you for working on this feature. I think I can provide more details about this since I was working on https://github.com/argoproj/argo-workflows/pull/13021 / https://github.com/argoproj/argo-workflows/pull/12736.
Before Unified workflows list UI and API was introduced, we supported listing archived workflow using a name prefix. However, due to some limitation of kubernetes(kubernetes doesn't allow us to list live workflows by using a name prefix), after unifying the APIs, we lost the ability to list by using nameprefix(ref: setting the name prefix to ""
for list archived workflows in the unifying PR)
With https://github.com/argoproj/argo-workflows/pull/13021 / https://github.com/argoproj/argo-workflows/pull/12736., we use a sqlite-based in memory store for live workflows, and this provide us a more unified query availability, and we can use SQL queries for both archived and live workflows, thus adding name prefix support becomes possible. and I already unified query experience between live workflow and archived workflow by using name prefix: https://github.com/argoproj/argo-workflows/pull/13021/files#diff-8a497f2f2e0827919658f6a6b4f18e3113cfeb24744b8ac1fd05ccdbf2b1c3b4R89-R157, and that means it is already supported to query both live workflows and archived workflows with a name prefix from database level.
The reason that we pass them as empty is because API doesn't support that. I think in your change, you might need to add namePrefix
to the WorkflowListRequest
:https://github.com/argoproj/argo-workflows/blob/bda02806cdc27216cd0ce33418997bf5a4215ae1/pkg/apiclient/workflow/workflow.proto#L31-L36 so that API can take the namePrefix and pass to [1] [2]. I didn't do it in https://github.com/argoproj/argo-workflows/pull/13021 because that PR was already too large and I wanted to make it easier for folks to review, also, adding name prefix in both API and UI worths a dedicated PR.
- Why for workflow templates do we have
NamePattern
as it's own a query parameter instead of a listOptions ? Should I implement it as it's own query parameters as well ? See here
I believe that's because it's using k8s's metav1.ListOptions
as the type, not the module's ListOptions
type, no?
- Why for
CanI
are we forwarding "Name" as we don't seem to use it ?
That actually might be a mistake; it used to be used prior to #2670 as this line was removed in the refactor. (git blame
is very useful if you spot something off like this)
If I'm understanding correctly, the name
would only make a difference when resourceNames
is used, so it may have just happened to work fine in most cases. But I'm not 100% sure if that's exactly how it works, I don't think I've ever tried with a specific name
.
Also note that all of this far predates me being a contributor so I am guesstimating a rationale and would not know concretely. Unfortunately that happens a lot with older codebases, context gets lost and newer contributors have to figure it out (so do older contributors, you will forget if you, for instance, didn't write it down in a PR description or somewhere) 😕
For the UI I inspired from grafana's interface as they have a good UX on filter in my opinion, hopefully the down arrow will help to understand that the field is clickable
Related: #11421
@jiachengxu @agilgur5 Thanks for your answers. I implemented your feedback on my PR https://github.com/argoproj/argo-workflows/pull/13160
I'll make the filtering archived/non-archived WF on a separate PR to make the review easier.
I'll make the filtering archived/non-archived WF on a separate PR to make the review easier.
Let's split that out into a separate issue as well and link back to here for reference. This issue does not include it in its title and we have #13151 for native date filtering already as well.
Archive filtering will be done in this issue https://github.com/argoproj/argo-workflows/issues/13171