fix: allow workspaces to be attempted to be deleted when experiments had existed historically [CM-412]

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)
 
 

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()

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)
 
 

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)
 

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)

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)

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{}

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) {

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`.
-

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...' });
     }

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;
 }

webui/react/typings/notebook.d.ts

@@ -1,3 +1,3 @@
 declare module 'notebook' {
-    export default any;
+  export default any;
 }