fix: API requests with single RM and recover from panic during unauthorized autocreate

amandavialva01 commented 3 months ago

Ticket

DET-10385, DET-10386

Description

This PR fixes the following issues:

API requests for single RM only work when the cluster name is specified in the request
Autocreate panic is not handled when attempted with OSS
Some lint issues after feature branch rebase with main
Autocreating namespace fails given workspace whose name doesn't match the accepted namespace regex pattern.

Test Plan

Spin up a single RM kubernetes EE cluster to execute the test plan. To test API requests for single RM, run the following commands and make sure that they work:

det w create ws1 --auto-create-namespace
det w create ws2 && det w bindings set ws2 --namespace default
det w create ws3 --namespace default
det w create ws4 && det w bindings set ws4 --auto-create-namespace

To test auto-create panic handled gracefully with an intuitive error message:

Kill your Kubernetes cluster (if it's EE) and spin up an OSS Kubernetes cluster (remove license.txt and public.txt from your determined directory).
Run det w create ws10 --auto-create-namespace and verify that we get an error saying that auto create is an EE-only feature
Run det w create ws11 && det w bindings set ws11 --auto-create-namespace and verify that we get an error saying that auto create is an EE-only feature

To test namespace auto-creation with workspaces whose names don't match the accepted namespace regex pattern, run det w create name,of_workspacE --auto-create-namespace and verify that a Kubernetes namespace is successfully created

Run det w create ANOTHER,name,of_workspacE && det w bindings set ANOTHER,name,of_workspacE --auto-create-namespace and verify that a Kubernetes namespace is successfully created

Checklist

[ ] Changes have been manually QA'd
[ ] New features have been approved by the corresponding PM
[ ] User-facing API changes have the "User-facing API Change" label
[ ] Release notes have been added as a separate file under docs/release-notes/ See Release Note for details.
[ ] Licenses have been included for new code which was copied and/or modified from any external code

netlify[bot] commented 3 months ago

Deploy Preview for determined-ui ready!

Name	Link
Latest commit	82011dccccf252242ef7ef553b0b64de1e02ed24
Latest deploy log	https://app.netlify.com/sites/determined-ui/deploys/667c323121ae3f0008b78e4b
Deploy Preview	https://deploy-preview-9573--determined-ui.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 41.66667% with 14 lines in your changes missing coverage. Please review.

Project coverage is 51.75%. Comparing base (86214e5) to head (95a68f0).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## wksp-namespace-binding #9573 +/- ## ========================================================== - Coverage 51.75% 51.75% -0.01% ========================================================== Files 1255 1255 Lines 154074 154076 +2 Branches 3120 3120 ========================================================== - Hits 79747 79746 -1 - Misses 74172 74175 +3 Partials 155 155 ``` | [Flag](https://app.codecov.io/gh/determined-ai/determined/pull/9573/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | Coverage Δ | | |---|---|---| | [backend](https://app.codecov.io/gh/determined-ai/determined/pull/9573/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `43.82% <69.23%> (-0.01%)` | :arrow_down: | | [harness](https://app.codecov.io/gh/determined-ai/determined/pull/9573/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `72.61% <9.09%> (-0.01%)` | :arrow_down: | | [web](https://app.codecov.io/gh/determined-ai/determined/pull/9573/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | `49.15% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/determined-ai/determined/pull/9573?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai) | Coverage Δ | | |---|---|---| | [master/internal/api\_workspace.go](https://app.codecov.io/gh/determined-ai/determined/pull/9573?src=pr&el=tree&filepath=master%2Finternal%2Fapi_workspace.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-bWFzdGVyL2ludGVybmFsL2FwaV93b3Jrc3BhY2UuZ28=) | `59.07% <69.23%> (+0.25%)` | :arrow_up: | | [harness/determined/cli/workspace.py](https://app.codecov.io/gh/determined-ai/determined/pull/9573?src=pr&el=tree&filepath=harness%2Fdetermined%2Fcli%2Fworkspace.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai#diff-aGFybmVzcy9kZXRlcm1pbmVkL2NsaS93b3Jrc3BhY2UucHk=) | `16.50% <9.09%> (-0.32%)` | :arrow_down: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/determined-ai/determined/pull/9573/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=determined-ai)

amandavialva01 commented 2 months ago

LGTM!

Minor suggestion: this PR fixes a couple bugs right? Are there any tests we could add to help confirm the issues are fixed and to ensure they don't pop up again? (If not, that's fine. You know the work better than I do!) We are planning to do automated testing that covers the bugs fixed here in separate PRs since there are a lot of features to test! I have manually tested everything following the description instructions!

determined-ai / determined