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 1826050: Health check editor shouldn't restrict ports to those in pod spec #5401

Merged
merged 1 commit into from
May 13, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ const AddHealthChecksForm: React.FC<AddHealthChecksFormProps> = ({
healthChecks: getHealthChecksData(resource.data, containerIndex),
containerName: container.name,
resources: getResourcesType(resource.data),
image: {
ports: container.ports || [],
},
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ import { PlusCircleIcon, MinusCircleIcon } from '@patternfly/react-icons';
import { GreenCheckCircleIcon } from '@console/shared';
import { Button, ButtonVariant } from '@patternfly/react-core';
import ProbeForm from './ProbeForm';
import {
getHealthChecksProbeConfig,
getContainerPorts,
healthChecksDefaultValues,
} from './health-checks-probe-utils';
import { getHealthChecksProbeConfig, healthChecksDefaultValues } from './health-checks-probe-utils';
import { HealthCheckProbeData } from './health-checks-types';
import './HealthChecksProbe.scss';

Expand All @@ -18,10 +14,7 @@ interface HealthCheckProbeProps {

const HealthCheckProbe: React.FC<HealthCheckProbeProps> = ({ probeType }) => {
const {
values: {
image: { ports },
healthChecks,
},
values: { healthChecks },
setFieldValue,
} = useFormikContext<FormikValues>();
const [temporaryProbeData, setTemporaryProbeData] = React.useState<HealthCheckProbeData>();
Expand Down Expand Up @@ -54,14 +47,7 @@ const HealthCheckProbe: React.FC<HealthCheckProbeProps> = ({ probeType }) => {

const renderProbe = () => {
if (healthChecks?.[probeType]?.showForm) {
return (
<ProbeForm
onSubmit={handleSubmit}
onClose={handleReset}
probeType={probeType}
containerPorts={getContainerPorts(ports)}
/>
);
return <ProbeForm onSubmit={handleSubmit} onClose={handleReset} probeType={probeType} />;
}
if (healthChecks?.[probeType]?.enabled) {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,20 @@ import { RequestType } from './health-checks-types';
import FormSection from '../import/section/FormSection';
import './ProbeForm.scss';

const getRequestTypeForm = (
value: string,
probeType: string,
ports?: { [port: number]: number },
) => {
const getRequestTypeForm = (value: string, probeType: string) => {
switch (value) {
case RequestType.HTTPGET:
return <HTTPRequestTypeForm ports={ports} probeType={probeType} />;
return <HTTPRequestTypeForm probeType={probeType} />;
case RequestType.ContainerCommand:
return <CommandRequestTypeForm probeType={probeType} />;
case RequestType.TCPSocket:
return <TCPRequestTypeForm ports={ports} probeType={probeType} />;
return <TCPRequestTypeForm probeType={probeType} />;
default:
return null;
}
};

interface ProbeFormProps {
containerPorts?: { [port: number]: number };
onSubmit: () => void;
onClose: () => void;
probeType: string;
Expand All @@ -42,7 +37,7 @@ enum RequestTypeOptions {
tcpSocket = 'TCP Socket',
}

const ProbeForm: React.FC<ProbeFormProps> = ({ onSubmit, onClose, containerPorts, probeType }) => {
const ProbeForm: React.FC<ProbeFormProps> = ({ onSubmit, onClose, probeType }) => {
const {
values: { healthChecks },
errors,
Expand All @@ -58,11 +53,7 @@ const ProbeForm: React.FC<ProbeFormProps> = ({ onSubmit, onClose, containerPorts
title={RequestType.HTTPGET}
fullWidth
/>
{getRequestTypeForm(
healthChecks?.[probeType]?.data?.requestType,
probeType,
containerPorts,
)}
{getRequestTypeForm(healthChecks?.[probeType]?.data?.requestType, probeType)}
<InputField
type={TextInputTypes.number}
name={`healthChecks.${probeType}.data.failureThreshold`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,15 @@ import * as React from 'react';
import * as _ from 'lodash';
import { FormikValues, useFormikContext } from 'formik';
import { TextInputTypes, FormGroup } from '@patternfly/react-core';
import {
InputField,
CheckboxField,
DropdownField,
getFieldId,
TextColumnField,
} from '@console/shared';
import { InputField, CheckboxField, getFieldId, TextColumnField } from '@console/shared';
import { NameValueEditor } from '@console/internal/components/utils/name-value-editor';
import { Resources } from '../import/import-types';

interface RequestTypeFormProps {
ports?: { [port: number]: number };
probeType?: string;
}

export const renderPortField = (
ports: { [port: number]: number },
fieldName: string,
defaultPort: number,
resourceType: Resources,
) => {
export const renderPortField = (fieldName: string, resourceType: Resources) => {
if (resourceType === Resources.KnativeService) {
return (
<InputField
Expand All @@ -34,27 +22,15 @@ export const renderPortField = (
/>
);
}
return _.isEmpty(ports) ? (
<InputField type={TextInputTypes.text} name={fieldName} label="Port" required />
) : (
<DropdownField
name={fieldName}
label="Port"
items={ports}
title={ports[defaultPort] || 'Select target port'}
fullWidth
required
/>
);
return <InputField type={TextInputTypes.text} name={fieldName} label="Port" required />;
};

export const HTTPRequestTypeForm: React.FC<RequestTypeFormProps> = ({ ports, probeType }) => {
export const HTTPRequestTypeForm: React.FC<RequestTypeFormProps> = ({ probeType }) => {
const {
values: { healthChecks, resources },
setFieldValue,
} = useFormikContext<FormikValues>();
const httpHeaders = healthChecks?.[probeType]?.data?.httpGet?.httpHeaders;
const defaultPort = healthChecks?.[probeType]?.data?.httpGet?.port;
const initialNameValuePairs = !_.isEmpty(httpHeaders)
? httpHeaders.map((val) => _.values(val))
: [['', '']];
Expand Down Expand Up @@ -107,18 +83,17 @@ export const HTTPRequestTypeForm: React.FC<RequestTypeFormProps> = ({ ports, pro
label="Path"
placeholder="/"
/>
{renderPortField(ports, portFieldName, defaultPort, resources)}
{renderPortField(portFieldName, resources)}
</>
);
};

export const TCPRequestTypeForm: React.FC<RequestTypeFormProps> = ({ ports, probeType }) => {
export const TCPRequestTypeForm: React.FC<RequestTypeFormProps> = ({ probeType }) => {
const {
values: { healthChecks, resources },
values: { resources },
} = useFormikContext<FormikValues>();
const defaultPort = healthChecks?.[probeType]?.data?.tcpSocket?.port;
const portFieldName = `healthChecks.${probeType}.data.tcpSocket.port`;
return renderPortField(ports, portFieldName, defaultPort, resources);
return renderPortField(portFieldName, resources);
};

export const CommandRequestTypeForm: React.FC<RequestTypeFormProps> = ({ probeType }) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
import { HealthChecksProbeType, RequestType, HealthCheckProbe } from './health-checks-types';

export const getContainerPorts = (ports): { [port: number]: number } => {
const containerPorts = ports.reduce((acc, port) => {
acc[port.containerPort] = port.containerPort;
return acc;
}, {});
return containerPorts;
};

export const getHealthChecksProbeConfig = (probe: string) => {
switch (probe) {
case HealthChecksProbeType.ReadinessProbe: {
Expand Down