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

Bug 1877368: Disable Helm Chart Install form and show error alert if chart is not reachable #6572

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -62,6 +62,7 @@ const HelmInstallUpgradePage: React.FunctionComponent<HelmInstallUpgradePageProp
const [appVersion, setAppVersion] = React.useState<string>('');
const [chartReadme, setChartReadme] = React.useState<string>('');
const [chartHasValues, setChartHasValues] = React.useState<boolean>(false);
const [chartError, setChartError] = React.useState<Error>(null);

const [initialYamlData, setInitialYamlData] = React.useState<string>('');
const [initialFormData, setInitialFormData] = React.useState<object>();
Expand All @@ -87,9 +88,12 @@ const HelmInstallUpgradePage: React.FunctionComponent<HelmInstallUpgradePageProp

const fetchHelmRelease = async () => {
let res;
let error: Error = null;
try {
res = await coFetchJSON(config.helmReleaseApi);
} catch {} // eslint-disable-line no-empty
} catch (e) {
error = e;
}
if (ignore) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a query. Why do we have this ignore check after the coFetch and not before it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I will try to explain it briefly. So we do this to prevent updating a state of an unmounted component and as you can see the state is being updated only after the API call. If a component is unmounted then anyways a new API call cannot be made but this helps when a call is already made but the response has not yet been received and the component unmounts before that, so when the call returns we prevent the state from being updated.

const chart: HelmChart = res?.chart || res;
const chartValues = getChartValuesYAML(chart);
Expand All @@ -100,12 +104,13 @@ const HelmInstallUpgradePage: React.FunctionComponent<HelmInstallUpgradePageProp
setInitialYamlData(valuesYAML);
setInitialFormData(valuesJSON);
setInitialFormSchema(valuesSchema);
setChartName(chart.metadata.name);
setChartVersion(chart.metadata.version);
setAppVersion(chart.metadata.appVersion);
setChartName(chart?.metadata.name);
setChartVersion(chart?.metadata.version);
setAppVersion(chart?.metadata.appVersion);
setChartReadme(getChartReadme(chart));
setChartHasValues(!!valuesYAML);
setChartData(chart);
setChartError(error);
};

fetchHelmRelease();
Expand Down Expand Up @@ -199,7 +204,7 @@ const HelmInstallUpgradePage: React.FunctionComponent<HelmInstallUpgradePageProp
});
};

if (!chartData) {
if (!chartData && !chartError) {
return <LoadingBox />;
}

Expand All @@ -224,6 +229,7 @@ const HelmInstallUpgradePage: React.FunctionComponent<HelmInstallUpgradePageProp
chartMetaDescription={chartMetaDescription}
helmActionConfig={config}
onVersionChange={setChartData}
chartError={chartError}
/>
)}
</Formik>
Expand Down
Expand Up @@ -165,7 +165,11 @@ const HelmChartVersionDropdown: React.FunctionComponent<HelmChartVersionDropdown
items={helmChartVersions}
helpText={helpText}
disabled={_.isEmpty(helmChartVersions) || _.keys(helmChartVersions).length === 1}
title={helmChartVersions[chartVersion] || concatVersions(chartVersion, appVersion)}
title={
_.isEmpty(helmChartVersions) && !chartVersion
? 'No versions available'
: helmChartVersions[chartVersion] || concatVersions(chartVersion, appVersion)
}
onChange={handleChartVersionChange}
required
fullWidth
Expand Down
@@ -1,7 +1,7 @@
import * as React from 'react';
import * as _ from 'lodash';
import { FormikProps, FormikValues } from 'formik';
import { TextInputTypes, Grid, GridItem, Button } from '@patternfly/react-core';
import { TextInputTypes, Grid, GridItem, Button, Alert } from '@patternfly/react-core';
import {
InputField,
FormFooter,
Expand All @@ -22,6 +22,7 @@ export interface HelmInstallUpgradeFormProps {
helmActionConfig: HelmActionConfigType;
chartMetaDescription: React.ReactNode;
onVersionChange: (chart: HelmChart) => void;
chartError: Error;
}

const HelmInstallUpgradeForm: React.FC<FormikProps<FormikValues> & HelmInstallUpgradeFormProps> = ({
Expand All @@ -36,12 +37,16 @@ const HelmInstallUpgradeForm: React.FC<FormikProps<FormikValues> & HelmInstallUp
dirty,
chartMetaDescription,
onVersionChange,
chartError,
}) => {
const { chartName, chartVersion, chartReadme, formData, formSchema, editorType } = values;
const { type: helmAction, title, subTitle } = helmActionConfig;

const isSubmitDisabled =
(helmAction === HelmActionType.Upgrade && !dirty) || status?.isSubmitting || !_.isEmpty(errors);
(helmAction === HelmActionType.Upgrade && !dirty) ||
status?.isSubmitting ||
!_.isEmpty(errors) ||
!!chartError;

const uiSchema = React.useMemo(() => getJSONSchemaOrder(formSchema, {}), [formSchema]);

Expand Down Expand Up @@ -88,6 +93,11 @@ const HelmInstallUpgradeForm: React.FC<FormikProps<FormikValues> & HelmInstallUp
return (
<FlexForm onSubmit={handleSubmit}>
<FormHeader title={title} helpText={formHelpText} marginBottom="lg" />
{chartError && (
<Alert variant="danger" isInline title="Helm chart cannot be installed">
The Helm chart is currently unavailable. {`${chartError}.`}
</Alert>
)}
<FormSection fullWidth>
<Grid hasGutter>
<GridItem xl={7} lg={8} md={12}>
Expand All @@ -97,7 +107,7 @@ const HelmInstallUpgradeForm: React.FC<FormikProps<FormikValues> & HelmInstallUp
label="Release Name"
helpText="A unique name for the Helm Chart release."
required
isDisabled={helmAction === HelmActionType.Upgrade}
isDisabled={!!chartError || helmAction === HelmActionType.Upgrade}
/>
</GridItem>
<GridItem xl={5} lg={4} md={12}>
Expand All @@ -110,11 +120,13 @@ const HelmInstallUpgradeForm: React.FC<FormikProps<FormikValues> & HelmInstallUp
</GridItem>
</Grid>
</FormSection>
<SyncedEditorField
name="editorType"
formContext={{ name: 'formData', editor: formEditor, isDisabled: !formSchema }}
yamlContext={{ name: 'yamlData', editor: yamlEditor }}
/>
{!chartError && (
<SyncedEditorField
name="editorType"
formContext={{ name: 'formData', editor: formEditor, isDisabled: !formSchema }}
yamlContext={{ name: 'yamlData', editor: yamlEditor }}
/>
)}
<FormFooter
handleReset={handleReset}
errorMessage={status && status.submitError}
Expand Down
@@ -1,12 +1,13 @@
import * as React from 'react';
import * as _ from 'lodash';
import { shallow } from 'enzyme';
import { InputField, SyncedEditorField, FormHeader } from '@console/shared';
import { InputField, SyncedEditorField, FormHeader, FormFooter } from '@console/shared';
import { EditorType } from '@console/shared/src/components/synced-editor/editor-toggle';
import HelmInstallUpgradeForm from '../HelmInstallUpgradeForm';
import { HelmActionType } from '../../helm-types';
import { coFetchJSON } from '@console/internal/co-fetch';
import { formikFormProps } from '@console/shared/src/test-utils/formik-props-utils';
import { Alert } from '@patternfly/react-core';

const formValues = {
releaseName: 'helm-release',
Expand Down Expand Up @@ -46,6 +47,7 @@ const componentProps = {
helmActionConfig: helmConfig,
onVersionChange: jest.fn(),
chartMetaDescription: <p>Some chart meta</p>,
chartError: null,
};

const props: React.ComponentProps<typeof HelmInstallUpgradeForm> = {
Expand Down Expand Up @@ -123,4 +125,26 @@ describe('HelmInstallUpgradeForm', () => {
expect(wrapper.find(InputField).props().label).toBe('Release Name');
expect(wrapper.find(InputField).props().isDisabled).toBe(true);
});

it('should show error alert when chart is not reachable', () => {
const newProps = _.cloneDeep(props);
newProps.chartError = new Error('Chart not reachable');
const wrapper = shallow(<HelmInstallUpgradeForm {...newProps} />);
expect(wrapper.find(Alert).exists()).toBe(true);
});

it('should disable Release Name field and Install button if chart is not reachable', () => {
const newProps = _.cloneDeep(props);
newProps.chartError = new Error('Chart not reachable');
const wrapper = shallow(<HelmInstallUpgradeForm {...newProps} />);
expect(wrapper.find(InputField).props().isDisabled).toBe(true);
expect(wrapper.find(FormFooter).props().disableSubmit).toBe(true);
});

it('should not show form editor if chart is not reachable', () => {
const newProps = _.cloneDeep(props);
newProps.chartError = new Error('Chart not reachable');
const wrapper = shallow(<HelmInstallUpgradeForm {...newProps} />);
expect(wrapper.find(SyncedEditorField).exists()).toBe(false);
});
});