feat: Filter workflows by "Finished before" and "Created Since" via API. Fixes #13151

[Adrien-D]

Fixes #13151

Motivation

• "Finished before" and "Created Since" are filtering workflows client-side so not all workflows are shown

Modifications

• I implemented createdAfter and finishedBefore on server side similarly to UI: 3.5 missing name + name prefix filters for archived workflows #12161.
• Remark on implementation: createdAfter is based on metadata.creationTimestamp which is stored as json so I needed to extract it is the request here
• I didn't update the CLI as mentioned here by @agilgur5 as I never worked with the CLI and don't really know what to do. Can someone give me an insight on that or maybe we could split this in another issue to implement in the CLI later ?

Verification

• Tests for ListWokflows function
• Local testing on existing worflows

Initial workflows
Filtering created since
Filtering finished before
Filtering finished before

[agilgur5]

• I didn't update the CLI as mentioned here by @agilgur5 as I never worked with the CLI and don't really know what to do. Can someone give me an insight on that [...]?

The CLI is one of the most straightforward parts of the codebase (simpler than the UI and API, IMO, so should be easier than the parts you've already went through!); each command has a file (e.g. cmd/argo/commands/list.go) with around ~100-200 LoC each (depending on the number of flags).

You can see the existing client-side date filters in the list command in this block:

argo-workflows/cmd/argo/commands/list.go

Lines 184 to 209 in 97b94f0

	if flags.createdSince != "" && flags.finishedBefore != "" {
	startTime, err := argotime.ParseSince(flags.createdSince)
	if err != nil {
	return nil, err
	}
	endTime, err := argotime.ParseSince(flags.finishedBefore)
	if err != nil {
	return nil, err
	}
	workflows = workflows.Filter(wfv1.WorkflowRanBetween(*startTime, *endTime))
	} else {
	if flags.createdSince != "" {
	t, err := argotime.ParseSince(flags.createdSince)
	if err != nil {
	return nil, err
	}
	workflows = workflows.Filter(wfv1.WorkflowCreatedAfter(*t))
	}
	if flags.finishedBefore != "" {
	t, err := argotime.ParseSince(flags.finishedBefore)
	if err != nil {
	return nil, err
	}
	workflows = workflows.Filter(wfv1.WorkflowFinishedBefore(*t))
	}
	}

Similar to the UI changes, those should be changed to be passed to the API instead of running client-side. The API call is made a few lines above in this block:

argo-workflows/cmd/argo/commands/list.go

Lines 166 to 170 in 97b94f0

	wfList, err := serviceClient.ListWorkflows(ctx, &workflowpkg.WorkflowListRequest{
	Namespace: flags.namespace,
	ListOptions: listOpts,
	Fields: flags.displayFields(),
	})

[agilgur5]

this should work on archived workflows too -- per #13151 (comment), no longer having a server-side date filter available in the archived UI is the regression at the core of the issue (that stems from 3.5's archived/live UI unification / #11121).

that half is actually a fix to a 3.5 regression, not a feature (as the feature was unintentionally partially removed in 3.5. the other half that adds it for live workflows via the SQLiteDB is a new feature; though as filtering on both is now required, that can be seen as a prereq for the fix)

Note that there is existing SQL code for minStartedAt and maxStartedAt, which were used before #11840, that could be re-used (renaming in the API might be breaking though if they were exposed before?)

[Adrien-D]

Indeed I missed that part for archived workflows, I'll change that and add tests.

For minStartedAt and maxStartedAt I didn't used those because I wasn't sure if they were still used somehow, and because they are not based on the same attribute so the query will be different. I can rename in the API if you tell me it's safe.

[Adrien-D]

I tried to update the code like this, but it seems that createdAfter and finishedBefore have no effect, not sure they are passed down to the API call. I spent few time trying to understand what's going on, do you have any idea ? I must miss something

var workflows wfv1.Workflows
createdAfter := ""
finishedBefore := ""
if flags.createdSince != "" {
	createdTime, err := argotime.ParseSince(flags.createdSince)
	if err != nil {
		return nil, err
	}
	createdAfter = createdTime.Format(time.RFC3339)
}
if flags.finishedBefore != "" {
	finishedTime, err := argotime.ParseSince(flags.createdSince)
	if err != nil {
		return nil, err
	}
	finishedBefore = finishedTime.Format(time.RFC3339)
}

for {
	log.WithField("listOpts", listOpts).Debug()
	wfList, err := serviceClient.ListWorkflows(ctx, &workflowpkg.WorkflowListRequest{
		Namespace:      flags.namespace,
		ListOptions:    listOpts,
		Fields:         flags.displayFields(),
		CreatedAfter:   createdAfter,
		FinishedBefore: finishedBefore,
	})
	if err != nil {
		return nil, err
	}
	workflows = append(workflows, wfList.Items...)
	if wfList.Continue == "" {
		break
	}
	listOpts.Continue = wfList.Continue
}

[Adrien-D]

@tooptoop4 what would be the best ? I'm a bit stuck with the CLI..!

Can we split this in another issue for later so we can merge this work already ?

[tooptoop4]

🚢 📦

[agilgur5]

Verification

you'll want to test when both are set simultaneously as well since there are some edge cases with that, although now that these are in the API, that can be covered with back-end tests (i.e. instead of manual testing)

and ensure that with and without archived workflows works correctly

[tooptoop4]

@agilgur5 join the band again 🎸

[agilgur5]

My original rationale and the original problems therein have not changed, so that won't happen, but if I'm tagged somewhere (as I was here) and it's related to something I wrote, I do try to be helpful as a general principle.
I didn't do an in-depth review, but as I was looking through anyway, I reviewed a small bit