Skip to content

Commit

Permalink
Fix started notebooks disappear issue
Browse files Browse the repository at this point in the history
  • Loading branch information
ppadti committed Jul 12, 2024
1 parent 8a4261f commit 7b4bc5e
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 10 deletions.
3 changes: 2 additions & 1 deletion backend/src/routes/api/status/adminAllowedUsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ const getUserActivityFromNotebook = async (
.map<[string | undefined, string | undefined]>((notebook) => [
notebook.metadata.annotations?.['opendatahub.io/username'],
notebook.metadata.annotations?.['notebooks.kubeflow.org/last-activity'] ||
notebook.metadata.annotations?.['kubeflow-resource-stopped'],
notebook.metadata.annotations?.['kubeflow-resource-stopped'] ||
'Now',
])
.filter(
(arr): arr is [string, string] =>
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/__mocks__/mockAllowedUsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ import { AllowedUser, PrivilegeState } from '~/pages/notebookController/screens/
type MockAllowedUsersType = {
username?: string;
privilege?: PrivilegeState;
lastActivity?: string;
};
export const mockAllowedUsers = ({
username = 'test-user',
privilege = PrivilegeState.USER,
lastActivity = '2024-02-14T14:22:05Z',
}: MockAllowedUsersType): AllowedUser => ({
username,
privilege,
lastActivity: '2024-02-14T14:22:05Z',
lastActivity,
});
12 changes: 12 additions & 0 deletions frontend/src/__tests__/cypress/cypress/pages/administration.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Modal } from './components/Modal';
import { TableRow } from './components/table';

class NotebookController {
Expand Down Expand Up @@ -95,5 +96,16 @@ class AdministrationUsersRow extends TableRow {
}
}

class StopNotebookModal extends Modal {
constructor() {
super('Stop server modal Stop server');
}

findStopNotebookServerButton() {
return this.find().findByTestId('stop-nb-server-button');
}
}

export const administration = new AdministrationTab();
export const notebookController = new NotebookController();
export const stopNotebookModal = new StopNotebookModal();
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import { mockRoleBindingK8sResource } from '~/__mocks__/mockRoleBindingK8sResource';
import { mockK8sResourceList } from '~/__mocks__';
import { mockK8sResourceList, mockNotebookK8sResource } from '~/__mocks__';
import type { RoleBindingSubject } from '~/types';
import { mockAllowedUsers } from '~/__mocks__/mockAllowedUsers';
import { mockNotebookImageInfo } from '~/__mocks__/mockNotebookImageInfo';
import {
administration,
notebookController,
stopNotebookModal,
} from '~/__tests__/cypress/cypress/pages/administration';
import { be } from '~/__tests__/cypress/cypress/utils/should';
import { asProductAdminUser, asProjectEditUser } from '~/__tests__/cypress/cypress/utils/users';
import type { AllowedUser } from '~/pages/notebookController/screens/admin/types';
import { testPagination } from '~/__tests__/cypress/cypress/utils/pagination';
import { mockStartNotebookData } from '~/__mocks__/mockStartNotebookData';

const groupSubjects: RoleBindingSubject[] = [
{
Expand All @@ -21,13 +23,13 @@ const groupSubjects: RoleBindingSubject[] = [
];

type HandlersProps = {
allowedUser?: AllowedUser[];
allowedUsers?: AllowedUser[];
};

const initIntercepts = ({
allowedUser = [mockAllowedUsers({}), mockAllowedUsers({ username: 'regularuser1' })],
allowedUsers = [mockAllowedUsers({}), mockAllowedUsers({ username: 'regularuser1' })],
}: HandlersProps) => {
cy.interceptOdh('GET /api/status/openshift-ai-notebooks/allowedUsers', allowedUser);
cy.interceptOdh('GET /api/status/openshift-ai-notebooks/allowedUsers', allowedUsers);
cy.interceptOdh(
'GET /api/rolebindings/opendatahub/openshift-ai-notebooks-image-pullers',
mockK8sResourceList([
Expand Down Expand Up @@ -103,7 +105,7 @@ describe('Administration Tab', () => {
username: `Test user-${i}`,
}),
);
initIntercepts({ allowedUser: mockAllowedUser });
initIntercepts({ allowedUsers: mockAllowedUser });
notebookController.visit();
notebookController.findAdministrationTab().click();
// top pagination
Expand All @@ -113,6 +115,38 @@ describe('Administration Tab', () => {
testPagination({ totalItems, firstElement: 'Test user-0', paginationVariant: 'bottom' });
});

it('Validate that last activity will be "Just now" and user can stop server from the table, when notebook lacks the last-activity annotation', () => {
const allowedUsers = [
mockAllowedUsers({}),
mockAllowedUsers({ username: 'regularuser1', lastActivity: 'Now' }),
];
initIntercepts({ allowedUsers });
cy.interceptOdh(
'GET /api/notebooks/openshift-ai-notebooks/:username/status',
{ path: { username: 'jupyter-nb-regularuser1' } },
{
notebook: mockNotebookK8sResource({ image: 'code-server-notebook:2023.2' }),
isRunning: true,
},
);
cy.interceptOdh('PATCH /api/notebooks', mockStartNotebookData({})).as('stopNotebookServer');
notebookController.visit();
notebookController.findAdministrationTab().click();

const userRow = administration.getRow('regularuser1');
userRow.shouldHavePrivilege('User');
userRow.shouldHaveLastActivity('Just now');
userRow.findServerStatusButton().should('have.text', 'View server');
userRow.findKebabAction('Stop server').click();

stopNotebookModal.findStopNotebookServerButton().should('be.enabled');
stopNotebookModal.findStopNotebookServerButton().click();

cy.wait('@stopNotebookServer').then((interception) => {
expect(interception.request.body).to.eql({ state: 'stopped', username: 'test-user' });
});
});

it('Validate that clicking on "Start server" button will open a form in administartion tab and "Start your server" button will navigate to notebook server tab', () => {
initIntercepts({});
notebookController.visit();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { mockNotebookImageInfo } from '~/__mocks__/mockNotebookImageInfo';
import { mockStartNotebookData } from '~/__mocks__/mockStartNotebookData';
import { notebookServer } from '~/__tests__/cypress/cypress/pages/notebookServer';
import { asProductAdminUser, asProjectEditUser } from '~/__tests__/cypress/cypress/utils/users';
import { notebookController } from '~/__tests__/cypress/cypress/pages/administration';
import {
notebookController,
stopNotebookModal,
} from '~/__tests__/cypress/cypress/pages/administration';
import { homePage } from '~/__tests__/cypress/cypress/pages/home/home';

const groupSubjects: RoleBindingSubject[] = [
Expand Down Expand Up @@ -87,8 +90,9 @@ describe('NotebookServer', () => {
isRunning: false,
},
);
notebookServer.findStopNotebookServerButton().should('be.visible');
notebookServer.findStopNotebookServerButton().click();

stopNotebookModal.findStopNotebookServerButton().should('be.enabled');
stopNotebookModal.findStopNotebookServerButton().click();

cy.wait('@stopNotebookServer').then((interception) => {
expect(interception.request.body).to.eql({ state: 'stopped', username: 'test-user' });
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { testHook } from '~/__tests__/unit/testUtils/hooks';
import { getAllowedUsers } from '~/redux/actions/actions';
import { mockAllowedUsers } from '~/__mocks__/mockAllowedUsers';
import useCheckForAllowedUsers from '~/pages/notebookController/screens/admin/useCheckForAllowedUsers';
import { AllowedUser } from '~/pages/notebookController/screens/admin/types';

jest.mock('~/redux/actions/actions', () => ({
getAllowedUsers: jest.fn(),
}));

jest.mock('~/pages/notebookController/useNamespaces', () => () => ({
notebookNamespace: 'test-project',
dashboardNamespace: 'opendatahub',
}));

const getAllowedUsersMock = jest.mocked(getAllowedUsers);

describe('useCheckForAllowedUsers', () => {
it('should return list of users', async () => {
const mockAllowedUser: AllowedUser = mockAllowedUsers({});
getAllowedUsersMock.mockResolvedValue([mockAllowedUser]);
const renderResult = testHook(useCheckForAllowedUsers)();

expect(getAllowedUsersMock).toHaveBeenCalledTimes(1);
expect(renderResult).hookToStrictEqual([[], false, undefined]);
expect(renderResult).hookToHaveUpdateCount(1);

await renderResult.waitForNextUpdate();
expect(getAllowedUsersMock).toHaveBeenCalledTimes(1);
expect(renderResult).hookToStrictEqual([[mockAllowedUser], true, undefined]);
expect(renderResult).hookToHaveUpdateCount(2);
});

it('should handle error', async () => {
const error = (message: string) => ({
response: {
data: {
message,
},
},
});
getAllowedUsersMock.mockRejectedValue(error('error1'));
const renderResult = testHook(useCheckForAllowedUsers)();

expect(getAllowedUsersMock).toHaveBeenCalledTimes(1);
expect(renderResult).hookToStrictEqual([[], false, undefined]);
expect(renderResult).hookToHaveUpdateCount(1);

await renderResult.waitForNextUpdate();
expect(getAllowedUsersMock).toHaveBeenCalledTimes(1);
expect(renderResult).hookToStrictEqual([[], false, new Error('error1')]);
expect(renderResult).hookToHaveUpdateCount(2);
});
});

0 comments on commit 7b4bc5e

Please sign in to comment.