Skip to content

Commit

Permalink
add non blocking pipeline creation flow
Browse files Browse the repository at this point in the history
  • Loading branch information
karthik committed Nov 19, 2020
1 parent 33da36b commit 687dcee
Show file tree
Hide file tree
Showing 10 changed files with 183 additions and 13 deletions.
Expand Up @@ -10,7 +10,7 @@ import {
SecretModel,
} from '@console/internal/models';
import * as pipelineUtils from '@console/pipelines-plugin/src/components/import/pipeline/pipeline-template-utils';
import { PipelineModel, PipelineRunModel } from '@console/pipelines-plugin/src/models';
import { PipelineModel } from '@console/pipelines-plugin/src/models';
import { Resources } from '../import-types';
import * as submitUtils from '../import-submit-utils';
import { defaultData, nodeJsBuilderImage as buildImage } from './import-submit-utils-data';
Expand Down Expand Up @@ -183,13 +183,7 @@ describe('Import Submit Utils', () => {
'createPipelineRunForImportFlow',
);

const returnValue = await createOrUpdateResources(
mockData,
buildImage.obj,
false,
false,
'create',
);
await createOrUpdateResources(mockData, buildImage.obj, false, false, 'create');
expect(createPipelineResourceSpy).toHaveBeenCalledWith(
mockData.name,
mockData.project.name,
Expand All @@ -200,8 +194,7 @@ describe('Import Submit Utils', () => {
mockData.docker.dockerfilePath,
);
expect(createPipelineRunResourceSpy).toHaveBeenCalledTimes(1);
const models = returnValue.map((data) => _.get(data, 'model.kind'));
expect(models.includes(PipelineRunModel.kind)).toEqual(true);
expect(createPipelineRunResourceSpy).not.toThrowError();
done();
});

Expand Down Expand Up @@ -232,6 +225,44 @@ describe('Import Submit Utils', () => {
expect(pipelineRunResource.metadata.name.includes(mockData.name)).toEqual(true);
done();
});
it('should suppress the error if the pipelinerun creation fails with the error', async (done) => {
const mockData = _.cloneDeep(defaultData);
mockData.resources = Resources.Kubernetes;
mockData.pipeline.enabled = true;
const createPipelineResourceSpy = jest.spyOn(pipelineUtils, 'createPipelineForImportFlow');

const createPipelineRunSpy = jest
.spyOn(pipelineUtils, 'createPipelineRunForImportFlow')
.mockImplementation(() => Promise.reject(new Error('PipelineRun error')));

const returnValue = await createOrUpdateResources(
mockData,
buildImage.obj,
false,
false,
'create',
);
const models = returnValue.map((r) => r['model.kind']);
expect(createPipelineResourceSpy).not.toThrow();
expect(createPipelineRunSpy).not.toThrow();
expect(returnValue).toBeDefined();
expect(models).toHaveLength(6);
done();
});

it('should throw error if the deployment creation fails with the error', async (done) => {
const mockData = _.cloneDeep(defaultData);
mockData.resources = Resources.Kubernetes;
mockData.pipeline.enabled = true;
jest
.spyOn(submitUtils, 'createOrUpdateDeployment')
.mockImplementation(() => Promise.reject(new Error('Deployment')));

await expect(
createOrUpdateResources(mockData, buildImage.obj, false, false, 'create'),
).rejects.toEqual(new Error('Deployment'));
done();
});

it('should not create pipeline resource if pipeline is not enabled', async (done) => {
const mockData = _.cloneDeep(defaultData);
Expand Down
Expand Up @@ -19,6 +19,8 @@ import {
createPipelineForImportFlow,
createPipelineRunForImportFlow,
} from '@console/pipelines-plugin/src/components/import/pipeline/pipeline-template-utils';
import { Perspective } from '@console/plugin-sdk';
import { setPipelineNotStarted } from '@console/pipelines-plugin/src/components/pipelines/pipeline-overview/pipeline-overview-utils';
import {
getAppLabels,
getPodLabels,
Expand All @@ -38,7 +40,6 @@ import {
GitReadableTypes,
Resources,
} from './import-types';
import { Perspective } from '@console/plugin-sdk';

export const generateSecret = () => {
// http://stackoverflow.com/questions/105034/create-guid-uuid-in-javascript
Expand Down Expand Up @@ -477,7 +478,13 @@ export const createOrUpdateResources = async (
formData.pipeline,
formData.docker.dockerfilePath,
);
requests.push(createPipelineRunForImportFlow(newPipeline));
try {
await createPipelineRunForImportFlow(newPipeline);
} catch (error) {
// eslint-disable-next-line no-console
console.log(error);
setPipelineNotStarted(newPipeline.metadata.name);
}
}

verb === 'create' && requests.push(createWebhookSecret(formData, 'generic', dryRun));
Expand Down
@@ -1 +1,2 @@
export const TOPOLOGY_DISPLAY_FILTERS_LOCAL_STORAGE_KEY = `bridge/topology-display-filters`;
export const PIPELINE_RUN_AUTO_START_FAILED = `bridge/pipeline-run-auto-start-failed`;
Expand Up @@ -124,6 +124,7 @@
"Select {{resourceType}} resource...": "Select {{resourceType}} resource...",
"Add another value": "Add another value",
"Must define at least one task": "Must define at least one task",
"Pipeline could not be started automatically": "Pipeline could not be started automatically",
"View all {{pipelineRunsLength}}": "View all {{pipelineRunsLength}}",
"View logs": "View logs",
"Name of the cluster.": "Name of the cluster.",
Expand Down
Expand Up @@ -3,6 +3,8 @@ import { shallow } from 'enzyme';
import PipelineOverview from './PipelineOverview';
import PipelineStartButton from './PipelineStartButton';
import TriggerLastRunButton from './TriggerLastRunButton';
import { setPipelineNotStarted } from './pipeline-overview-utils';
import PipelineOverviewAlert from './PipelineOverviewAlert';

jest.mock('react-i18next', () => {
const reactI18next = require.requireActual('react-i18next');
Expand Down Expand Up @@ -51,4 +53,10 @@ describe('Pipeline sidebar overview', () => {
const wrapper = shallow(<PipelineOverview {...props} />);
expect(wrapper.find(TriggerLastRunButton)).toHaveLength(1);
});

it('should show the pipeline not started Alert', () => {
setPipelineNotStarted(props.item.pipelines[0].metadata.name);
const wrapper = shallow(<PipelineOverview {...props} />);
expect(wrapper.find(PipelineOverviewAlert)).toHaveLength(1);
});
});
Expand Up @@ -9,11 +9,13 @@ import {
resourcePath,
} from '@console/internal/components/utils';
import { referenceForModel } from '@console/internal/module/k8s';
import { PipelineRunModel, PipelineModel } from '../../../models';
import { TopologyOverviewItem } from '@console/dev-console/src/components/topology/topology-types';
import { PipelineRunModel, PipelineModel } from '../../../models';
import TriggerLastRunButton from './TriggerLastRunButton';
import PipelineRunItem from './PipelineRunItem';
import PipelineStartButton from './PipelineStartButton';
import { isPipelineNotStarted, removePipelineNotStarted } from './pipeline-overview-utils';
import PipelineOverviewAlert from './PipelineOverviewAlert';

const MAX_VISIBLE = 3;

Expand All @@ -31,9 +33,24 @@ const PipelinesOverview: React.FC<PipelinesOverviewProps> = ({
const {
metadata: { name, namespace },
} = pipeline;
const [showAlert, setShowAlert] = React.useState(isPipelineNotStarted(name));

React.useEffect(() => {
setShowAlert(isPipelineNotStarted(name));
}, [name]);

return (
<>
<SidebarSectionHeading text={PipelineRunModel.labelPlural}>
{showAlert && (
<PipelineOverviewAlert
title={t('pipelines-plugin~Pipeline could not be started automatically')}
onClose={() => {
setShowAlert(false);
removePipelineNotStarted(name);
}}
/>
)}
{pipelineRuns.length > MAX_VISIBLE && (
<Link
className="sidebar__section-view-all"
Expand Down
@@ -0,0 +1,3 @@
.pipeline-overview-alert {
margin-top: var(--pf-global--spacer--md);
}
@@ -0,0 +1,26 @@
import * as React from 'react';
import { Alert, AlertActionCloseButton, AlertProps } from '@patternfly/react-core';

import './PipelineOverviewAlert.scss';

type PipelineOverviewAlertProps = {
title: string;
onClose?: () => void;
};

const PipelineOverviewAlert: React.FC<PipelineOverviewAlertProps & AlertProps> = ({
title,
onClose,
}) => {
return (
<Alert
className="pipeline-overview-alert"
variant="default"
isInline
title={title}
actionClose={<AlertActionCloseButton onClose={onClose} />}
/>
);
};

export default PipelineOverviewAlert;
@@ -0,0 +1,54 @@
import * as overviewUtils from '../pipeline-overview-utils';

describe('pipeline-overview-utils', () => {
beforeEach(() => {
sessionStorage.clear();
});

it('should properly set the failed pipeline names in the session storage', () => {
overviewUtils.setPipelineNotStarted('pipeline-one');
overviewUtils.setPipelineNotStarted('pipeline-two');
overviewUtils.setPipelineNotStarted('pipeline-three');

expect(overviewUtils.getNotStartedPipelines()).toHaveLength(3);
});

it('should not set the session storage if empty string or null is passed', () => {
overviewUtils.setPipelineNotStarted('');
overviewUtils.setPipelineNotStarted(null);
overviewUtils.setPipelineNotStarted(undefined);

expect(overviewUtils.getNotStartedPipelines()).toHaveLength(0);
});

it('should remove the pipeline names from the session storage', () => {
overviewUtils.setPipelineNotStarted('pipeline-one');
overviewUtils.setPipelineNotStarted('pipeline-two');
overviewUtils.setPipelineNotStarted('pipeline-three');

overviewUtils.removePipelineNotStarted('pipeline-three');
overviewUtils.removePipelineNotStarted('pipeline-two');
const failedPipelines = overviewUtils.getNotStartedPipelines();

expect(failedPipelines).toHaveLength(1);
expect(failedPipelines[0]).toBe('pipeline-one');
});

it('should not affect existing values if we pass null or undefined values to remove operation', () => {
overviewUtils.setPipelineNotStarted('pipeline-one');

overviewUtils.removePipelineNotStarted(null);
overviewUtils.removePipelineNotStarted(undefined);

expect(overviewUtils.getNotStartedPipelines()).toHaveLength(1);
});

it('should return true or false based on pipeline name availability in the session storage', () => {
overviewUtils.setPipelineNotStarted('pipeline-one');
overviewUtils.setPipelineNotStarted('pipeline-two');

expect(overviewUtils.isPipelineNotStarted('pipeline-one')).toBeTruthy();
expect(overviewUtils.isPipelineNotStarted('pipeline-two')).toBeTruthy();
expect(overviewUtils.isPipelineNotStarted('pipeline-three')).toBeFalsy();
});
});
@@ -0,0 +1,22 @@
import { PIPELINE_RUN_AUTO_START_FAILED } from '@console/dev-console/src/components/topology/redux/const';

export const getNotStartedPipelines = () => {
return JSON.parse(sessionStorage.getItem(PIPELINE_RUN_AUTO_START_FAILED) ?? '[]');
};
export const isPipelineNotStarted = (pipelineName: string): boolean => {
return getNotStartedPipelines().includes(pipelineName);
};

export const removePipelineNotStarted = (pipelineName: string): void => {
const newList = getNotStartedPipelines().filter((pName) => pName !== pipelineName);
sessionStorage.setItem(PIPELINE_RUN_AUTO_START_FAILED, JSON.stringify(newList));
};

export const setPipelineNotStarted = (pipelineName: string): void => {
if (!pipelineName) return;
const pipelineList = getNotStartedPipelines();
if (pipelineList.length === 0 || !pipelineList.includes(pipelineName)) {
pipelineList.push(pipelineName);
}
sessionStorage.setItem(PIPELINE_RUN_AUTO_START_FAILED, JSON.stringify(pipelineList));
};

0 comments on commit 687dcee

Please sign in to comment.