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

fix: when resource does not exist node menu and resource details shou… #12360

Merged

Conversation

jphelton
Copy link
Contributor

@jphelton jphelton commented Feb 9, 2023

When a resource is "missing" the resource tree action items menu and the resource details sliding panel would not render properly due to an uncaught error from the GetResourceLinks API

Signed-off-by: Joshua Helton jdoghelton@gmail.com

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).

…ld still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
@codecov
Copy link

codecov bot commented Feb 9, 2023

Codecov Report

Base: 47.62% // Head: 47.62% // No change to project coverage 👍

Coverage data is based on head (1fb0240) compared to base (17167fc).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #12360   +/-   ##
=======================================
  Coverage   47.62%   47.62%           
=======================================
  Files         246      246           
  Lines       41806    41806           
=======================================
  Hits        19911    19911           
  Misses      19902    19902           
  Partials     1993     1993           
Impacted Files Coverage Δ
util/settings/settings.go 49.36% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jphelton jphelton marked this pull request as ready for review February 9, 2023 03:26
@crenshaw-dev
Copy link
Collaborator

I feel like we should probably fix this server-side. If someone requests links for a resource that is in the resource tree but not yet created, it seems valid to return the links rather than an error. @alexmt what do you think?

@jphelton
Copy link
Contributor Author

jphelton commented Feb 9, 2023

I feel like we should probably fix this server-side. If someone requests links for a resource that is in the resource tree but not yet created, it seems valid to return the links rather than an error. @alexmt what do you think?

It seems reasonable to make that change as well, but I still think that this change would be required as well. A failure from the ResourceLink API shouldn't block the rendering of either the node-dropdown, or the resource-detail panel. This error handling behavior is present for the most other API calls for these components

Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is a blocking regression, I would fix it in UI first and then work on improvement for v2.7.

We have exact same behavior with resource actions: UI attempts to load the actions list and just silently fallback to an empty list in case of error. So we could improve both in v2.7.

So LGTM. WDYT @crenshaw-dev

@crenshaw-dev
Copy link
Collaborator

Makes sense!

@alexmt alexmt merged commit 3d02e01 into argoproj:master Feb 9, 2023
@jphelton
Copy link
Contributor Author

jphelton commented Feb 9, 2023

I've never tried to get a change added into a patch version before.

Should I cherry-pick these changes onto release-2.6 and open a new PR? Or is that something that only collaborators do?

jphelton added a commit to jphelton/argo-cd that referenced this pull request Feb 9, 2023
argoproj#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
(cherry picked from commit 3d02e01)
alexmt pushed a commit to alexmt/argo-cd that referenced this pull request Feb 10, 2023
argoproj#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
@alexmt
Copy link
Collaborator

alexmt commented Feb 10, 2023

@jphelton just cherry-picked changes into 2.6 release branch. Works great - thank you!

rumstead pushed a commit to rumstead/argo-cd that referenced this pull request Feb 10, 2023
argoproj#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
@jakubtomiak97
Copy link

Hey @alexmt, did you merge that into 2.6 branch? As I see that PR was merged only to the master branch at the moment, so on version argocd 2.6 this is still broken.

alexmt pushed a commit that referenced this pull request Feb 16, 2023
#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
@alexmt
Copy link
Collaborator

alexmt commented Feb 16, 2023

Ah, sorry! Apparently, I did not run git push. Just pushed it but we need another 2.6 patch unfortunately.

@jphelton jphelton deleted the bugfix/resource-details-when-missing branch February 16, 2023 17:10
@bakito
Copy link

bakito commented Feb 22, 2023

Ah, sorry! Apparently, I did not run git push. Just pushed it but we need another 2.6 patch unfortunately.

Has this second patch been released already, as I still get the error with v2.6.2?

@chrism417
Copy link

I'm also seeing this same error with 2.6.2

crenshaw-dev added a commit that referenced this pull request Feb 24, 2023
* add unit test reproducing

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* feat: Begin polishing top bar design (#12327)

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* chore: add dist to path to use our kustomize version (#12352)

* chore: add dist to path to use our kustomize version

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* correct path

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* missed a spot

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: when resource does not exist node menu and resource details shou… (#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* remove duplicate code

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* Retrigger CI pipeline

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* revert gitignore

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* update from code review

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

---------

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: jphelton <jdoghelton@gmail.com>
gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Feb 24, 2023
* add unit test reproducing

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* feat: Begin polishing top bar design (#12327)

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* chore: add dist to path to use our kustomize version (#12352)

* chore: add dist to path to use our kustomize version

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* correct path

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* missed a spot

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: when resource does not exist node menu and resource details shou… (#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* remove duplicate code

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* Retrigger CI pipeline

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* revert gitignore

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* update from code review

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

---------

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: jphelton <jdoghelton@gmail.com>
gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Feb 24, 2023
* add unit test reproducing

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* feat: Begin polishing top bar design (#12327)

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* chore: add dist to path to use our kustomize version (#12352)

* chore: add dist to path to use our kustomize version

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* correct path

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* missed a spot

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: when resource does not exist node menu and resource details shou… (#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* remove duplicate code

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* Retrigger CI pipeline

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* revert gitignore

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* update from code review

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

---------

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: jphelton <jdoghelton@gmail.com>
crenshaw-dev added a commit that referenced this pull request Feb 24, 2023
…) (#12611)

* add unit test reproducing




* feat: Begin polishing top bar design (#12327)



* chore: add dist to path to use our kustomize version (#12352)

* chore: add dist to path to use our kustomize version



* correct path



* missed a spot



---------




* fix: when resource does not exist node menu and resource details shou… (#12360)

* fix: when resource does not exist node menu and resource details should still render



* Retrigger CI pipeline



---------




* fix: traverse generator tree when getting requeue time



* fix: traverse generator tree when getting requeue time



* remove duplicate code



* Retrigger CI pipeline



* revert gitignore



* update from code review



---------

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Co-authored-by: rumstead <37445536+rumstead@users.noreply.github.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: jphelton <jdoghelton@gmail.com>
crenshaw-dev added a commit that referenced this pull request Feb 24, 2023
* add unit test reproducing




* feat: Begin polishing top bar design (#12327)



* chore: add dist to path to use our kustomize version (#12352)

* chore: add dist to path to use our kustomize version



* correct path



* missed a spot



---------




* fix: when resource does not exist node menu and resource details shou… (#12360)

* fix: when resource does not exist node menu and resource details should still render



* Retrigger CI pipeline



---------




* fix: traverse generator tree when getting requeue time



* fix: traverse generator tree when getting requeue time



* remove duplicate code



* Retrigger CI pipeline



* revert gitignore



* update from code review



---------

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Co-authored-by: rumstead <37445536+rumstead@users.noreply.github.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: jphelton <jdoghelton@gmail.com>
crenshaw-dev added a commit that referenced this pull request Feb 24, 2023
* add unit test reproducing

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* feat: Begin polishing top bar design (#12327)

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* chore: add dist to path to use our kustomize version (#12352)

* chore: add dist to path to use our kustomize version

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* correct path

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* missed a spot

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: when resource does not exist node menu and resource details shou… (#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* remove duplicate code

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* Retrigger CI pipeline

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* revert gitignore

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* update from code review

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

---------

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: jphelton <jdoghelton@gmail.com>
crenshaw-dev added a commit that referenced this pull request Feb 24, 2023
* add unit test reproducing




* feat: Begin polishing top bar design (#12327)



* chore: add dist to path to use our kustomize version (#12352)

* chore: add dist to path to use our kustomize version



* correct path



* missed a spot



---------




* fix: when resource does not exist node menu and resource details shou… (#12360)

* fix: when resource does not exist node menu and resource details should still render



* Retrigger CI pipeline



---------




* fix: traverse generator tree when getting requeue time



* fix: traverse generator tree when getting requeue time



* remove duplicate code



* Retrigger CI pipeline



* revert gitignore



* update from code review



---------

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Co-authored-by: rumstead <37445536+rumstead@users.noreply.github.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Co-authored-by: jphelton <jdoghelton@gmail.com>
yyzxw pushed a commit to yyzxw/argo-cd that referenced this pull request Aug 9, 2023
… (argoproj#12409)

* add unit test reproducing

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* feat: Begin polishing top bar design (argoproj#12327)

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* chore: add dist to path to use our kustomize version (argoproj#12352)

* chore: add dist to path to use our kustomize version

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* correct path

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

* missed a spot

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>

---------

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: when resource does not exist node menu and resource details shou… (argoproj#12360)

* fix: when resource does not exist node menu and resource details should still render

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

* Retrigger CI pipeline

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>

---------

Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* fix: traverse generator tree when getting requeue time

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* remove duplicate code

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* Retrigger CI pipeline

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* revert gitignore

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

* update from code review

Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>

---------

Signed-off-by: rumstead <rjumstead@gmail.com>
Signed-off-by: rumstead <37445536+rumstead@users.noreply.github.com>
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Signed-off-by: Joshua Helton <jdoghelton@gmail.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Co-authored-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Co-authored-by: jphelton <jdoghelton@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants