UI: 3.5 missing name + name prefix filters for archived workflows

[tooptoop4]

Pre-requisites

• I have double-checked my configuration
• 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

[maxisam]

We also need a filter for archive items. We didn't need it because It was a separated view.

[Adrien-D]

I can work on this one. Just to make things clear, here's what I can implement :

• Filtering by workflow name, using Contains, Exact and Prefix. 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 and Archived

Visual preview proposal :

Edit: I added the Contains filtering and the dropdown select options

[agilgur5]

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 #12025 (comment), 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 #12025 (comment) 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 #12025 (comment) part 1 and others in #12237. The API implementation should roughly follow #12237 (comment)

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.

[Adrien-D]

@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

[jiachengxu]

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 #13021 / #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 #13021 / #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:

argo-workflows/pkg/apiclient/workflow/workflow.proto

Lines 31 to 36 in bda0280

	message WorkflowListRequest {
	string namespace = 1;
	k8s.io.apimachinery.pkg.apis.meta.v1.ListOptions listOptions = 2;
	// Fields to be included or excluded in the response. e.g. "items.spec,items.status.phase", "-items.status.nodes"
	string fields = 3;
	}

so that API can take the namePrefix and pass to [1] [2]. I didn't do it in #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.

[agilgur5]

• 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

[Adrien-D]

@jiachengxu @agilgur5 Thanks for your answers. I implemented your feedback on my PR #13160

I'll make the filtering archived/non-archived WF on a separate PR to make the review easier.

[agilgur5]

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.

[Adrien-D]

Archive filtering will be done in this issue #13171