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: Display error msg when live state is empty #13031

Merged

Conversation

ashutosh16
Copy link
Contributor

@ashutosh16 ashutosh16 commented Mar 28, 2023

This PR displays the error message in the live state editor when argocd failed to fetch the resource manifest due to a change in the API version. This issue occurred when k8s cluster is upgraded to 1.22 and the API networking.k8s.io/v1beta1 is deprecated. ArgoCD is unable to fetch the live state of the resource and displays the {} state. In this PR, the user will see relevant information regarding the correct API version to use

@ashutosh16 ashutosh16 force-pushed the fix-display-user--on-live-manifest branch from d30fafe to fce426e Compare March 28, 2023 16:30
@ashutosh16 ashutosh16 marked this pull request as ready for review March 28, 2023 16:30
@ashutosh16
Copy link
Contributor Author

Screenshot 2023-03-24 at 9 16 29 AM

@codecov
Copy link

codecov bot commented Mar 28, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (696631e) 49.06% compared to head (bfc0e02) 49.05%.

❗ Current head bfc0e02 differs from pull request most recent head 67da494. Consider uploading reports for the commit 67da494 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13031      +/-   ##
==========================================
- Coverage   49.06%   49.05%   -0.01%     
==========================================
  Files         246      246              
  Lines       42561    42561              
==========================================
- Hits        20881    20878       -3     
- Misses      19565    19567       +2     
- Partials     2115     2116       +1     

see 1 file with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ashutosh16 ashutosh16 force-pushed the fix-display-user--on-live-manifest branch from fce426e to 7900853 Compare March 29, 2023 21:01
@ashutosh16 ashutosh16 requested a review from ciiay March 29, 2023 21:01
@ashutosh16
Copy link
Contributor Author

Additional screenshots showing the message when API version is different from the git
Screenshot 2023-03-29 at 2 09 04 PM
Screenshot 2023-03-29 at 2 08 58 PM

Copy link
Contributor

@ciiay ciiay left a comment

Choose a reason for hiding this comment

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

LGTM

@drpaneas
Copy link
Contributor

cool, it displays the error msg, thanks for providing the screenshot as well
/lgtm

@crenshaw-dev
Copy link
Member

/cherry-pick release-2.7

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>
Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>
Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>
@ashutosh16 ashutosh16 force-pushed the fix-display-user--on-live-manifest branch from 262e18d to 9823896 Compare March 30, 2023 20:12
@crenshaw-dev
Copy link
Member

Testing locally I get this error:

TypeError: Cannot read properties of null (reading 'kind')
    at getResourceTabs (webpack-internal:///./src/app/applications/components/resource-details/resource-details.tsx:136:30)
    at Object.eval [as children] (webpack-internal:///./src/app/applications/components/resource-details/resource-details.tsx:345:11)
    at DataLoader.render (webpack-internal:///./node_modules/argo-ui/src/components/data-loader.tsx:79:25)
    at finishClassComponent (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:11841:35)
    at updateClassComponent (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:11809:28)
    at beginWork (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:12758:18)
    at HTMLUnknownElement.callCallback2 (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:103:18)
    at Object.invokeGuardedCallbackDev (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:128:20)
    at invokeGuardedCallback (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:159:35)
    at beginWork$1 (webpack-internal:///./node_modules/react-dom/cjs/react-dom.development.js:15669:11)

Test app:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: test
spec:
  destination:
    namespace: default
    server: https://kubernetes.default.svc
  project: default
  source:
    path: kustomize-guestbook
    repoURL: https://github.com/crenshaw-dev/argocd-example-apps
    targetRevision: bad-api-version

@ashutosh16
Copy link
Contributor Author

@crenshaw-dev I successfully created and sync the application using the manifest you provide and I couldn't reproduce the error locally, the UI message shows up fine (error msg shows in the live manifest).
If possible can @drpaneas/@ciiay take a second look, try creating a new application using the below config and Sync

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: test
spec:
  destination:
    namespace: default
    server: https://kubernetes.default.svc
  project: default
  source:
    path: kustomize-guestbook
    repoURL: https://github.com/crenshaw-dev/argocd-example-apps
    targetRevision: bad-api-version

@ashutosh16 ashutosh16 requested review from drpaneas and ciiay April 3, 2023 16:16
@ciiay
Copy link
Contributor

ciiay commented Apr 3, 2023

I was able to create the app from the below command. It's OutOfSync but as expected to test the PR I think.

➜  ~ cat <<EOF | oc apply -f -
apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: test
spec:
  destination:
    namespace: default
    server: https://kubernetes.default.svc
  project: default
  source:
    path: kustomize-guestbook
    repoURL: https://github.com/crenshaw-dev/argocd-example-apps
    targetRevision: bad-api-version
EOF
application.argoproj.io/test created

@crenshaw-dev crenshaw-dev merged commit cd46c76 into argoproj:master May 28, 2023
@gcp-cherry-pick-bot
Copy link

Cherry-pick failed with Merge error cd46c76ef8f95c6525ae7c8f81131a3b0f038d0b into temp-cherry-pick-6876bf-release-2.7

yyzxw pushed a commit to yyzxw/argo-cd that referenced this pull request Aug 9, 2023
* fix: disply error when live state is empty

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>

* fix: disply error when live state is empty

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>

* fix: disply error when live state is empty

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>

---------

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>
Signed-off-by: asingh <11219262+ashutosh16@users.noreply.github.com>
tesla59 pushed a commit to tesla59/argo-cd that referenced this pull request Dec 16, 2023
* fix: disply error when live state is empty

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>

* fix: disply error when live state is empty

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>

* fix: disply error when live state is empty

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>

---------

Signed-off-by: ashutosh16 <11219262+ashutosh16@users.noreply.github.com>
Signed-off-by: asingh <11219262+ashutosh16@users.noreply.github.com>
@jsoref jsoref mentioned this pull request Feb 27, 2024
3 tasks
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.

4 participants