From 1ebf38b7d0cc53535f5bb0f130d76d91841d329f Mon Sep 17 00:00:00 2001 From: Oleksii Orel Date: Tue, 17 Sep 2024 15:18:14 +0300 Subject: [PATCH] fix: remove conditions before restart a workspace Signed-off-by: Oleksii Orel --- .../WorkspaceProgress/__tests__/utils.spec.ts | 119 ++++++++++++++++++ .../components/WorkspaceProgress/index.tsx | 8 +- .../src/components/WorkspaceProgress/utils.ts | 20 +++ .../devWorkspaces/__tests__/actions.spec.ts | 69 ++++++++++ .../store/Workspaces/devWorkspaces/index.ts | 9 +- 5 files changed, 221 insertions(+), 4 deletions(-) create mode 100644 packages/dashboard-frontend/src/components/WorkspaceProgress/__tests__/utils.spec.ts diff --git a/packages/dashboard-frontend/src/components/WorkspaceProgress/__tests__/utils.spec.ts b/packages/dashboard-frontend/src/components/WorkspaceProgress/__tests__/utils.spec.ts new file mode 100644 index 000000000..73ddf02a1 --- /dev/null +++ b/packages/dashboard-frontend/src/components/WorkspaceProgress/__tests__/utils.spec.ts @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2018-2024 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ + +import { getStartWorkspaceConditions } from '@/components/WorkspaceProgress/utils'; +import { DevWorkspaceBuilder } from '@/store/__mocks__/devWorkspaceBuilder'; + +describe('WorkspaceProgress utils', () => { + beforeEach(() => {}); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('getStartWorkspaceCondition', () => { + it('should return an empty array as a default value', () => { + const devWorkspace = new DevWorkspaceBuilder().build(); + + expect(devWorkspace.status?.conditions).toBeUndefined(); + + const conditions = getStartWorkspaceConditions(devWorkspace); + + expect(conditions).toEqual([]); + }); + it('should return conditions from the devWorkspace', () => { + const status = { + conditions: [ + { + type: 'Stopped', + status: 'False', + reason: 'LimitReached', + message: 'Workspace stopped due to error.', + }, + ], + }; + const devWorkspace = new DevWorkspaceBuilder().withStatus(status).build(); + + const conditions = getStartWorkspaceConditions(devWorkspace); + + expect(conditions).toEqual(status.conditions); + }); + }); + it('should filter conditions that are not related to the workspace start', () => { + const status = { + conditions: [ + { + message: 'DevWorkspace is starting', + status: 'True', + type: 'Started', + }, + { + message: 'Resolved plugins and parents from DevWorkspace', + status: 'True', + type: 'DevWorkspaceResolved', + }, + { + message: 'Storage ready', + status: 'True', + type: 'StorageReady', + }, + { + message: 'Networking ready', + status: 'True', + type: 'RoutingReady', + }, + { + message: 'DevWorkspace serviceaccount ready', + status: 'True', + type: 'ServiceAccountReady', + }, + { + message: 'Waiting for workspace deployment', + status: 'False', + type: 'DeploymentReady', + }, + ], + }; + const devWorkspace = new DevWorkspaceBuilder().withStatus(status).build(); + + const conditions = getStartWorkspaceConditions(devWorkspace); + + expect(conditions).not.toEqual(status.conditions); + expect(conditions).toEqual([ + { + message: 'DevWorkspace is starting', + status: 'True', + type: 'Started', + }, + { + message: 'Resolved plugins and parents from DevWorkspace', + status: 'True', + type: 'DevWorkspaceResolved', + }, + { + message: 'Storage ready', + status: 'True', + type: 'StorageReady', + }, + { + message: 'Networking ready', + status: 'True', + type: 'RoutingReady', + }, + { + message: 'DevWorkspace serviceaccount ready', + status: 'True', + type: 'ServiceAccountReady', + }, + ]); + }); +}); diff --git a/packages/dashboard-frontend/src/components/WorkspaceProgress/index.tsx b/packages/dashboard-frontend/src/components/WorkspaceProgress/index.tsx index 12ebf070f..57bd1d178 100644 --- a/packages/dashboard-frontend/src/components/WorkspaceProgress/index.tsx +++ b/packages/dashboard-frontend/src/components/WorkspaceProgress/index.tsx @@ -30,7 +30,11 @@ import StartingStepInitialize from '@/components/WorkspaceProgress/StartingSteps import StartingStepOpenWorkspace from '@/components/WorkspaceProgress/StartingSteps/OpenWorkspace'; import StartingStepStartWorkspace from '@/components/WorkspaceProgress/StartingSteps/StartWorkspace'; import StartingStepWorkspaceConditions from '@/components/WorkspaceProgress/StartingSteps/WorkspaceConditions'; -import { ConditionType, isWorkspaceStatusCondition } from '@/components/WorkspaceProgress/utils'; +import { + ConditionType, + getStartWorkspaceConditions, + isWorkspaceStatusCondition, +} from '@/components/WorkspaceProgress/utils'; import WorkspaceProgressWizard, { WorkspaceProgressWizardStep, } from '@/components/WorkspaceProgress/Wizard'; @@ -184,7 +188,7 @@ class Progress extends React.Component { workspace.status === DevWorkspaceStatus.FAILING || workspace.status === DevWorkspaceStatus.FAILED) ) { - const conditions = workspace.ref.status?.conditions || []; + const conditions = getStartWorkspaceConditions(workspace.ref); const lastScore = this.scoreConditions(this.state.conditions); const score = this.scoreConditions(conditions); diff --git a/packages/dashboard-frontend/src/components/WorkspaceProgress/utils.ts b/packages/dashboard-frontend/src/components/WorkspaceProgress/utils.ts index 8c13b5d77..a085db491 100644 --- a/packages/dashboard-frontend/src/components/WorkspaceProgress/utils.ts +++ b/packages/dashboard-frontend/src/components/WorkspaceProgress/utils.ts @@ -12,6 +12,8 @@ import { V1alpha2DevWorkspaceStatusConditions } from '@devfile/api'; +import devfileApi from '@/services/devfileApi'; + export type ConditionType = V1alpha2DevWorkspaceStatusConditions & { status: 'True' | 'False' | 'Unknown'; }; @@ -37,6 +39,24 @@ export function isConditionReady( ); } +export function getStartWorkspaceConditions( + workspace: devfileApi.DevWorkspace, +): V1alpha2DevWorkspaceStatusConditions[] { + if (!workspace.status?.conditions || workspace.status.conditions.length === 0) { + return []; + } + const conditions = [...workspace.status.conditions]; + // remove all conditions that are not related to the workspace start + for (let i = conditions.length; i > 0; i--) { + if (conditions[i - 1].type === 'ServiceAccountReady') { + conditions.length = i; + break; + } + } + + return conditions; +} + export function isConditionError( condition: ConditionType, prevCondition: ConditionType | undefined, diff --git a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/__tests__/actions.spec.ts b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/__tests__/actions.spec.ts index d3a16aeee..446b2c0df 100644 --- a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/__tests__/actions.spec.ts +++ b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/__tests__/actions.spec.ts @@ -447,6 +447,75 @@ describe('DevWorkspace store, actions', () => { expect(actions).toStrictEqual(expectedActions); }); + it('should remove all workspace conditions in the store before start a DevWorkspace', async () => { + (checkRunningWorkspacesLimit as jest.Mock).mockImplementation(() => undefined); + (isRunningDevWorkspacesClusterLimitExceeded as jest.Mock).mockReturnValue( + Promise.resolve(true), + ); + + // set a condition with an error message to the devWorkspace + devWorkspace.status = { + devworkspaceId: '1234', + conditions: [ + { + type: 'Stopped', + status: 'False', + reason: 'LimitReached', + message: 'Workspace stopped due to error.', + }, + ], + }; + + const store = storeBuilder.withDevWorkspaces({ workspaces: [devWorkspace] }).build(); + + await store.dispatch(testStore.actionCreators.startWorkspace(devWorkspace)); + + const actions = store.getActions(); + + const expectedDevWorkspaceWithEmptyConditions = Object.assign({}, devWorkspace, { + status: { + devworkspaceId: '1234', + conditions: [], + }, + }); + + const expectedActions: Array< + | testStore.KnownAction + | testDevWorkspaceClusterStore.KnownAction + | ServerConfigStore.KnownAction + > = [ + { + type: testStore.Type.UPDATE_DEVWORKSPACE, + workspace: expectedDevWorkspaceWithEmptyConditions, + }, + { + type: testDevWorkspaceClusterStore.Type.REQUEST_DEVWORKSPACES_CLUSTER, + check: AUTHORIZED, + }, + { + type: testDevWorkspaceClusterStore.Type.RECEIVED_DEVWORKSPACES_CLUSTER, + isRunningDevWorkspacesClusterLimitExceeded: true, + }, + { + type: testStore.Type.REQUEST_DEVWORKSPACE, + check: AUTHORIZED, + }, + { + type: 'REQUEST_DW_SERVER_CONFIG', + }, + { + config: {} as api.IServerConfig, + type: 'RECEIVE_DW_SERVER_CONFIG', + }, + { + type: testStore.Type.UPDATE_DEVWORKSPACE, + workspace: devWorkspace, + }, + ]; + + expect(actions).toStrictEqual(expectedActions); + }); + it('should create REQUEST_DEVWORKSPACE and RECEIVE_DEVWORKSPACE_ERROR when failed to start a DevWorkspace', async () => { (checkRunningWorkspacesLimit as jest.Mock).mockImplementation(() => { throw new Error('Limit reached.'); diff --git a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/index.ts b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/index.ts index 341ca237a..d42c53470 100644 --- a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/index.ts +++ b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/index.ts @@ -310,6 +310,13 @@ export const actionCreators: ActionCreators = { console.warn(`Workspace ${_workspace.metadata.name} already started`); return; } + if (workspace.status?.conditions && workspace.status?.conditions?.length > 0) { + workspace.status.conditions = []; + dispatch({ + type: Type.UPDATE_DEVWORKSPACE, + workspace, + }); + } try { await OAuthService.refreshTokenIfProjectExists(workspace); await dispatch( @@ -390,8 +397,6 @@ export const actionCreators: ActionCreators = { const startingTimeout = 10000; await Promise.race([defer.promise, delay(startingTimeout)]); toDispose.dispose(); - - getDevWorkspaceClient().checkForDevWorkspaceError(startingWorkspace); } catch (e) { // Skip unauthorised errors. The page is redirecting to an SCM authentication page. if (common.helpers.errors.includesAxiosResponse(e) && isOAuthResponse(e.response.data)) {