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

Workflow Details page makes unnecessary Archived request #12814

Closed
4 tasks done
ryancurrah opened this issue Mar 16, 2024 · 6 comments · Fixed by #12972
Closed
4 tasks done

Workflow Details page makes unnecessary Archived request #12814

ryancurrah opened this issue Mar 16, 2024 · 6 comments · Fixed by #12972
Labels
area/api Argo Server API area/ui area/workflow-archive P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important solution/suggested A solution to the bug has been suggested. Someone needs to implement it. solution/workaround There's a workaround, might not be great, but exists type/bug type/regression Regression from previous behavior (a specific type of bug)

Comments

@ryancurrah
Copy link
Contributor

ryancurrah commented Mar 16, 2024

Pre-requisites

  • I have double-checked my configuration
  • I can confirm the issue exists when I tested with :latest
  • I have searched existing issues and could not find a match for this bug
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what did you expect to happen?

Users of Argo Workflows version 3.5.5 3.5 experience an issue where the Workflow Details page attempts to fetch workflows as if they were archived, even when the archived workflows feature is not utilized. This behavior results in an unnecessary API call, leading to the error message getting archived workflows not supported and potentially degrading the UI experience.

The issue manifests before the workflow details page loads, with the UI erroneously trying to retrieve an archived version of the workflow based on its UID.

Initial troubleshooting suggested setting the ARGO_ARTIFACT_SERVER environment variable to false as a potential fix. However, the error persisted on the workflow details page, and archived logs ceased to function after applying this workaround. edited by agilgur5: this is actually unrelated, see below comment.

Discussion on Slack highlighted a section of the frontend code responsible for this behavior, indicating that the UI checks for an archived workflow status unnecessarily. This check occurs even when archived workflows are not enabled, which should indicate that the workflow is not considered archived.

The conversation concluded with the suggestion to file a GitHub issue to track this problem, aiming for a more concise and efficient handling of workflow status checks in the UI to prevent unnecessary API calls and improve the user experience.

Potential Solutions:

  1. Adjust the frontend code to ensure that archived workflow checks are only performed when necessary, based on the presence of appropriate configuration settings.

Version

v3.5.5

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.

metadata:
  name: delightful-bear
  namespace: default
  labels:
    example: 'true'
spec:
  arguments:
    parameters:
      - name: message
        value: hello argo
  entrypoint: argosay
  templates:
    - name: argosay
      inputs:
        parameters:
          - name: message
            value: '{{workflow.parameters.message}}'
      container:
        name: main
        image: argoproj/argosay:v2
        command:
          - /argosay
        args:
          - echo
          - '{{inputs.parameters.message}}'
  ttlStrategy:
    secondsAfterCompletion: 300
  podGC:
    strategy: OnPodCompletion

Ensure workflow archives are disabled.

View the workflow details page. Look for the error message before the page loads.

2024-03-15 09 43 39

@agilgur5 agilgur5 added the type/regression Regression from previous behavior (a specific type of bug) label Mar 17, 2024
@agilgur5
Copy link
Contributor

agilgur5 commented Mar 17, 2024

For posterity, this was also mentioned in #11671 (reply in thread), #12759 (comment), and maybe others, but a separate issue for this hadn't been filed till now. This has existed since 3.5.0-rc1 and is due to the Archived/Live Workflow UI merge in 3.5 (i.e. #11121).

Also, per Slack, the message only shows up once and quickly disappears, the GIF is showing Ryan refresh a few times to capture the error message.

Initial troubleshooting suggested setting the ARGO_ARTIFACT_SERVER environment variable to false as a potential fix. However, the error persisted on the workflow details page, and archived logs ceased to function after applying this workaround.

Also for reference, that is correct behavior, that suggestion was not correct. Archive Logs and Archived Workflows are two separate features (though often used together), the former of which uses the Artifact Server, the latter of which uses a DB.

@agilgur5 agilgur5 added area/api Argo Server API area/ui area/workflow-archive P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important labels Mar 17, 2024
@agilgur5
Copy link
Contributor

agilgur5 commented Mar 17, 2024

  1. Adjust the frontend code to ensure that archived workflow checks are only performed when necessary, based on the presence of appropriate configuration settings.

Longer-term, I actually think of all of the front-end fallback calls on the Workflow Details page should be rewritten to run in the back-end. That would line up with the intent of #10781.

Also there's technically at least two problems here, one is the retrieval plus error message in the UI, the other is that the server error is a 500 (suggesting it was an unhandled error), as opposed to a 4xx or a different 5xx or something.
404 or 204 could perhaps makes sense. There is no status code for unsupported by the Server which would be most accurate though. Ironically, a 418 almost makes sense for an unsupported request.

@agilgur5 agilgur5 added the solution/workaround There's a workaround, might not be great, but exists label Mar 17, 2024
@ryancurrah
Copy link
Contributor Author

Ha funnily enough I do the inverse, I use a coffee press to brew loose leaf tea! Maybe that could be a 218 status code.

Do you think it's worth implementing a workaround or wait for the long term refactor?

My only concern about not resolving this is receiving feedback from users of Argo (At our company) multiple times about the brief error every time they view a workflow detail.

@agilgur5
Copy link
Contributor

agilgur5 commented Mar 18, 2024

about the brief error every time they view a workflow detail.

Yes I agree, and that is why I think it is worth having a shorter term fix. It would also be more correct. Then all of the Details calls could be refactored to run in the back-end later.

The one issue fixing it in the front-end is that it does not currently know of all settings such as Archived Workflows. So the fallback is necessary. But the ordering could be changed and the error could be caught and not shown to the user.

@agilgur5 agilgur5 added the solution/suggested A solution to the bug has been suggested. Someone needs to implement it. label Mar 25, 2024
@agilgur5 agilgur5 added this to the v3.5.x patches milestone Apr 4, 2024
@ryancurrah
Copy link
Contributor Author

ryancurrah commented Apr 23, 2024

ordering could be changed and the error could be caught and not shown to the user.

Ack I will take peek at this.

ryancurrah added a commit to ryancurrah/argo-workflows that referenced this issue Apr 23, 2024
Retrieving an archived workflow is the fallback now. Curently an error appears when loading a workflow because archived workflows are not enabled and the ui tries to get the archived workflow first. Related issue: argoproj#12814 and Slack thread: https://cloud-native.slack.com/archives/C01QW9QSSSK/p1710600357875049\?thread_ts\=1710510295.856659\&cid\=C01QW9QSSSK

Signed-off-by: Ryan Currah <ryan@currah.ca>
ryancurrah added a commit to ryancurrah/argo-workflows that referenced this issue Apr 24, 2024
Retrieving an archived workflow is the fallback now. Curently an error appears when loading a workflow because archived workflows are not enabled and the ui tries to get the archived workflow first. Related issue: argoproj#12814 and Slack thread: https://cloud-native.slack.com/archives/C01QW9QSSSK/p1710600357875049\?thread_ts\=1710510295.856659\&cid\=C01QW9QSSSK

Signed-off-by: Ryan Currah <ryan@currah.ca>
ryancurrah added a commit to ryancurrah/argo-workflows that referenced this issue Apr 24, 2024
Retrieving an archived workflow is the fallback now. Curently an error appears when loading a workflow because archived workflows are not enabled and the ui tries to get the archived workflow first. Related issue: argoproj#12814 and Slack thread: https://cloud-native.slack.com/archives/C01QW9QSSSK/p1710600357875049\?thread_ts\=1710510295.856659\&cid\=C01QW9QSSSK

Signed-off-by: Ryan Currah <ryan@currah.ca>
@agilgur5
Copy link
Contributor

agilgur5 commented Oct 25, 2024

Longer-term, I actually think of all of the front-end fallback calls on the Workflow Details page should be rewritten to run in the back-end. That would line up with the intent of #10781.

Ok apparently this already exists: #11055 was a precursor to #11121.

So the UI may need some refactoring / simplification as such, as it wouldn't need a fallback then

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 area/workflow-archive P2 Important. All bugs with >=3 thumbs up that aren’t P0 or P1, plus: Any other bugs deemed important solution/suggested A solution to the bug has been suggested. Someone needs to implement it. solution/workaround There's a workaround, might not be great, but exists type/bug type/regression Regression from previous behavior (a specific type of bug)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants