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

Unify the workflow + workflow archive UI/API #10781

Closed
jessesuen opened this issue Mar 29, 2023 · 8 comments · Fixed by #11121
Closed

Unify the workflow + workflow archive UI/API #10781

jessesuen opened this issue Mar 29, 2023 · 8 comments · Fixed by #11121
Assignees
Labels
area/api Argo Server API area/ui type/feature Feature request

Comments

@jessesuen
Copy link
Member

Summary

We have two categories/tabs of workflows when presenting them in our UI:

  • in-cluster (only in k8s cluster)
  • archived to a database

Depending on the archive configuration, the ttl strategy, and whether or not workflows are completed or not, a workflow may appear in the in-cluster list, the archive list, or even both.

A complaint among users is that end users somehow need to understand where to find their workflow in either of these two places. But they don't necessarily understand or care where to look.

Contrast this to just about any other batch processing system, which makes no distinction between something that is live vs. archived. You just arrive at a list of your recent runs, and there is a single page where you can find all your jobs.

The feature that would benefit the UX, is providing a single unified table where both in-cluster workflows and workflows from the archive can be seen in a single place.

The technical challenge is that archived workflows must be retrieved by UID (but can be searched by name), whereas live workflows must be retrieved by name.

One way to potentially mitigate this is: give an option such that a workflow name can be a primary/unique value in the database. It would be the responsibility of the end user, to name their workflows with a name that would not collide with a previous run. And if there was a collision, the latest one would override the previous.

Use Cases

Any users that making use of the persistence feature would likely prefer this mode of operation.


Message from the maintainers:

Love this enhancement proposal? Give it a 👍. We prioritise the proposals with the most 👍.

@jessesuen jessesuen added the type/feature Feature request label Mar 29, 2023
@jessesuen jessesuen changed the title Unify the workflow + workflow archive view in UI Unify the workflow + workflow archive UI/API Mar 29, 2023
@jessesuen
Copy link
Member Author

One key thing about this feature is that I consider this more of an Argo API Server feature rather than a UI feature. It's not just about unifying the UI view. It's also about unifying the API itself, so things like RBAC, API requests, etc... are consistent on whether you are getting a workflow in-cluster or from the archive.

Another way to put it, it should be possible to write the front-end or CLI in a way where it does not know if workflows are archived or not.

@jessesuen
Copy link
Member Author

jessesuen commented Mar 29, 2023

Here is another reason why needs to be addressed in the API layer. Below is a valid complaint about what an application needs to deal with the argo API:

  • We need to check the status of workflows that may be running or may have been archived.
  • We call the k8s api and look for a workflow. If it exists we return the current running state.
  • If it does not exist we call the /api/v1/archived-workflows?namePrefix=WORKFLOW_NAME
  • If we find a workflow in that list we grab the UID and then make another api call to /api/v1/archived-workflows/UID and grab the finished status.

All that results in up to 3 api calls to return the status

This supports my proposal that if we allow an persistance option where the workflow name + namespace is used as the primary key (as opposed to UID), then that would enable a unified API.

@terrytangyuan terrytangyuan added area/ui area/api Argo Server API labels Apr 4, 2023
@alexec
Copy link
Contributor

alexec commented Apr 11, 2023

I think this should go into the API too

Namespace+name in not the primary key. It is UID.

But I don’t think it matters.

Update GetWorkflow. This could accept namespace+name. If the workflow is found, return it. If not, look for the archived workflow. If only one is found, return that. If two are found, return an error.

I did not see anything in your requirements that would suggest ListWorkflows needs changing.

@sarabala1979
Copy link
Member

I like this idea to combine in-cluster workflow and archive workflow. Instead of modifying the existing API, we can add a new API and deprecate the existing if there is a change in definition.

UI should show the icon of whether the workflow is archived or not (List page and workflow Details ).

@terrytangyuan
Copy link
Member

Migrating my comment #11055 (comment) to here for further discussions.

Introducing new API will bring more maintenance effort IMO. There are already multiple artifacts APIs that need to be refactored/deprecated later. Perhaps we can add a parameter, e.g. lookUpArchived=true, to toggle this functionality to maintain compatibility?

Any thoughts? @alexec @jessesuen

@sarabala1979
Copy link
Member

Is it going to be a breaking change? Can we support this feature with backward compatibility?
I would recommend adding a new API for GetAllWorkflows, and ListAllWorkflows and deprecating the archive APIs.
So Release will be backward compatible

@terrytangyuan
Copy link
Member

Is it going to be a breaking change? Can we support this feature with backward compatibility?

The toggle I mentioned above should provide backward compatibility so it won't be a breaking change. If lookUpArchived=false (by default), we stick with the current behavior.

@sarabala1979
Copy link
Member

I am fine as long as there is no existing API definition/signature change and only just internal functions. But I saw there is proto change and API change on PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Argo Server API area/ui type/feature Feature request
Projects
Development

Successfully merging a pull request may close this issue.

4 participants