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

Enhance error handling in _kube_api_error_handling #762

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Ygnas
Copy link
Contributor

@Ygnas Ygnas commented Nov 15, 2024

Issue link

https://issues.redhat.com/browse/RHOAIENG-15225

What changes have been made

Enhanced error messages to give more detailed errors to our users so that they have more information to feed the RHOAI administrators.

Verification steps

To test this install new codeflare-sdk from this PR, you could try deleteting non existent cluster, creating 2 clusters with same name, trying to create appwrapper.

Errors appear like this:

  • Conflict
    • A conflict occurred with the RayCluster resource. Only one RayCluster with the same name is allowed.
    • Please delete or rename the existing RayCluster before creating a new one with the desired name.
    • Response: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"rayclusters.ray.io "raytest" already exists","reason":"AlreadyExists","details":{"name":"raytest","group":"ray.io","kind":"rayclusters"},"code":409}
  • Forbidden
    • Access denied to the Kubernetes resource. Ensure your role has sufficient permissions for this operation.
    • Response: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"appwrappers.workload.codeflare.dev is forbidden: User "ignas" cannot create resource "appwrappers" in API group "workload.codeflare.dev" in the namespace "ignas"","reason":"Forbidden","details":{"group":"workload.codeflare.dev","kind":"appwrappers"},"code":403}
  • Unauthorized
    • Access to the API is unauthorized. Check your credentials or permissions.
    • Response: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"Unauthorized","reason":"Unauthorized","code":401}
  • Not found
    • The requested resource could not be located. Please verify the resource name and namespace.
    • Response: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"rayclusters.ray.io "raytest" not found","reason":"NotFound","details":{"name":"raytest","group":"ray.io","kind":"rayclusters"},"code":404}

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • Testing is not required for this change

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.04%. Comparing base (2bf575b) to head (8481faa).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #762      +/-   ##
==========================================
- Coverage   93.05%   93.04%   -0.01%     
==========================================
  Files          36       36              
  Lines        2403     2401       -2     
==========================================
- Hits         2236     2234       -2     
  Misses        167      167              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Bobbins228 Bobbins228 left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2024
@Ygnas Ygnas added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2024
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 18, 2024
Copy link
Contributor

openshift-ci bot commented Nov 18, 2024

New changes are detected. LGTM label has been removed.

Copy link
Contributor

openshift-ci bot commented Nov 18, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from bobbins228. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Ygnas Ygnas removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 18, 2024
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.

2 participants