[Pre-release] Workflows are shown as not archived after workflow GC

[toyamagu-2021]

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?

AhivedWorkflows are marked Archive: false in UI after workflows GC.

1. Submit Workflow, then marked as Archived.
   
2. After GC, workflows marked as Pending
   
3. Workflow are marked as not archived in workflow list page
   

Should we change this logic?

argo-workflows/ui/src/models/workflows.ts

Lines 544 to 546 in 97b6fa8

	export function isArchivedWorkflow(wf: Workflow): boolean {
	return wf.metadata.labels && wf.metadata.labels[archivalStatus] === 'Archived';
	}

Version

master

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.

ANY Workflows

Logs from the workflow controller

N/A

Logs from in your workflow's wait container

N/A

[terrytangyuan]

This should probably be fixed in the backend. Those workflows with labels "Pending" are the ones in the database. They didn't become "Archived" yet since they were still being archived. We should change it to "Archived" before saving in the database.

[toyamagu-2021]

Thanks.
Firstly, I think we should change serverside, but I noticed it may intended behavior as commended below.

argo-workflows/server/workflow/workflow_server.go

Lines 143 to 144 in 97b6fa8

	// The archived workflow we saved in the database will only have "Pending" as the archival status.
	// We want to only keep the workflow that has the correct label to display correctly in the UI.

It is easy to save workflows as Archived in the following function, but I worry break related logic....

argo-workflows/persist/sqldb/workflow_archive.go

Line 85 in 97b6fa8

func (r *workflowArchive) ArchiveWorkflow(wf *wfv1.Workflow) error {

If you agree with changing the server side, I try to write a PR.

[terrytangyuan]

We can simply the logic in the first link you posted to the following: mergedWfs = append(mergedWfs, v[0])

Yes that's good direction.

[alexec]

You can’t label a workflow as Archived until after it is archived because any failure during archiving will result in the workflow never getting archived, then getting deleted forever. Workflows in the archive will always be labeled Pending.

What about a new Archiving status?

[terrytangyuan]

@alexec I meant that we don't modify the live workflow YAML at all. We only use modify the YAML that's sent to the database. So in the live workflow, the archival status will still reflect the true status. The archived workflow YAML in the database will be have "Archived" label since it's already saved in the database.

[toyamagu-2021]

OK, I think we should distinguish the following status for clarification.

1. Pending: has live manifest and is not archived
2. Archived: has live manifest and is archived
3. Retrieved: retrieved from DB (may or a may not be lived)

We should distinguish 2 from 3 for batch deletion on the frontend or convenience.
What do you think about this?
sample PR: toyamagu-2021#3

[terrytangyuan]

Sounds good to me.