Skip to content

Commit

Permalink
Merge branch 'main' into f/model-serving
Browse files Browse the repository at this point in the history
  • Loading branch information
lucferbux committed Oct 19, 2023
2 parents 4f62765 + c0dca01 commit 5c15171
Show file tree
Hide file tree
Showing 11 changed files with 184 additions and 59 deletions.
12 changes: 12 additions & 0 deletions backend/src/routes/api/dsc/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { KubeFastifyInstance } from '../../../types';
import { secureRoute } from '../../../utils/route-security';
import { getClusterStatus } from '../../../utils/dsc';

module.exports = async (fastify: KubeFastifyInstance) => {
fastify.get(
'/status',
secureRoute(fastify)(async () => {
return getClusterStatus(fastify);
}),
);
};
79 changes: 41 additions & 38 deletions backend/src/routes/api/groups-config/groupsConfigUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,12 @@ const SYSTEM_AUTHENTICATED = 'system:authenticated';
export const getGroupsConfig = async (fastify: KubeFastifyInstance): Promise<GroupsConfig> => {
const customObjectsApi = fastify.kube.customObjectsApi;

try {
const groupsCluster = await getAllGroups(customObjectsApi);
const groupsData = getGroupsCR();
const groupsProcessed = processGroupData(groupsData);
const groupsConfigProcessed = processGroupConfig(groupsProcessed, groupsCluster);
await removeDeletedGroups(fastify, groupsData, groupsConfigProcessed.groupsCRData);

return groupsConfigProcessed.groupsConfig;
} catch (e) {
fastify.log.error(e, 'Error retrieving group configuration.');
const error = createError(500, 'Error retrieving group configuration');
throw error;
}
const groupsCluster = await getAllGroups(customObjectsApi);
const groupsData = getGroupsCR();
const groupsProcessed = processGroupData(groupsData);
const groupsConfigProcessed = processGroupConfig(fastify, groupsProcessed, groupsCluster);
await removeDeletedGroups(fastify, groupsData, groupsConfigProcessed.groupsCRData);
return groupsConfigProcessed.groupsConfig;
};

const transformGroupsConfig = (groupStatus: GroupStatus[]): string[] => {
Expand All @@ -38,7 +31,7 @@ const transformGroupsConfig = (groupStatus: GroupStatus[]): string[] => {
export const updateGroupsConfig = async (
fastify: KubeFastifyInstance,
request: FastifyRequest<{ Body: GroupsConfig }>,
): Promise<{ success: GroupsConfig | null; error: string | null }> => {
): Promise<GroupsConfig> => {
const customObjectsApi = fastify.kube.customObjectsApi;
const { namespace } = fastify.kube;

Expand All @@ -58,26 +51,17 @@ export const updateGroupsConfig = async (
const error = createError(403, 'Error, groups cannot be empty');
throw error;
}
try {
const dataUpdated: GroupsConfigBody = {
adminGroups: adminConfig.join(','),
allowedGroups: allowedConfig.join(','),
};

const groupsData = await updateGroupsCR(fastify, dataUpdated);
const groupsProcessed = processGroupData(groupsData);
const groupsCluster = await getAllGroups(customObjectsApi);
const updatedConfig = processGroupConfig(groupsProcessed, groupsCluster);
await removeDeletedGroups(fastify, groupsData, updatedConfig.groupsCRData);
return {
success: updatedConfig.groupsConfig,
error: null,
};
} catch (e) {
fastify.log.error(e, 'Error updating group configuration.');
const error = createError(500, 'Error updating group configuration');
throw error;
}
const dataUpdated: GroupsConfigBody = {
adminGroups: adminConfig.join(','),
allowedGroups: allowedConfig.join(','),
};

const groupsData = await updateGroupsCR(fastify, dataUpdated);
const groupsProcessed = processGroupData(groupsData);
const groupsCluster = await getAllGroups(customObjectsApi);
const updatedConfig = processGroupConfig(fastify, groupsProcessed, groupsCluster);
await removeDeletedGroups(fastify, groupsData, updatedConfig.groupsCRData);
return updatedConfig.groupsConfig;
};

const processGroupData = (groupsData: GroupsConfigBody): GroupsConfigBodyList => {
Expand Down Expand Up @@ -105,6 +89,7 @@ const mapListToGroupStatus =
* @returns Processed object with the groups, removing missing groups that might be selected
*/
const processGroupConfig = (
fastify: KubeFastifyInstance,
groupsDataList: GroupsConfigBodyList,
groups: string[],
): { groupsConfig: GroupsConfig; groupsCRData: GroupsConfigBody } => {
Expand All @@ -120,9 +105,14 @@ const processGroupConfig = (
const groupsConfig: GroupsConfig = {
adminGroups: adminGroupsConfig,
allowedGroups: allowedGroupsConfig,
errorAdmin: getError(groupsDataList.adminGroups, (group) => !groups.includes(group)),
errorAdmin: getError(
fastify,
groupsDataList.adminGroups.filter((group) => group),
(group) => !groups.includes(group),
),
errorUser: getError(
groupsDataList.allowedGroups,
fastify,
groupsDataList.allowedGroups.filter((group) => group),
(group) => !groups.includes(group) && group !== SYSTEM_AUTHENTICATED,
),
};
Expand All @@ -137,13 +127,26 @@ const processGroupConfig = (
return { groupsConfig, groupsCRData: updatedBody };
};

const getError = (array: string[], predicate: (group: string) => boolean): string | undefined => {
const getError = (
fastify: KubeFastifyInstance,
array: string[],
predicate: (group: string) => boolean,
): string | undefined => {
let error;
if (array.length === 0) {
error = 'No group is set in the group config, please set one or more group.';
fastify.log.error(error);
return error;
}

const missingItems = array.filter(predicate);
if (missingItems.length === 0) return undefined;

return `The group${missingItems.length === 1 ? '' : 's'} ${missingItems.join(
error = `The group${missingItems.length === 1 ? '' : 's'} ${missingItems.join(
', ',
)} no longer exists in OpenShift and has been removed from the selected group list.`;
fastify.log.error(error);
return error;
};

/**
Expand Down
9 changes: 7 additions & 2 deletions backend/src/routes/api/groups-config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@ import { secureAdminRoute } from '../../../utils/route-security';
export default async (fastify: FastifyInstance): Promise<void> => {
fastify.get(
'/',
secureAdminRoute(fastify)(async () => {
return getGroupsConfig(fastify);
secureAdminRoute(fastify)(async (request, reply) => {
return getGroupsConfig(fastify).catch((e) => {
fastify.log.error(
`Error retrieving group configuration, ${e.response?.body?.message || e.message}`,
);
reply.status(500).send({ message: e.response?.body?.message || e.message });
});
}),
);

Expand Down
36 changes: 30 additions & 6 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,6 @@ export type KubeDecorator = KubeStatus & {
customObjectsApi: k8s.CustomObjectsApi;
rbac: k8s.RbacAuthorizationV1Api;
currentToken: string;

};

export type KubeFastifyInstance = FastifyInstance & {
Expand Down Expand Up @@ -761,10 +760,10 @@ export type GPUInfo = {

export type AcceleratorInfo = {
configured: boolean;
available: {[key: string]: number};
total: {[key: string]: number};
allocated: {[key: string]: number};
}
available: { [key: string]: number };
total: { [key: string]: number };
allocated: { [key: string]: number };
};

export type EnvironmentVariable = EitherNotBoth<
{ value: string | number },
Expand Down Expand Up @@ -884,7 +883,6 @@ export type SupportedModelFormats = {
autoSelect?: boolean;
};


export enum ContainerResourceAttributes {
CPU = 'cpu',
MEMORY = 'memory',
Expand Down Expand Up @@ -949,3 +947,29 @@ export enum KnownLabels {
MODEL_SERVING_PROJECT = 'modelmesh-enabled',
DATA_CONNECTION_AWS = 'opendatahub.io/managed',
}

type ComponentNames =
| 'codeflare'
| 'data-science-pipelines-operator'
| 'kserve'
| 'model-mesh'
// Bug: https://github.com/opendatahub-io/opendatahub-operator/issues/641
| 'odh-dashboard'
| 'ray'
| 'workbenches';

export type DataScienceClusterKindStatus = {
conditions: [];
installedComponents: { [key in ComponentNames]: boolean };
phase?: string;
};

export type DataScienceClusterKind = K8sResourceCommon & {
spec: unknown; // we should never need to look into this
status: DataScienceClusterKindStatus;
};

export type DataScienceClusterList = {
kind: 'DataScienceClusterList';
items: DataScienceClusterKind[];
};
26 changes: 26 additions & 0 deletions backend/src/utils/dsc.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import {
DataScienceClusterKind,
DataScienceClusterKindStatus,
DataScienceClusterList,
KubeFastifyInstance,
} from '../types';
import { createCustomError } from './requestUtils';

export const getClusterStatus = async (
fastify: KubeFastifyInstance,
): Promise<DataScienceClusterKindStatus> => {
const result: DataScienceClusterKind | null = await fastify.kube.customObjectsApi
.listClusterCustomObject('datasciencecluster.opendatahub.io', 'v1', 'datascienceclusters')
.then((res) => (res.body as DataScienceClusterList).items[0])
.catch((e) => {
fastify.log.error(`Failure to fetch dsc: ${e.response.body}`);
return null;
});

if (!result) {
// May not be using v2 Operator
throw createCustomError('DSC Unavailable', 'Unable to get status', 404);
}

return result.status;
};
2 changes: 1 addition & 1 deletion backend/src/utils/groupsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class MissingGroupError extends Error {
}

export const getGroupsCR = (): GroupsConfigBody => {
if (typeof getDashboardConfig().spec.groupsConfig !== 'undefined') {
if (getDashboardConfig().spec.groupsConfig) {
return getDashboardConfig().spec.groupsConfig;
}
throw new Error(`Failed to retrieve Dashboard CR groups configuration`);
Expand Down
17 changes: 17 additions & 0 deletions frontend/src/k8sTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -770,3 +770,20 @@ export type K8sResourceListResult<TResource extends Partial<K8sResourceCommon>>
continue: string;
};
};

type ComponentNames =
| 'codeflare'
| 'data-science-pipelines-operator'
| 'kserve'
| 'model-mesh'
// Bug: https://github.com/opendatahub-io/opendatahub-operator/issues/641
| 'odh-dashboard'
| 'ray'
| 'workbenches';

/** We don't need or should ever get the full kind, this is the status section */
export type DataScienceClusterKindStatus = {
conditions: K8sCondition[];
installedComponents: { [key in ComponentNames]: boolean };
phase?: string;
};
4 changes: 1 addition & 3 deletions frontend/src/services/groupSettingsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ export const fetchGroupsSettings = (): Promise<GroupsConfig> => {
});
};

export const updateGroupsSettings = (
settings: GroupsConfig,
): Promise<{ success: GroupsConfig | null; error: string | null }> => {
export const updateGroupsSettings = (settings: GroupsConfig): Promise<GroupsConfig> => {
const url = '/api/groups-config';
return axios
.put(url, settings)
Expand Down
34 changes: 34 additions & 0 deletions frontend/src/utilities/__tests__/useWatchGroups.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { testHook } from '~/__tests__/unit/testUtils/hooks';
import { fetchGroupsSettings } from '~/services/groupSettingsService';
import { useWatchGroups } from '~/utilities/useWatchGroups';

jest.mock('~/services/groupSettingsService', () => ({
fetchGroupsSettings: jest.fn(),
}));

jest.mock('react-redux', () => ({
useDispatch: jest.fn(),
}));

const fetchGroupSettingsMock = fetchGroupsSettings as jest.Mock;

describe('useWatchGroups', () => {
it('should fetch groups successfully', async () => {
const mockEmptyGroupSettings = {
adminGroups: [],
allowedGroups: [],
};
const mockGroupSettings = {
adminGroups: ['odh-admins'],
allowedGroups: [],
};
fetchGroupSettingsMock.mockReturnValue(Promise.resolve(mockGroupSettings));

const renderResult = testHook(useWatchGroups)();
expect(fetchGroupSettingsMock).toHaveBeenCalledTimes(1);
expect(renderResult.result.current.groupSettings).toStrictEqual(mockEmptyGroupSettings);

await renderResult.waitForNextUpdate();
expect(renderResult.result.current.groupSettings).toStrictEqual(mockGroupSettings);
});
});
16 changes: 7 additions & 9 deletions frontend/src/utilities/useWatchGroups.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,24 +48,22 @@ export const useWatchGroups = (): {

React.useEffect(() => {
if (errorAdmin) {
notification.error(`Group no longer exists`, errorAdmin);
notification.error(`Group error`, errorAdmin);
}
if (errorUser) {
notification.error(`Group no longer exists`, errorUser);
notification.error(`Group error`, errorUser);
}
}, [errorAdmin, errorUser, notification]);

const updateGroups = (group: GroupsConfig) => {
setIsLoading(true);
updateGroupsSettings(group)
.then((response) => {
if (response.success) {
setGroupSettings(response.success);
notification.success(
'Group settings changes saved',
'It may take up to 2 minutes for configuration changes to be applied.',
);
}
setGroupSettings(response);
notification.success(
'Group settings changes saved',
'It may take up to 2 minutes for configuration changes to be applied.',
);
})
.catch((error) => {
setLoadError(error);
Expand Down
8 changes: 8 additions & 0 deletions manifests/base/cluster-role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -164,3 +164,11 @@ rules:
- delete
resources:
- notebooks
- apiGroups:
- datasciencecluster.opendatahub.io
verbs:
- list
- watch
- get
resources:
- datascienceclusters

0 comments on commit 5c15171

Please sign in to comment.