Skip to content

Commit

Permalink
Merge branch 'main' into f/mserving-metrics
Browse files Browse the repository at this point in the history
  • Loading branch information
alexcreasy committed Jun 19, 2023
2 parents e2f799d + 73dc3ba commit 7cd7cf2
Show file tree
Hide file tree
Showing 57 changed files with 839 additions and 439 deletions.
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ COPY --chown=default:root ${SOURCE_CODE} /usr/src/app
# Change file ownership to the assemble user
USER default

RUN npm cache clean --force
RUN npm cache clean --force

RUN npm ci --omit=optional

Expand All @@ -39,7 +39,7 @@ COPY --chown=default:root --from=builder /usr/src/app/.npmrc /usr/src/app/backen
COPY --chown=default:root --from=builder /usr/src/app/.env /usr/src/app/.env
COPY --chown=default:root --from=builder /usr/src/app/data /usr/src/app/data

RUN cd backend && npm cache clean --force && npm ci --omit=dev --omit=optional
RUN cd backend && npm cache clean --force && npm ci --omit=dev --omit=optional && chmod -R g+w ${HOME}/.npm

WORKDIR /usr/src/app/backend

Expand Down
41 changes: 27 additions & 14 deletions backend/src/routes/api/images/imageUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IMAGE_ANNOTATIONS } from '../../../utils/constants';
import { IMAGE_ANNOTATIONS, imageUrlRegex } from '../../../utils/constants';
import { convertLabelsToString } from '../../../utils/componentUtils';
import {
ImageStreamTag,
Expand All @@ -11,7 +11,6 @@ import {
BYONImageUpdateRequest,
BYONImagePackage,
BYONImage,
BYONImageStatus,
} from '../../../types';
import { FastifyRequest } from 'fastify';
import createError from 'http-errors';
Expand Down Expand Up @@ -91,6 +90,18 @@ const getImageStreams = async (
return await requestPromise;
};

const isBYONImage = (imageStream: ImageStream) =>
imageStream.metadata.labels?.['app.kubernetes.io/created-by'] === 'byon';

const getBYONImageErrorMessage = (imageStream: ImageStream) => {
// there will be always only 1 tag in the spec for BYON images
// status tags could be more than one
const activeTag = imageStream.status?.tags?.find(
(statusTag) => statusTag.tag === imageStream.spec.tags?.[0].name,
);
return activeTag?.conditions?.[0]?.message;
};

export const processImageInfo = (imageStream: ImageStream): ImageInfo => {
const annotations = imageStream.metadata.annotations || {};

Expand All @@ -102,6 +113,7 @@ export const processImageInfo = (imageStream: ImageStream): ImageInfo => {
tags: getTagInfo(imageStream),
order: +annotations[IMAGE_ANNOTATIONS.IMAGE_ORDER] || 100,
dockerImageRepo: imageStream.status?.dockerImageRepository || '',
error: isBYONImage && getBYONImageErrorMessage(imageStream),
};

return imageInfo;
Expand Down Expand Up @@ -180,11 +192,8 @@ const mapImageStreamToBYONImage = (is: ImageStream): BYONImage => ({
id: is.metadata.name,
name: is.metadata.annotations['opendatahub.io/notebook-image-name'],
description: is.metadata.annotations['opendatahub.io/notebook-image-desc'],
phase: is.metadata.annotations['opendatahub.io/notebook-image-phase'] as BYONImageStatus,
visible: is.metadata.labels['opendatahub.io/notebook-image'] === 'true',
error: is.metadata.annotations['opendatahub.io/notebook-image-messages']
? JSON.parse(is.metadata.annotations['opendatahub.io/notebook-image-messages'])
: [],
error: getBYONImageErrorMessage(is),
packages:
is.spec.tags &&
(JSON.parse(
Expand All @@ -207,7 +216,14 @@ export const postImage = async (
const customObjectsApi = fastify.kube.customObjectsApi;
const namespace = fastify.kube.namespace;
const body = request.body as BYONImageCreateRequest;
const imageTag = body.url.split(':')[1];
const fullUrl = body.url;
const matchArray = fullUrl.match(imageUrlRegex);
// check if the host is valid
if (!matchArray[1]) {
fastify.log.error('Invalid repository URL unable to add notebook image');
return { success: false, error: 'Invalid repository URL: ' + fullUrl };
}
const imageTag = matchArray[4];
const labels = {
'app.kubernetes.io/created-by': 'byon',
'opendatahub.io/notebook-image': 'true',
Expand All @@ -228,11 +244,8 @@ export const postImage = async (
annotations: {
'opendatahub.io/notebook-image-desc': body.description ? body.description : '',
'opendatahub.io/notebook-image-name': body.name,
'opendatahub.io/notebook-image-url': body.url,
'opendatahub.io/notebook-image-url': fullUrl,
'opendatahub.io/notebook-image-creator': body.user,
'opendatahub.io/notebook-image-phase': 'Succeeded',
'opendatahub.io/notebook-image-origin': 'Admin',
'opendatahub.io/notebook-image-messages': '',
},
name: `byon-${Date.now()}`,
namespace: namespace,
Expand All @@ -247,13 +260,13 @@ export const postImage = async (
annotations: {
'opendatahub.io/notebook-software': packagesToString(body.software),
'opendatahub.io/notebook-python-dependencies': packagesToString(body.packages),
'openshift.io/imported-from': body.url,
'openshift.io/imported-from': fullUrl,
},
from: {
kind: 'DockerImage',
name: body.url,
name: fullUrl,
},
name: imageTag,
name: imageTag || 'latest',
},
],
},
Expand Down
1 change: 1 addition & 0 deletions backend/src/routes/api/proxy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export default async (fastify: KubeFastifyInstance): Promise<void> => {
url,
overrideContentType: contentType,
requestData,
rejectUnauthorized: false,
}).catch((error) => {
if (error.code && error.response) {
const { code, response } = error;
Expand Down
19 changes: 10 additions & 9 deletions backend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,19 +465,11 @@ export type ODHSegmentKey = {
segmentKey: string;
};

export type BYONImageError = {
severity: string;
message: string;
};

export type BYONImageStatus = 'Importing' | 'Validating' | 'Succeeded' | 'Failed';

export type BYONImage = {
id: string;
phase?: BYONImageStatus;
user?: string;
uploaded?: Date;
error?: BYONImageError;
error?: string;
} & BYONImageCreateRequest &
BYONImageUpdateRequest;

Expand Down Expand Up @@ -531,6 +523,14 @@ export type ImageStreamStatusTagItem = {
export type ImageStreamStatusTag = {
tag: string;
items: ImageStreamStatusTagItem[];
conditions?: ImageStreamStatusTagCondition[];
};

export type ImageStreamStatusTagCondition = {
type: string;
status: string;
reason: string;
message: string;
};

export type ImageStreamStatus = {
Expand Down Expand Up @@ -590,6 +590,7 @@ export type ImageInfo = {
default?: boolean;
order?: number;
dockerImageRepo?: string;
error?: string;
};

export type ImageType = 'byon' | 'jupyter' | 'other';
Expand Down
5 changes: 4 additions & 1 deletion backend/src/utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ export const blankDashboardCR: DashboardConfig = {
disableProjectSharing: false,
disableCustomServingRuntimes: false,
modelMetricsNamespace: '',
disablePipelines: true,
disableBiasMetrics: false,
disablePipelines: false,
},
notebookController: {
enabled: true,
Expand Down Expand Up @@ -131,3 +131,6 @@ export const DEFAULT_NOTEBOOK_SIZES: NotebookSize[] = [
},
},
];

export const imageUrlRegex =
/^([\w.\-_]+((?::\d+|)(?=\/[a-z0-9._-]+\/[a-z0-9._-]+))|)(?:\/|)([a-z0-9.\-_]+(?:\/[a-z0-9.\-_]+|))(?::([\w.\-_]{1,127})|)/;
49 changes: 49 additions & 0 deletions frontend/src/__tests__/dockerRepositoryURL.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// https://cloud.google.com/artifact-registry/docs/docker/names
// The full name for a container image is one of the following formats:
// LOCATION-docker.pkg.dev/PROJECT-ID/REPOSITORY/IMAGE
// LOCATION-docker.pkg.dev/PROJECT-ID/REPOSITORY/IMAGE:TAG
// LOCATION-docker.pkg.dev/PROJECT-ID/REPOSITORY/IMAGE@IMAGE-DIGEST

import { REPOSITORY_URL_REGEX } from '~/utilities/const';

test('Invalid URL', () => {
const url = 'docker.io';
const match = url.match(REPOSITORY_URL_REGEX);
expect(match?.[1]).toBe('');
});

test('Docker container URL without tag', () => {
const url = 'docker.io/library/mysql';
const match = url.match(REPOSITORY_URL_REGEX);
expect(match?.[1]).toBe('docker.io');
expect(match?.[4]).toBe(undefined);
});

test('Docker container URL with tag', () => {
const url = 'docker.io/library/mysql:test-tag';
const match = url.match(REPOSITORY_URL_REGEX);
expect(match?.[1]).toBe('docker.io');
expect(match?.[4]).toBe('test-tag');
});

test('OpenShift internal registry URL without tag', () => {
const url = 'image-registry.openshift-image-registry.svc:5000/opendatahub/s2i-minimal-notebook';
const match = url.match(REPOSITORY_URL_REGEX);
expect(match?.[1]).toBe('image-registry.openshift-image-registry.svc:5000');
expect(match?.[4]).toBe(undefined);
});

test('OpenShift internal registry URL with tag', () => {
const url =
'image-registry.openshift-image-registry.svc:5000/opendatahub/s2i-minimal-notebook:v0.3.0-py36';
const match = url.match(REPOSITORY_URL_REGEX);
expect(match?.[1]).toBe('image-registry.openshift-image-registry.svc:5000');
expect(match?.[4]).toBe('v0.3.0-py36');
});

test('Quay URL with port and tag', () => {
const url = 'quay.io:443/opendatahub/odh-dashboard:main-55e19fa';
const match = url.match(REPOSITORY_URL_REGEX);
expect(match?.[1]).toBe('quay.io:443');
expect(match?.[4]).toBe('main-55e19fa');
});
16 changes: 13 additions & 3 deletions frontend/src/api/k8s/serviceAccounts.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { k8sCreateResource, k8sDeleteResource } from '@openshift/dynamic-plugin-sdk-utils';
import {
k8sCreateResource,
k8sDeleteResource,
k8sGetResource,
} from '@openshift/dynamic-plugin-sdk-utils';
import { ServiceAccountModel } from '~/api/models';
import { K8sAPIOptions, K8sStatus, ServiceAccountKind } from '~/k8sTypes';
import { applyK8sAPIOptions } from '~/api/apiMergeUtils';
Expand All @@ -15,7 +19,13 @@ export const assembleServiceAccount = (name: string, namespace: string): Service
return serviceAccount;
};

export const createServiceAccount = async (
export const getServiceAccount = (name: string, namespace: string): Promise<ServiceAccountKind> =>
k8sGetResource<ServiceAccountKind>({
model: ServiceAccountModel,
queryOptions: { name, ns: namespace },
});

export const createServiceAccount = (
data: ServiceAccountKind,
opts?: K8sAPIOptions,
): Promise<ServiceAccountKind> =>
Expand All @@ -26,7 +36,7 @@ export const createServiceAccount = async (
}),
);

export const deleteServiceAccount = async (
export const deleteServiceAccount = (
name: string,
ns: string,
opts?: K8sAPIOptions,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// remove this file when https://github.com/patternfly/patternfly/issues/5605 is solved
.odh-dashboard__code-editor {
height: 100%;

.pf-c-code-editor__main,
.pf-c-file-upload,
.pf-c-code-editor__code {
height: 100%;
}
}
19 changes: 19 additions & 0 deletions frontend/src/concepts/dashboard/codeEditor/DashboardCodeEditor.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import * as React from 'react';
import { CodeEditor, CodeEditorProps } from '@patternfly/react-code-editor';

import './DashboardCodeEditor.scss';

type DashboardCodeEditorProps = Omit<CodeEditorProps, 'ref'>;

const DashboardCodeEditor: React.FC<Partial<DashboardCodeEditorProps>> = ({
// 38px is the code editor toolbar height+border
// calculate the div height to avoid container scrolling
height = 'calc(100% - 38px)',
...props
}) => (
<div style={{ height }}>
<CodeEditor height="100%" className="odh-dashboard__code-editor" {...props} />
</div>
);

export default DashboardCodeEditor;
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import * as React from 'react';
import { Alert, Button, Form, Modal, Stack, StackItem } from '@patternfly/react-core';
import { DataConnection } from '~/pages/projects/types';
import { EMPTY_AWS_SECRET_DATA } from '~/pages/projects/dataConnections/const';
import './ConfigurePipelinesServerModal.scss';
import { convertAWSSecretData } from '~/pages/projects/screens/detail/data-connections/utils';
import { usePipelinesAPI } from '~/concepts/pipelines/context';
import useDataConnections from '~/pages/projects/screens/detail/data-connections/useDataConnections';
import { useContextResourceData } from '~/utilities/useContextResourceData';
import { isAWSValid } from '~/pages/projects/screens/spawner/spawnerUtils';
import { createPipelinesCR, deleteSecret } from '~/api';
import useDataConnections from '~/pages/projects/screens/detail/data-connections/useDataConnections';
import { PipelinesDatabaseSection } from './PipelinesDatabaseSection';
import { ObjectStorageSection } from './ObjectStorageSection';
import {
Expand All @@ -35,11 +33,17 @@ export const ConfigurePipelinesServerModal: React.FC<ConfigurePipelinesServerMod
open,
}) => {
const { project, namespace } = usePipelinesAPI();
const dataConnections = useContextResourceData<DataConnection>(useDataConnections(namespace));
const [dataConnections, , , refresh] = useDataConnections(namespace);
const [fetching, setFetching] = React.useState(false);
const [error, setError] = React.useState<Error | null>(null);
const [config, setConfig] = React.useState<PipelineServerConfigType>(FORM_DEFAULTS);

React.useEffect(() => {
if (open) {
refresh();
}
}, [open, refresh]);

const canSubmit = () => {
const databaseIsValid = config.database.useDefault
? true
Expand Down Expand Up @@ -69,9 +73,7 @@ export const ConfigurePipelinesServerModal: React.FC<ConfigurePipelinesServerMod
let objectStorage: PipelineServerConfigType['objectStorage'];
if (config.objectStorage.useExisting) {
const existingName = config.objectStorage.existingName;
const existingValue = dataConnections.data?.find(
(dc) => dc.data.metadata.name === existingName,
);
const existingValue = dataConnections?.find((dc) => dc.data.metadata.name === existingName);
if (existingValue) {
objectStorage = {
existingValue: convertAWSSecretData(existingValue),
Expand Down Expand Up @@ -144,7 +146,11 @@ export const ConfigurePipelinesServerModal: React.FC<ConfigurePipelinesServerMod
submit();
}}
>
<ObjectStorageSection setConfig={setConfig} config={config} />
<ObjectStorageSection
setConfig={setConfig}
config={config}
dataConnections={dataConnections}
/>
<PipelinesDatabaseSection setConfig={setConfig} config={config} />
</Form>
</StackItem>
Expand Down
Loading

0 comments on commit 7cd7cf2

Please sign in to comment.