Skip to content

Commit

Permalink
Data connection clean up
Browse files Browse the repository at this point in the history
  • Loading branch information
@ashley-o0o
ashley-o0o committed Jan 21, 2025
1 parent b368894 commit 4944255
Show file tree
Hide file tree
Showing 11 changed files with 31 additions and 128 deletions.
11 changes: 3 additions & 8 deletions backend/src/routes/api/connection-types/connectionTypeUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ import { getNamespaces } from '../../../utils/notebookUtils';
import { errorHandler } from '../../../utils';

const isConnectionTypeConfigMap = (configMap: V1ConfigMap): boolean =>
configMap.metadata.labels &&
configMap.metadata.labels[KnownLabels.DASHBOARD_RESOURCE] === 'true' &&
configMap.metadata.labels[KnownLabels.CONNECTION_TYPE] === 'true';
configMap.metadata.labels && configMap.metadata.labels[KnownLabels.DASHBOARD_RESOURCE] === 'true';

const isExistingConnectionType = async (
fastify: KubeFastifyInstance,
Expand Down Expand Up @@ -34,7 +32,6 @@ export const listConnectionTypes = async (fastify: KubeFastifyInstance): Promise
undefined,
_continue,
undefined,
`${KnownLabels.DASHBOARD_RESOURCE} = true, ${KnownLabels.CONNECTION_TYPE} = true`,
);
connectionTypes.push(...(response?.body?.items ?? []));
remainingItemCount = response?.body.metadata?.remainingItemCount;
Expand Down Expand Up @@ -128,10 +125,8 @@ export const patchConnectionType = async (
const { dashboardNamespace } = getNamespaces(fastify);

if (
(partialConfigMap.metadata?.labels?.[KnownLabels.DASHBOARD_RESOURCE] &&
partialConfigMap.metadata.labels[KnownLabels.DASHBOARD_RESOURCE] !== 'true') ||
(partialConfigMap.metadata?.labels?.[KnownLabels.CONNECTION_TYPE] &&
partialConfigMap.metadata.labels[KnownLabels.CONNECTION_TYPE] !== 'true')
partialConfigMap.metadata?.labels?.[KnownLabels.DASHBOARD_RESOURCE] &&
partialConfigMap.metadata.labels[KnownLabels.DASHBOARD_RESOURCE] !== 'true'
) {
const error = 'Unable to update connection type, incorrect labels.';
fastify.log.error(error);
Expand Down
2 changes: 0 additions & 2 deletions backend/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ export type DashboardConfig = K8sResourceCommon & {
disableModelRegistry: boolean;
disableModelRegistrySecureDB: boolean;
disableServingRuntimeParams: boolean;
disableConnectionTypes: boolean;
disableStorageClasses: boolean;
disableNIMModelServing: boolean;
};
Expand Down Expand Up @@ -998,7 +997,6 @@ export enum KnownLabels {
PROJECT_SHARING = 'opendatahub.io/project-sharing',
MODEL_SERVING_PROJECT = 'modelmesh-enabled',
DATA_CONNECTION_AWS = 'opendatahub.io/managed',
CONNECTION_TYPE = 'opendatahub.io/connection-type',
LABEL_SELECTOR_MODEL_REGISTRY = 'component=model-registry',
}

Expand Down
1 change: 0 additions & 1 deletion backend/src/utils/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ export const blankDashboardCR: DashboardConfig = {
disableModelRegistry: false,
disableModelRegistrySecureDB: false,
disableServingRuntimeParams: false,
disableConnectionTypes: false,
disableStorageClasses: false,
disableNIMModelServing: true,
},
Expand Down
2 changes: 0 additions & 2 deletions frontend/src/__mocks__/mockDashboardConfig.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ export const mockDashboardConfig = ({
disableModelRegistry = false,
disableModelRegistrySecureDB = false,
disableServingRuntimeParams = false,
disableConnectionTypes = true,
disableStorageClasses = false,
disableNotebookController = false,
disableNIMModelServing = true,
Expand Down Expand Up @@ -173,7 +172,6 @@ export const mockDashboardConfig = ({
disableModelRegistry,
disableModelRegistrySecureDB,
disableServingRuntimeParams,
disableConnectionTypes,
disableStorageClasses,
disableNIMModelServing,
},
Expand Down
18 changes: 0 additions & 18 deletions frontend/src/__tests__/cypress/cypress/tests/mocked/connectionTypes/connectionTypes.cy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,6 @@ it('Connection types should not be available for non product admins', () => {
connectionTypesPage.findNavItem().should('not.exist');
});

it('Connection types should be hidden by feature flag', () => {
asProductAdminUser();

cy.visitWithLogin('/connectionTypes');
connectionTypesPage.shouldReturnNotFound();

cy.interceptOdh(
'GET /api/config',
mockDashboardConfig({
disableConnectionTypes: false,
}),
);

cy.interceptOdh('GET /api/connection-types', []);
connectionTypesPage.visit();
connectionTypesPage.shouldBeEmpty();
});

describe('Connection types', () => {
beforeEach(() => {
asProductAdminUser();
Expand Down
3 changes: 1 addition & 2 deletions frontend/src/concepts/areas/const.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export const allFeatureFlags: string[] = Object.keys({
disableModelRegistry: false,
disableModelRegistrySecureDB: false,
disableServingRuntimeParams: false,
disableConnectionTypes: false,
disableStorageClasses: false,
disableNIMModelServing: true,
} satisfies DashboardCommonConfig);
Expand All @@ -53,7 +52,7 @@ export const SupportedAreasStateMap: SupportedAreasState = {
reliantAreas: [SupportedArea.MODEL_SERVING],
},
[SupportedArea.CONNECTION_TYPES]: {
featureFlags: ['disableConnectionTypes'],
featureFlags: [], // TODO: Will need to disable
},
[SupportedArea.STORAGE_CLASSES]: {
featureFlags: ['disableStorageClasses'],
Expand Down
1 change: 0 additions & 1 deletion frontend/src/k8sTypes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,6 @@ export type DashboardCommonConfig = {
disableModelRegistry: boolean;
disableModelRegistrySecureDB: boolean;
disableServingRuntimeParams: boolean;
disableConnectionTypes: boolean;
disableStorageClasses: boolean;
disableNIMModelServing: boolean;
};
Expand Down
36 changes: 8 additions & 28 deletions ...pages/modelServing/screens/projects/InferenceServiceModal/ManageInferenceServiceModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import { AwsKeys } from '~/pages/projects/dataConnections/const';
import { getDisplayNameFromK8sResource } from '~/concepts/k8s/utils';
import { RegisteredModelDeployInfo } from '~/pages/modelRegistry/screens/RegisteredModels/useRegisteredModelDeployInfo';
import usePrefillDeployModalFromModelRegistry from '~/pages/modelRegistry/screens/RegisteredModels/usePrefillDeployModalFromModelRegistry';
import useConnectionTypesEnabled from '~/concepts/connectionTypes/useConnectionTypesEnabled';
import { Connection } from '~/concepts/connectionTypes/types';
import K8sNameDescriptionField, {
useK8sNameDescriptionFieldData,
Expand All @@ -27,7 +26,6 @@ import DataConnectionSection from './DataConnectionSection';
import ProjectSection from './ProjectSection';
import InferenceServiceFrameworkSection from './InferenceServiceFrameworkSection';
import InferenceServiceServingRuntimeSection from './InferenceServiceServingRuntimeSection';
import { ConnectionSection } from './ConnectionSection';

type ManageInferenceServiceModalProps = {
onClose: (submit: boolean) => void;
Expand Down Expand Up @@ -70,9 +68,7 @@ const ManageInferenceServiceModal: React.FC<ManageInferenceServiceModalProps> =
registeredModelDeployInfo,
);

const isConnectionTypesEnabled = useConnectionTypesEnabled();
const [connection, setConnection] = React.useState<Connection>();
const [isConnectionValid, setIsConnectionValid] = React.useState(false);
const [connection] = React.useState<Connection>();

const hasEditInfo = !!editInfo;
React.useEffect(() => {
Expand All @@ -92,17 +88,10 @@ const ManageInferenceServiceModal: React.FC<ManageInferenceServiceModalProps> =
return !!createData.storage.uri;
}
if (createData.storage.type === InferenceServiceStorageType.EXISTING_STORAGE) {
if (isConnectionTypesEnabled) {
return isConnectionValid;
}
return (
createData.storage.dataConnection !== '' && isConnectionPathValid(createData.storage.path)
);
}
// NEW_STORAGE
if (isConnectionTypesEnabled) {
return isConnectionValid;
}
return (
isAWSValid(createData.storage.awsData, [AwsKeys.AWS_S3_BUCKET]) &&
isConnectionPathValid(createData.storage.path)
Expand Down Expand Up @@ -210,22 +199,13 @@ const ManageInferenceServiceModal: React.FC<ManageInferenceServiceModalProps> =
registeredModelFormat={registeredModelDeployInfo?.modelFormat}
/>
<FormSection title="Source model location" id="model-location">
{isConnectionTypesEnabled ? (
<ConnectionSection
data={createData}
setData={setCreateData}
setConnection={setConnection}
setIsConnectionValid={setIsConnectionValid}
/>
) : (
<DataConnectionSection
data={createData}
setData={setCreateData}
loaded={!!projectContext?.dataConnections || dataConnectionsLoaded}
loadError={dataConnectionsLoadError}
dataConnections={dataConnections}
/>
)}
<DataConnectionSection
data={createData}
setData={setCreateData}
loaded={!!projectContext?.dataConnections || dataConnectionsLoaded}
loadError={dataConnectionsLoadError}
dataConnections={dataConnections}
/>
</FormSection>
</>
)}
Expand Down
35 changes: 8 additions & 27 deletions frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ import {
FormTrackingEventProperties,
TrackingOutcome,
} from '~/concepts/analyticsTracking/trackingProperties';
import useConnectionTypesEnabled from '~/concepts/connectionTypes/useConnectionTypesEnabled';
import { Connection } from '~/concepts/connectionTypes/types';
import { ConnectionSection } from '~/pages/modelServing/screens/projects/InferenceServiceModal/ConnectionSection';
import K8sNameDescriptionField, {
useK8sNameDescriptionFieldData,
} from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField';
Expand Down Expand Up @@ -118,9 +116,7 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
registeredModelDeployInfo,
);

const isConnectionTypesEnabled = useConnectionTypesEnabled();
const [connection, setConnection] = React.useState<Connection>();
const [isConnectionValid, setIsConnectionValid] = React.useState(false);
const [connection] = React.useState<Connection>();

const isAuthorinoEnabled = useIsAreaAvailable(SupportedArea.K_SERVE_AUTH).status;
const currentProjectName = projectContext?.currentProject.metadata.name;
Expand Down Expand Up @@ -181,18 +177,12 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
return !!createDataInferenceService.storage.uri;
}
if (createDataInferenceService.storage.type === InferenceServiceStorageType.EXISTING_STORAGE) {
if (isConnectionTypesEnabled) {
return isConnectionValid;
}
return (
createDataInferenceService.storage.dataConnection !== '' &&
isConnectionPathValid(createDataInferenceService.storage.path)
);
}
// NEW_STORAGE
if (isConnectionTypesEnabled) {
return isConnectionValid;
}
return (
isAWSValid(createDataInferenceService.storage.awsData, [AwsKeys.AWS_S3_BUCKET]) &&
isConnectionPathValid(createDataInferenceService.storage.path)
Expand Down Expand Up @@ -442,22 +432,13 @@ const ManageKServeModal: React.FC<ManageKServeModalProps> = ({
</FormSection>
{!hideForm && (
<FormSection title="Source model location" id="model-location">
{isConnectionTypesEnabled ? (
<ConnectionSection
data={createDataInferenceService}
setData={setCreateDataInferenceService}
setConnection={setConnection}
setIsConnectionValid={setIsConnectionValid}
/>
) : (
<DataConnectionSection
data={createDataInferenceService}
setData={setCreateDataInferenceService}
loaded={!!projectContext?.dataConnections || dataConnectionsLoaded}
loadError={dataConnectionsLoadError}
dataConnections={dataConnections}
/>
)}
<DataConnectionSection
data={createDataInferenceService}
setData={setCreateDataInferenceService}
loaded={!!projectContext?.dataConnections || dataConnectionsLoaded}
loadError={dataConnectionsLoadError}
dataConnections={dataConnections}
/>
</FormSection>
)}
{servingRuntimeParamsEnabled && (
Expand Down
16 changes: 3 additions & 13 deletions frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,12 @@ import {
} from './service';
import { checkRequiredFieldsForNotebookStart, getPvcVolumeDetails } from './spawnerUtils';
import { getNotebookDataConnection } from './dataConnection/useNotebookDataConnection';
import { setConnectionsOnEnvFrom } from './connections/utils';

type SpawnerFooterProps = {
startNotebookData: StartNotebookData;
storageData: StorageData[];
envVariables: EnvVariable[];
dataConnection: DataConnectionData;
isConnectionTypesEnabled?: boolean;
connections?: Connection[];
canEnablePipelines: boolean;
};
Expand All @@ -59,7 +57,6 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
storageData,
envVariables,
dataConnection,
isConnectionTypesEnabled,
connections = [],
canEnablePipelines,
}) => {
Expand All @@ -74,7 +71,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
const {
notebooks: { data: notebooks, refresh: refreshNotebooks },
dataConnections: { data: existingDataConnections, refresh: refreshDataConnections },
connections: { data: projectConnections, refresh: refreshConnections },
connections: { refresh: refreshConnections },
} = React.useContext(ProjectDetailsContext);
const { notebookName } = useParams();
const notebookState = notebooks.find(
Expand Down Expand Up @@ -184,7 +181,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({

await Promise.all(restartConnectedNotebooksPromises);

let envFrom = await updateConfigMapsAndSecretsForNotebook(
const envFrom = await updateConfigMapsAndSecretsForNotebook(
projectName,
editNotebook,
envVariables,
Expand All @@ -194,10 +191,6 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
dryRun,
);

if (isConnectionTypesEnabled) {
envFrom = setConnectionsOnEnvFrom(connections, envFrom, projectConnections);
}

const annotations = { ...editNotebook.metadata.annotations };
if (envFrom.length > 0) {
annotations['notebooks.opendatahub.io/notebook-restart'] = 'true';
Expand Down Expand Up @@ -249,16 +242,13 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({

await Promise.all(restartConnectedNotebooksPromises);

let envFrom = await createConfigMapsAndSecretsForNotebook(
const envFrom = await createConfigMapsAndSecretsForNotebook(
projectName,
[...envVariables, ...newDataConnection],
dryRun,
);

const { volumes, volumeMounts } = pvcVolumeDetails;
if (isConnectionTypesEnabled) {
envFrom = setConnectionsOnEnvFrom(connections, envFrom, projectConnections);
}
const newStartData: StartNotebookData = {
...startNotebookData,
volumes,
Expand Down
Loading

0 comments on commit 4944255

Please sign in to comment.