Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support a devfile that includes a parent with the storage-type attribute #1303

Merged
merged 6 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export class BasicViewer extends React.PureComponent<Props> {
});
editor.setSize(`100%`, `100%`);
editor.setValue(this.props.value);
editor.focus();

this.editor = editor;
}
Expand All @@ -44,7 +43,6 @@ export class BasicViewer extends React.PureComponent<Props> {
componentDidUpdate(prevProps: Readonly<Props>): void {
if (this.editor && this.props.value !== prevProps.value) {
this.editor.setValue(this.props.value);
this.editor.focus();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,16 @@ export class DevfileViewer extends React.PureComponent<Props> {
lineWrapping: true,
readOnly: true,
autoRefresh: true,
autofocus: true,
gooters: true,
});
editor.setSize(`100%`, `100%`);
editor.setValue(this.props.value);
editor.focus();

this.editor = editor;
}
}

componentDidUpdate(): void {
this.editor.setValue(this.props.value);
this.editor.focus();
}

public render(): React.ReactElement {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ const devfile = {
},
} as devfileApi.Devfile;

// mute console.error
console.error = jest.fn();

describe('Creating steps, applying a devfile', () => {
let searchParams: URLSearchParams;
let factoryId: string;
Expand Down Expand Up @@ -245,6 +248,7 @@ describe('Creating steps, applying a devfile', () => {
factoryId,
undefined,
false,
undefined,
),
);
await waitFor(() => expect(mockCreateWorkspaceFromDevfile).toHaveBeenCalled());
Expand Down Expand Up @@ -334,6 +338,7 @@ describe('Creating steps, applying a devfile', () => {
factoryId,
undefined,
false,
undefined,
),
);
await waitFor(() => expect(mockCreateWorkspaceFromDevfile).toHaveBeenCalled());
Expand Down Expand Up @@ -410,6 +415,7 @@ describe('Creating steps, applying a devfile', () => {
factoryId,
undefined,
false,
undefined,
),
);
await waitFor(() => expect(mockCreateWorkspaceFromDevfile).toHaveBeenCalled());
Expand Down Expand Up @@ -488,6 +494,7 @@ describe('Creating steps, applying a devfile', () => {
factoryId,
undefined,
false,
undefined,
),
);
await waitFor(() => expect(mockCreateWorkspaceFromDevfile).toHaveBeenCalled());
Expand All @@ -511,7 +518,7 @@ describe('Creating steps, applying a devfile', () => {
await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

await waitFor(() =>
expect(prepareDevfile).toHaveBeenCalledWith(devfile, factoryId, undefined, true),
expect(prepareDevfile).toHaveBeenCalledWith(devfile, factoryId, undefined, true, undefined),
);
});

Expand All @@ -534,7 +541,7 @@ describe('Creating steps, applying a devfile', () => {
await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

await waitFor(() =>
expect(prepareDevfile).toHaveBeenCalledWith(devfile, factoryId, undefined, true),
expect(prepareDevfile).toHaveBeenCalledWith(devfile, factoryId, undefined, true, undefined),
);
});

Expand All @@ -554,7 +561,13 @@ describe('Creating steps, applying a devfile', () => {
await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

await waitFor(() =>
expect(prepareDevfile).toHaveBeenCalledWith(devfile, factoryId, undefined, false),
expect(prepareDevfile).toHaveBeenCalledWith(
devfile,
factoryId,
undefined,
false,
undefined,
),
);
});
});
Expand Down Expand Up @@ -679,7 +692,7 @@ describe('Creating steps, applying a devfile', () => {
expect(mockOnNextStep).not.toHaveBeenCalled();
expect(mockOnError).not.toHaveBeenCalled();

expect(mockCreateWorkspaceFromDevfile).toHaveBeenCalledTimes(1);
await waitFor(() => expect(mockCreateWorkspaceFromDevfile).toHaveBeenCalledTimes(1));
});

test('action callback to continue with default devfile', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jest.mock('@/services/helpers/generateName');

describe('FactoryLoaderContainer/prepareDevfile', () => {
describe('DEVWORKSPACE_METADATA_ANNOTATION attribute', () => {
test('add the attribute with annotation', () => {
test('add the attribute with annotation', async () => {
const devfile = {
schemaVersion: '2.2.0',
metadata: {
Expand All @@ -49,7 +49,7 @@ describe('FactoryLoaderContainer/prepareDevfile', () => {
});
});

test('update the attribute with annotation', () => {
test('update the attribute with annotation', async () => {
const customAnnotation = {
'custom-annotation': 'value',
};
Expand Down Expand Up @@ -226,5 +226,112 @@ describe('FactoryLoaderContainer/prepareDevfile', () => {
expect(newDevfile.metadata.attributes?.[DEVWORKSPACE_STORAGE_TYPE_ATTR]).toBeUndefined();
expect(newDevfile.attributes?.[DEVWORKSPACE_STORAGE_TYPE_ATTR]).toEqual('ephemeral');
});

describe('has parent', () => {
describe('with registryUrl', () => {
it('with storage-type attribute', async () => {
// mute console logs
console.warn = jest.fn();
const devfile = {
schemaVersion: '2.2.0',
metadata: {
name: 'wksp-test',
},
parent: {
id: 'nodejs',
registryUrl: 'https://registry.devfile.io/',
},
} as devfileApi.Devfile;

const newDevfile = prepareDevfile(devfile, factoryId, 'ephemeral', false, {
schemaVersion: '2.2.2',
metadata: {
generateName: 'nodejs',
},
attributes: {
'controller.devfile.io/storage-type': 'ephemeral',
},
} as devfileApi.Devfile);

expect(console.warn).toHaveBeenCalledWith(
'Unable to apply controller.devfile.io/storage-type attribute.',
);
expect(newDevfile.attributes?.[DEVWORKSPACE_STORAGE_TYPE_ATTR]).toBeUndefined();
});

it('without storage-type attribute', async () => {
const devfile = {
schemaVersion: '2.2.0',
metadata: {
name: 'wksp-test',
},
parent: {
id: 'nodejs',
registryUrl: 'https://registry.devfile.io/',
},
} as devfileApi.Devfile;

const newDevfile = prepareDevfile(devfile, factoryId, 'ephemeral', false, {
schemaVersion: '2.2.2',
metadata: {
generateName: 'nodejs',
},
} as devfileApi.Devfile);

expect(newDevfile.attributes?.[DEVWORKSPACE_STORAGE_TYPE_ATTR]).toEqual('ephemeral');
});
});
describe('with uri', () => {
it('with storage-type attribute', async () => {
// mute console logs
console.warn = jest.fn();
const devfile = {
schemaVersion: '2.2.0',
metadata: {
name: 'wksp-test',
},
parent: {
uri: 'https://raw.githubusercontent.com/test/devfile.yaml',
},
} as devfileApi.Devfile;

const newDevfile = prepareDevfile(devfile, factoryId, 'ephemeral', false, {
schemaVersion: '2.2.2',
metadata: {
generateName: 'nodejs',
},
attributes: {
'controller.devfile.io/storage-type': 'ephemeral',
},
} as devfileApi.Devfile);

expect(console.warn).toHaveBeenCalledWith(
'Unable to apply controller.devfile.io/storage-type attribute.',
);
expect(newDevfile.attributes?.[DEVWORKSPACE_STORAGE_TYPE_ATTR]).toBeUndefined();
});

it('without storage-type attribute', async () => {
const devfile = {
schemaVersion: '2.2.0',
metadata: {
name: 'wksp-test',
},
parent: {
uri: 'https://raw.githubusercontent.com/test/devfile.yaml',
},
} as devfileApi.Devfile;

const newDevfile = prepareDevfile(devfile, factoryId, 'ephemeral', false, {
schemaVersion: '2.2.2',
metadata: {
generateName: 'nodejs',
},
} as devfileApi.Devfile);

expect(newDevfile.attributes?.[DEVWORKSPACE_STORAGE_TYPE_ATTR]).toEqual('ephemeral');
});
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import { RootState } from '@/store';
import { selectDefaultDevfile } from '@/store/DevfileRegistries/selectors';
import { selectFactoryResolver } from '@/store/FactoryResolver/selectors';
import { selectDefaultNamespace } from '@/store/InfrastructureNamespaces/selectors';
import { selectPvcStrategy } from '@/store/ServerConfig';
import { workspacesActionCreators } from '@/store/Workspaces';
import { selectDevWorkspaceWarnings } from '@/store/Workspaces/devWorkspaces/selectors';
import { selectAllWorkspaces } from '@/store/Workspaces/selectors';
Expand Down Expand Up @@ -176,7 +177,7 @@ class CreatingStepApplyDevfile extends ProgressStep<Props, State> {
private updateCurrentDevfile(devfile: devfileApi.Devfile): void {
const { factoryResolver, allWorkspaces, defaultDevfile } = this.props;
const { factoryParams } = this.state;
const { factoryId, policiesCreate, sourceUrl, storageType, remotes } = factoryParams;
const { factoryId, policiesCreate, sourceUrl, remotes } = factoryParams;

// when using the default devfile instead of a user devfile
if (factoryResolver === undefined && isEqual(devfile, defaultDevfile)) {
Expand Down Expand Up @@ -214,8 +215,16 @@ class CreatingStepApplyDevfile extends ProgressStep<Props, State> {
// test the devfile name to decide if we need to append a suffix to is
const nameConflict = allWorkspaces.some(w => devfile.metadata.name === w.name);

const storageType = factoryParams.storageType || this.props.preferredStorageType || undefined;
const appendSuffix = policiesCreate === 'perclick' || nameConflict;
const updatedDevfile = prepareDevfile(devfile, factoryId, storageType, appendSuffix);
const parentDevfile = factoryResolver?.parentDevfile;
const updatedDevfile = prepareDevfile(
devfile,
factoryId,
storageType,
appendSuffix,
parentDevfile,
);

this.setState({
devfile: updatedDevfile,
Expand Down Expand Up @@ -465,6 +474,7 @@ const mapStateToProps = (state: RootState) => ({
factoryResolver: selectFactoryResolver(state),
defaultDevfile: selectDefaultDevfile(state),
devWorkspaceWarnings: selectDevWorkspaceWarnings(state),
preferredStorageType: selectPvcStrategy(state),
});

const connector = connect(mapStateToProps, workspacesActionCreators, null, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function prepareDevfile(
factoryId: string,
storageType: che.WorkspaceStorageType | undefined,
appendSuffix: boolean,
parentDevfile?: devfileApi.Devfile | undefined,
): devfileApi.Devfile {
const devfile = cloneDeep(_devfile);
const attributes = DevfileAdapter.getAttributes(devfile);
Expand Down Expand Up @@ -60,8 +61,15 @@ export function prepareDevfile(
devfile.metadata.name = sanitizeName(devfile.metadata.name);

// propagate storage type
if (storageType === 'ephemeral') {
attributes[DEVWORKSPACE_STORAGE_TYPE_ATTR] = 'ephemeral';
if (storageType) {
attributes[DEVWORKSPACE_STORAGE_TYPE_ATTR] = storageType;
}
if (parentDevfile && attributes[DEVWORKSPACE_STORAGE_TYPE_ATTR]) {
const parentDevfileAttributes = DevfileAdapter.getAttributes(parentDevfile);
if (parentDevfileAttributes[DEVWORKSPACE_STORAGE_TYPE_ATTR]) {
delete attributes[DEVWORKSPACE_STORAGE_TYPE_ATTR];
console.warn(`Unable to apply ${DEVWORKSPACE_STORAGE_TYPE_ATTR} attribute.`);
}
}

return devfile;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { selectDevWorkspaceResources } from '@/store/DevfileRegistries/selectors
import { factoryResolverActionCreators } from '@/store/FactoryResolver';
import { selectFactoryResolver } from '@/store/FactoryResolver/selectors';
import { selectDefaultNamespace } from '@/store/InfrastructureNamespaces/selectors';
import { selectPvcStrategy } from '@/store/ServerConfig';
import { workspacesActionCreators } from '@/store/Workspaces';
import { selectDevWorkspaceWarnings } from '@/store/Workspaces/devWorkspaces/selectors';
import { selectAllWorkspaces } from '@/store/Workspaces/selectors';
Expand Down Expand Up @@ -174,7 +175,7 @@ class CreatingStepApplyResources extends ProgressStep<Props, State> {
protected async runStep(): Promise<boolean> {
const { devWorkspaceResources } = this.props;
const { factoryParams, shouldCreate, resources, warning } = this.state;
const { cheEditor, factoryId, sourceUrl, storageType, policiesCreate } = factoryParams;
const { cheEditor, factoryId, sourceUrl, policiesCreate } = factoryParams;

if (warning) {
const newName = `Warning: ${warning}`;
Expand Down Expand Up @@ -202,7 +203,7 @@ class CreatingStepApplyResources extends ProgressStep<Props, State> {
return true;
}

if (shouldCreate === false) {
if (!shouldCreate) {
throw new Error('The workspace creation unexpectedly failed.');
}

Expand All @@ -218,6 +219,7 @@ class CreatingStepApplyResources extends ProgressStep<Props, State> {
);
const appendSuffix = policiesCreate === 'perclick' || nameConflict;

const storageType = factoryParams.storageType || this.props.preferredStorageType || undefined;
// create a workspace using pre-generated resources
const [devWorkspace, devWorkspaceTemplate] = prepareResources(
_resources,
Expand Down Expand Up @@ -297,6 +299,7 @@ const mapStateToProps = (state: RootState) => ({
factoryResolver: selectFactoryResolver(state),
devWorkspaceResources: selectDevWorkspaceResources(state),
devWorkspaceWarnings: selectDevWorkspaceWarnings(state),
preferredStorageType: selectPvcStrategy(state),
});

const connector = connect(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export default function prepareResources(
}

// set storage type attribute
if (storageType === 'ephemeral') {
if (storageType) {
if (!devWorkspace.spec.template.attributes) {
devWorkspace.spec.template.attributes = {};
}
Expand Down
Loading
Loading