diff --git a/e2e_tests/tests/cluster/test_agent_user_group.py b/e2e_tests/tests/cluster/test_agent_user_group.py index 740f0330efa..43b320db60b 100644 --- a/e2e_tests/tests/cluster/test_agent_user_group.py +++ b/e2e_tests/tests/cluster/test_agent_user_group.py @@ -102,6 +102,10 @@ def test_workspace_post_gid() -> None: _check_test_experiment(sess, p.id) _check_test_command(sess, w.name) finally: + # Delete the project so the workspace can be deleted + bindings.delete_DeleteProject(sess, id=p.id) + # Wait for the delete to finish + time.sleep(0.5) _delete_workspace_and_check(sess, w) @@ -141,6 +145,10 @@ def test_workspace_patch_gid() -> None: _check_test_experiment(sess, p.id) _check_test_command(sess, w.name) finally: + # Delete the project so the workspace can be deleted + bindings.delete_DeleteProject(sess, id=p.id) + # Wait for the delete to finish + time.sleep(0.5) _delete_workspace_and_check(sess, w) diff --git a/e2e_tests/tests/cluster/test_rbac_ntsc.py b/e2e_tests/tests/cluster/test_rbac_ntsc.py index d595138f3a8..9f292a22f66 100644 --- a/e2e_tests/tests/cluster/test_rbac_ntsc.py +++ b/e2e_tests/tests/cluster/test_rbac_ntsc.py @@ -1,4 +1,5 @@ import contextlib +import time from typing import Generator, List, Optional, Sequence import pytest @@ -134,6 +135,7 @@ def can_access_logs(sess: api.Session, ntsc_id: str) -> bool: ], ] ) as (workspaces, creds): + created_projects: List[int] = [] # launch one of each ntsc in the first workspace for typ in conf.ALL_NTSC: experiment_id = None @@ -143,6 +145,7 @@ def can_access_logs(sess: api.Session, ntsc_id: str) -> bool: body=bindings.v1PostProjectRequest(name="test", workspaceId=workspaces[0].id), workspaceId=workspaces[0].id, ).project.id + created_projects.append(pid) # experiment for tensorboard experiment_id = noop.create_experiment( @@ -233,6 +236,11 @@ def can_access_logs(sess: api.Session, ntsc_id: str) -> bool: # kill the ntsc api_utils.kill_ntsc(creds[0], typ, created_id) + # Delete the project so the workspace can be deleted + for pid in created_projects: + bindings.delete_DeleteProject(creds[0], id=pid) + # Wait for deletion + time.sleep(0.5) @pytest.mark.e2e_cpu_rbac @@ -256,6 +264,7 @@ def get_proxy(sess: api.Session, task_id: str) -> Optional[errors.APIException]: ], ] ) as (workspaces, creds): + created_projects: List[int] = [] # launch one of each ntsc in the first workspace for typ in conf.PROXIED_NTSC: experiment_id = None @@ -265,6 +274,7 @@ def get_proxy(sess: api.Session, task_id: str) -> Optional[errors.APIException]: body=bindings.v1PostProjectRequest(name="test", workspaceId=workspaces[0].id), workspaceId=workspaces[0].id, ).project.id + created_projects.append(pid) # experiment for tensorboard experiment_id = noop.create_experiment( @@ -298,6 +308,12 @@ def get_proxy(sess: api.Session, task_id: str) -> Optional[errors.APIException]: # kill the ntsc api_utils.kill_ntsc(creds[0], typ, created_id) + for pid in created_projects: + # Delete the project so the workspace can be deleted + bindings.delete_DeleteProject(creds[0], id=pid) + # Wait for deletion + time.sleep(0.5) + @pytest.mark.e2e_cpu_rbac @api_utils.skipif_rbac_not_enabled() diff --git a/e2e_tests/tests/cluster/test_webhooks.py b/e2e_tests/tests/cluster/test_webhooks.py index fdcb65fd597..3149e4e0c61 100644 --- a/e2e_tests/tests/cluster/test_webhooks.py +++ b/e2e_tests/tests/cluster/test_webhooks.py @@ -190,6 +190,10 @@ def test_log_pattern_send_webhook(should_match: bool) -> None: for i in ws_id: bindings.delete_DeleteWebhook(sess, id=i or 0) + # Delete the project so the workspace can be deleted + bindings.delete_DeleteProject(sess, id=project.id) + # Wait for deletion + time.sleep(0.5) test_agent_user_group._delete_workspace_and_check(sess, workspace) @@ -274,6 +278,10 @@ def test_custom_webhook(isSlack: bool) -> None: assert str(experiment_id) in responses["/"] bindings.delete_DeleteWebhook(sess, id=w.id or 0) + # Delete the project so the workspace can be deleted + bindings.delete_DeleteProject(sess, id=project.id) + # Wait for deletion + time.sleep(0.5) test_agent_user_group._delete_workspace_and_check(sess, workspace) @@ -336,6 +344,11 @@ def test_specific_webhook() -> None: bindings.delete_DeleteWebhook(sess, id=webhook_res_1.id or 0) bindings.delete_DeleteWebhook(sess, id=webhook_res_2.id or 0) + + # Delete the project so the workspace can be deleted + bindings.delete_DeleteProject(sess, id=project.id) + # Wait for deletion + time.sleep(0.5) test_agent_user_group._delete_workspace_and_check(sess, workspace) diff --git a/e2e_tests/tests/cluster/test_workspace_org.py b/e2e_tests/tests/cluster/test_workspace_org.py index f22996ed69d..ffba1eb6881 100644 --- a/e2e_tests/tests/cluster/test_workspace_org.py +++ b/e2e_tests/tests/cluster/test_workspace_org.py @@ -4,6 +4,7 @@ import random import re import tempfile +import time import uuid from typing import Generator, List, Optional, Tuple @@ -381,6 +382,10 @@ def test_workspace_org() -> None: assert e.value.status_code == http.HTTPStatus.CONFLICT finally: + # Clean out projects so workspaces can be cleaned + for p in test_projects: + bindings.delete_DeleteProject(sess, id=p.id) + time.sleep(0.5) # Clean out workspaces and all dependencies. for w in test_workspaces: bindings.delete_DeleteWorkspace(sess, id=w.id) @@ -469,9 +474,18 @@ def setup_workspaces( for e in exps: if e.workspaceId not in wids: continue - bindings.post_KillExperiment(session, id=e.id) + bindings.delete_DeleteExperiment(session, experimentId=e.id) + time.sleep(0.5) for w in workspaces: + projects = bindings.get_GetWorkspaceProjects( + session, + id=w.id, + sortBy=bindings.v1GetWorkspaceProjectsRequestSortBy.NAME, + ).projects + for p in projects: + bindings.delete_DeleteProject(session, id=p.id) + time.sleep(0.5) # TODO check if it needs deleting. bindings.delete_DeleteWorkspace(session, id=w.id) diff --git a/e2e_tests/tests/experiment/test_allocation_csv.py b/e2e_tests/tests/experiment/test_allocation_csv.py index ef3483af697..cbe9d711979 100644 --- a/e2e_tests/tests/experiment/test_allocation_csv.py +++ b/e2e_tests/tests/experiment/test_allocation_csv.py @@ -3,6 +3,7 @@ import io import re import sys +import time import uuid from typing import Optional @@ -99,6 +100,8 @@ def test_experiment_capture() -> None: assert r.status_code == requests.codes.ok, r.text validate_trial_csv_rows(r.text, exp_ref.id, w1.name) + bindings.delete_DeleteProject(session=sess, id=p1.id) + time.sleep(0.5) # Clean up test workspaces bindings.delete_DeleteWorkspace(session=sess, id=w1.id) bindings.delete_DeleteWorkspace(session=sess, id=w2.id) diff --git a/e2e_tests/tests/experiment/test_api.py b/e2e_tests/tests/experiment/test_api.py index a3707cae516..6bc40b198d3 100644 --- a/e2e_tests/tests/experiment/test_api.py +++ b/e2e_tests/tests/experiment/test_api.py @@ -1,3 +1,4 @@ +import time import uuid from typing import Dict, List @@ -13,6 +14,7 @@ def test_archived_proj_exp_list() -> None: admin = api_utils.admin_session() workspaces: List[bindings.v1Workspace] = [] + created_projects: List[int] = [] count = 2 for _ in range(count): @@ -34,6 +36,7 @@ def test_archived_proj_exp_list() -> None: workspaceId=wrkspc.id, ).project.id workspace_projects.append(pid) + created_projects.append(pid) for p in workspace_projects: for _ in range(count): @@ -105,5 +108,8 @@ def test_archived_proj_exp_list() -> None: for e_id in experiments: assert e_id in returned_e_id + for pid in created_projects: + bindings.delete_DeleteProject(admin, id=pid) + time.sleep(0.5) for w in workspaces: bindings.delete_DeleteWorkspace(admin, id=w.id) diff --git a/master/internal/api_workspace.go b/master/internal/api_workspace.go index d082467cc3d..98a74748ef4 100644 --- a/master/internal/api_workspace.go +++ b/master/internal/api_workspace.go @@ -307,6 +307,17 @@ func (a *apiServer) workspaceHasModels(ctx context.Context, workspaceID int32) ( return exists, nil } +func (a *apiServer) workspaceHasExperiments(ctx context.Context, workspaceID int32) (bool, error) { + exists, err := db.Bun().NewSelect().Table("experiments"). + Join("INNER JOIN projects ON experiments.project_id = projects.id"). + Where("projects.workspace_id=?", workspaceID). + Exists(ctx) + if err != nil { + return false, fmt.Errorf("checking workspace for experiments: %w", err) + } + return exists, nil +} + func (a *apiServer) GetWorkspace( ctx context.Context, req *apiv1.GetWorkspaceRequest, ) (*apiv1.GetWorkspaceResponse, error) { @@ -1293,13 +1304,22 @@ func (a *apiServer) DeleteWorkspace( return nil, err } - modelsExist, err := a.workspaceHasModels(ctx, req.Id) - if err != nil { - return nil, err + checks := []struct { + checkFunc func(context.Context, int32) (bool, error) + errorStr string + }{ + {a.workspaceHasModels, "workspace (%d) contains models; move or delete models first"}, + {a.workspaceHasExperiments, "workspace (%d) contains experiments; move or delete experiments first"}, } - if modelsExist { - return nil, status.Errorf(codes.FailedPrecondition, "workspace (%d) contains models; move or delete models first", - req.Id) + + for _, check := range checks { + exists, err := check.checkFunc(ctx, req.Id) + if err != nil { + return nil, err + } + if exists { + return nil, status.Errorf(codes.FailedPrecondition, check.errorStr, req.Id) + } } holder := &workspacev1.Workspace{} diff --git a/master/internal/api_workspace_intg_test.go b/master/internal/api_workspace_intg_test.go index 72e6d3358d3..cc1ee8b22a7 100644 --- a/master/internal/api_workspace_intg_test.go +++ b/master/internal/api_workspace_intg_test.go @@ -10,6 +10,7 @@ import ( "fmt" "strconv" "testing" + "time" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -939,7 +940,7 @@ func TestDeleteWorkspace(t *testing.T) { mockRM := MockRM() noName := "" // set up api server - api, _, ctx := setupAPITest(t, nil, mockRM) + api, curUser, ctx := setupAPITest(t, nil, mockRM) api.m.allRms = map[string]rm.ResourceManager{noName: mockRM} // set up command service - required for successful DeleteWorkspaceRequest calls cs, err := command.NewService(api.m.db, api.m.rm) @@ -1002,6 +1003,44 @@ func TestDeleteWorkspace(t *testing.T) { Id: resp.Workspace.Id, }) require.Error(t, err) + + // create another workspace, and add a experiment + w, p := createProjectAndWorkspace(ctx, t, api) + err = db.Bun().RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error { + autoGeneratedNamespaceName, err := getAutoGeneratedNamespaceName(ctx, w, &tx) + require.NoError(t, err) + autoCreatedNamespace = autoGeneratedNamespaceName + return nil + }) + require.NoError(t, err) + e := createTestExpWithProjectID(t, api, curUser, p) + _, err = db.Bun().NewUpdate().Table("experiments"). + Set("state = ?", model.CompletedState). + Where("id = ?", e.ID).Exec(ctx) + require.NoError(t, err) + _, err = api.DeleteWorkspace(ctx, &apiv1.DeleteWorkspaceRequest{ + Id: int32(w), + }) + + require.Error(t, err) + // delete experiment, so that workspace can be deleted + _, err = api.DeleteExperiment(ctx, &apiv1.DeleteExperimentRequest{ExperimentId: int32(e.ID)}) + require.NoError(t, err) + + // since delete experiment is async, we need to wait for the experiment to be deleted + for i := 0; i < 10; i++ { + _, err = api.GetExperiment(ctx, &apiv1.GetExperimentRequest{ExperimentId: int32(e.ID)}) + if err != nil { + break + } + time.Sleep(1 * time.Second) + } + // delete the workspace successfully + mockRM.On("DeleteNamespace", *autoCreatedNamespace).Return(nil).Once() + _, err = api.DeleteWorkspace(ctx, &apiv1.DeleteWorkspaceRequest{ + Id: int32(w), + }) + require.NoError(t, err) } func TestSetWorkspaceNamespaceBindings(t *testing.T) { diff --git a/webui/react/scripts/README.md b/webui/react/scripts/README.md index b78c950bdea..536affd21d6 100644 --- a/webui/react/scripts/README.md +++ b/webui/react/scripts/README.md @@ -7,7 +7,6 @@ eg a remote Determined master. For example, to connect the WebUI to a remote cluster with address `MY_SERVER_ADDRESS` you'd run the proxy with `./proxy.js MY_SERVER_ADDRESS`. This will start a local server which is -by default on port `8100`. This local server would now behave similar to `MY_SERVER_ADDRESS`. +by default on port `8100`. This local server would now behave similar to `MY_SERVER_ADDRESS`. You can now Use `http://localhost:8100/fixed` wherever you were running into CORS issues with before, instead of `MY_SERVER_ADDRESS`. - diff --git a/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx b/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx index 99784d5adbd..c6d0b669482 100644 --- a/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx +++ b/webui/react/src/pages/WorkspaceList/WorkspaceActionDropdown.tsx @@ -156,7 +156,7 @@ export const useWorkspaceActionMenu: (props: WorkspaceMenuPropsIn) => WorkspaceM } else if (!workspace.archived) { menuItems.push({ key: MenuKey.Edit, label: 'View Config' }); } - if (canDeleteWorkspace({ workspace }) && workspace.numExperiments === 0) { + if (canDeleteWorkspace({ workspace })) { menuItems.push({ type: 'divider' }); menuItems.push({ danger: true, key: MenuKey.Delete, label: 'Delete...' }); } diff --git a/webui/react/typings/dayjs.d.ts b/webui/react/typings/dayjs.d.ts index f35ab0a9772..ec30292005e 100644 --- a/webui/react/typings/dayjs.d.ts +++ b/webui/react/typings/dayjs.d.ts @@ -1,8 +1,8 @@ declare module 'moment' { import { Dayjs } from 'dayjs'; namespace moment { - type Moment = Dayjs + type Moment = Dayjs; } - export = moment - export as namespace moment + export = moment; + export as namespace moment; } diff --git a/webui/react/typings/notebook.d.ts b/webui/react/typings/notebook.d.ts index df03185fd0f..6cf06c120af 100644 --- a/webui/react/typings/notebook.d.ts +++ b/webui/react/typings/notebook.d.ts @@ -1,3 +1,3 @@ declare module 'notebook' { - export default any; + export default any; }