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

fix: show correct health checker #1784

Merged
merged 7 commits into from
Apr 19, 2021
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.
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 @@ -61,7 +61,7 @@ context('Can select service_id skip upstream in route', () => {
cy.get(this.domSelector.name).type(this.data.routeName);
cy.contains('Next').click();
cy.get(this.domSelector.upstreamSelector).click();
cy.contains('None').should('not.exist');
cy.get('.ant-select-item-option-disabled > .ant-select-item-option-content').contains('None');

cy.contains('Previous').click();
cy.wait(500);
Expand Down Expand Up @@ -91,31 +91,34 @@ context('Can select service_id skip upstream in route', () => {
cy.contains(this.data.routeName).siblings().contains('Configure').click();
cy.get(this.domSelector.serviceSelector).click();
cy.contains('None').click();
cy.get(this.domSelector.notification).should('contain', 'Please check the configuration of binding service');
cy.get(this.domSelector.notificationCloseIcon).click();

cy.contains('Next').click();
cy.get(this.domSelector.upstream_id).click();
cy.contains('None').should('not.exist');
cy.wait(500);
cy.get('[data-cy=upstream_selector]').click();
cy.contains(this.data.upstreamName).click();
cy.contains('Next').click();
cy.contains('Next').click();
cy.contains('Submit').click();
cy.contains(this.data.submitSuccess);
});

it('should delete upstream, service and route', function () {
cy.visit('/');
cy.contains('Service').click();
cy.contains(this.data.serviceName).siblings().contains('Delete').click();
cy.contains('button', 'Confirm').click();
cy.get(this.domSelector.notification).should('contain', this.data.deleteServiceSuccess);

it('should delete route, service and upstream', function () {
cy.visit('/');
cy.contains('Route').click();
cy.contains(this.data.routeName).siblings().contains('More').click();
cy.contains('Delete').click();
cy.get(this.domSelector.deleteAlert).should('be.visible').within(() => {
cy.contains('OK').click();
});

cy.get(this.domSelector.notification).should('contain', this.data.deleteRouteSuccess);
cy.visit('/');
cy.contains('Service').click();
cy.contains(this.data.serviceName).siblings().contains('Delete').click();
cy.contains('button', 'Confirm').click();
cy.get(this.domSelector.notification).should('contain', this.data.deleteServiceSuccess);

cy.visit('/');
cy.contains('Upstream').click();
Expand Down
13 changes: 8 additions & 5 deletions web/cypress/integration/route/create-route-with-upstream.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ context('Create Route with Upstream', () => {
cy.get(this.domSelector.input).should('be.disabled');
// should enable Upstream input boxes after selecting Custom mode
cy.get(this.domSelector.upstreamSelector).click();
cy.contains('Custom').click();
cy.get(this.domSelector.input).should('not.be.disabled');
cy.contains('.ant-select-item-option-content', 'Custom').click();

cy.get(this.domSelector.nodes_0_host).clear().type(this.data.ip1);
cy.get(this.domSelector.nodes_0_port).type(this.data.port);
Expand All @@ -77,7 +76,9 @@ context('Create Route with Upstream', () => {
cy.contains(this.data.routeName).siblings().contains('Configure').click();

cy.get(this.domSelector.name).should('value', this.data.routeName);
cy.contains('Next').click({ force: true });
cy.contains('Next').click({
force: true
});

// check if the changes have been saved
cy.get(this.domSelector.nodes_0_host).should('value', this.data.ip1);
Expand All @@ -87,7 +88,7 @@ context('Create Route with Upstream', () => {
cy.get(this.domSelector.input).should('be.disabled');

cy.contains(this.data.upstreamName).click();
cy.contains('Custom').click();
cy.contains('.ant-select-item-option-content', 'Custom').click();
cy.get(this.domSelector.input).should('not.be.disabled');

cy.get(this.domSelector.nodes_0_host).clear().type(this.data.ip2);
Expand All @@ -107,7 +108,9 @@ context('Create Route with Upstream', () => {
cy.contains(this.data.routeName).siblings().contains('Configure').click();
// ensure it has already changed to edit page
cy.get(this.domSelector.name).should('value', this.data.routeName);
cy.contains('Next').click({ force: true });
cy.contains('Next').click({
force: true
});
cy.get(this.domSelector.nodes_0_host).should('value', this.data.ip2);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,14 @@ context('Edit Service with Upstream', () => {
cy.contains('Search').click();
cy.contains(this.data.serviceName).siblings().contains('Configure').click();

cy.get(this.domSelector.nodes_0_host).click({ force: true }).should('value', this.data.ip1);
cy.wait(500);
cy.get(this.domSelector.nodes_0_host).click({
force: true
}).should('value', this.data.ip1);
cy.get(this.domSelector.input).should('be.disabled');

cy.get(this.domSelector.upstreamSelector).click();
cy.contains('Custom').click();
cy.contains('.ant-select-item-option-content', 'Custom').click();
cy.get(this.domSelector.nodes_0_host).should('not.be.disabled').clear().type(this.data.ip2);
cy.get(this.domSelector.nodes_0_port).type(this.data.port);
cy.get(this.domSelector.nodes_0_weight).type(this.data.weight);
Expand Down
129 changes: 67 additions & 62 deletions web/src/components/Upstream/UpstreamForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Divider, Form, Switch } from 'antd';
import { Divider, Form, notification, Switch } from 'antd';
import React, { useState, forwardRef, useImperativeHandle, useEffect } from 'react';
import { useIntl } from 'umi';
import type { FormInstance } from 'antd/es/form';

import PanelSection from '@/components/PanelSection';
import { transformRequest } from '@/pages/Upstream/transform';
import PassiveCheck from './components/passive-check';
import ActiveCheck from './components/active-check'
import Nodes from './components/Nodes'
Expand All @@ -31,7 +30,7 @@ import UpstreamSelector from './components/UpstreamSelector';
import Retries from './components/Retries';
import PassHost from './components/PassHost';
import TLSComponent from './components/TLS';
import { transformUpstreamDataFromRequest } from './service';
import { convertToRequestData } from './service';

type Upstream = {
name?: string;
Expand All @@ -46,14 +45,18 @@ type Props = {
// FIXME: use proper typing
ref?: any;
required?: boolean;
neverReadonly?: boolean
};

/**
* UpstreamForm is used to reuse Upstream Form UI,
* before using this component, we need to execute the following command:
* form.setFieldsValue(convertToFormData(VALUE_FROM_API))
*/
const UpstreamForm: React.FC<Props> = forwardRef(
({ form, disabled, list = [], showSelector, required = true }, ref) => {
({ form, disabled = false, list = [], showSelector = false, required = true, neverReadonly = false }, ref) => {
const { formatMessage } = useIntl();
const [readonly, setReadonly] = useState(
Boolean(form.getFieldValue('upstream_id')) || disabled,
);
const [readonly, setReadonly] = useState(false);
const [hiddenForm, setHiddenForm] = useState(false);

const timeoutFields = [
Expand All @@ -75,39 +78,58 @@ const UpstreamForm: React.FC<Props> = forwardRef(
];

useImperativeHandle(ref, () => ({
getData: () => transformRequest(form.getFieldsValue()),
getData: () => convertToRequestData(form.getFieldsValue()),
}));

useEffect(() => {
const formData = transformRequest(form.getFieldsValue()) || {};
const { upstream_id } = form.getFieldsValue();
const resetForm = (upstream_id: string) => {
if (upstream_id === undefined) {
return
}

if (!neverReadonly) {
setReadonly(!["Custom", "None"].includes(upstream_id) || disabled);
}

/**
* upstream_id === None <==> required === false
* No need to bind Upstream object.
* When creating Route and binds with a Service, no need to configure Upstream in Route.
*/
if (upstream_id === 'None') {
setHiddenForm(true);
if (required) {
requestAnimationFrame(() => {
form.resetFields();
setHiddenForm(false);
});
}
} else {
if (upstream_id) {
requestAnimationFrame(() => {
const targetData = list.find((item) => item.id === upstream_id) as UpstreamComponent.ResponseData
if (targetData) {
form.setFieldsValue(transformUpstreamDataFromRequest(targetData));
}
});
}
if (!required && !Object.keys(formData).length) {
requestAnimationFrame(() => {
form.setFieldsValue({ upstream_id: 'None' });
setHiddenForm(true);
});
}
form.resetFields()
form.setFieldsValue({ upstream_id: 'None' })
return
}

setHiddenForm(false)

// NOTE: Use Ant Design's form object to set data automatically
if (upstream_id === "Custom") {
return
}

// NOTE: Set data from Upstream List (Upstream Selector)
if (list.length === 0) {
return
}
form.resetFields()
const targetData = list.find((item) => item.id === upstream_id) as UpstreamComponent.ResponseData
if (targetData) {
form.setFieldsValue(targetData);
}
setReadonly(Boolean(upstream_id) || disabled);
}, [list]);
}

/**
* upstream_id
* - None: No need to bind Upstream to a resource (e.g Service).
* - Custom: Users could input values on UpstreamForm
* - Upstream ID from API
*/
useEffect(() => {
const upstream_id = form.getFieldValue('upstream_id');
resetForm(upstream_id)
}, [form.getFieldValue('upstream_id'), list]);

const ActiveHealthCheck = () => (
<React.Fragment>
Expand Down Expand Up @@ -192,19 +214,26 @@ const UpstreamForm: React.FC<Props> = forwardRef(
}
</Form.Item>
<Divider orientation="left" plain />
<Form.Item label={formatMessage({ id: 'page.upstream.step.healthyCheck.passive' })} name={['custom', 'checks', 'passive']} valuePropName="checked">
<Form.Item label={formatMessage({ id: 'page.upstream.step.healthyCheck.passive' })} name={['custom', 'checks', 'passive']} valuePropName="checked" tooltip={formatMessage({ id: 'component.upstream.other.health-check.passive-only' })}>
<Switch disabled={readonly} />
</Form.Item>
<Form.Item shouldUpdate noStyle>
<Form.Item shouldUpdate={(prev, next) => prev.custom?.checks?.passive !== next.custom?.checks?.passive} noStyle>
{
() => {
const passive = form.getFieldValue(['custom', 'checks', 'passive'])
const active = form.getFieldValue(['custom', 'checks', 'active'])
if (passive) {
/*
* When enable passive check, we should enable active check, too.
* When we use form.setFieldsValue to enable active check, error throws.
* We choose to alert users first, and need users to enable active check manually.
*/
if (!active) {
notification.warn({
message: formatMessage({ id: 'component.upstream.other.health-check.invalid' }),
description: formatMessage({ id: 'component.upstream.other.health-check.passive-only' })
})
}
return <PassiveHealthCheck />
}
return null
Expand All @@ -225,32 +254,8 @@ const UpstreamForm: React.FC<Props> = forwardRef(
list={list}
disabled={disabled}
required={required}
shouldUpdate={(prev, next) => {
setReadonly(Boolean(next.upstream_id));
if (prev.upstream_id !== next.upstream_id) {
const id = next.upstream_id;
if (id) {
const targetData = list.find((item) => item.id === id) as UpstreamComponent.ResponseData
if (targetData) {
form.setFieldsValue(transformUpstreamDataFromRequest(targetData));
}
form.setFieldsValue({
upstream_id: id,
});
}
}
return prev.upstream_id !== next.upstream_id;
}}
onChange={(upstream_id) => {
setReadonly(Boolean(upstream_id));
setHiddenForm(Boolean(upstream_id === 'None'));
const targetData = list.find((item) => item.id === upstream_id) as UpstreamComponent.ResponseData
if (targetData) {
form.setFieldsValue(transformUpstreamDataFromRequest(targetData));
}
if (upstream_id === '') {
form.resetFields();
}
onChange={(nextUpstreamId) => {
resetForm(nextUpstreamId);
}}
/>
)}
Expand Down
10 changes: 9 additions & 1 deletion web/src/components/Upstream/components/PassHost.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/
import React from 'react'
import { Form, Input, Select } from 'antd'
import { Form, Input, notification, Select } from 'antd'
import { useIntl } from 'umi'
import type { FormInstance } from 'antd/lib/form'

Expand Down Expand Up @@ -79,6 +79,14 @@ const Component: React.FC<Props> = ({ form, readonly }) => {
</Form.Item>
);
}

if (form.getFieldValue('pass_host') === 'node' && (form.getFieldValue('nodes') || []).length !== 1) {
notification.warning({
message: formatMessage({id: 'component.upstream.other.pass_host-with-multiple-nodes.title'}),
description: formatMessage({id: 'component.upstream.other.pass_host-with-multiple-nodes'})
})
form.setFieldsValue({pass_host: 'pass'})
}
return null;
}}
</Form.Item>
Expand Down
10 changes: 4 additions & 6 deletions web/src/components/Upstream/components/UpstreamSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,16 @@ type Props = {
list?: Upstream[];
disabled?: boolean;
required?: boolean;
shouldUpdate: (prev: any, next: any) => void;
onChange: (id: string) => void
}

const Component: React.FC<Props> = ({ shouldUpdate, onChange, list = [], disabled, required }) => {
const UpstreamSelector: React.FC<Props> = ({ onChange, list = [], disabled, required }) => {
const { formatMessage } = useIntl()

return (
<Form.Item
label={formatMessage({ id: 'page.upstream.step.select.upstream' })}
name="upstream_id"
shouldUpdate={shouldUpdate as any}
>
<Select
showSearch
Expand All @@ -49,11 +47,11 @@ const Component: React.FC<Props> = ({ shouldUpdate, onChange, list = [], disable
item?.children.toLowerCase().includes(input.toLowerCase())
}
>
{Boolean(!required) && <Select.Option value={'None'}>None</Select.Option>}
<Select.Option value="None" disabled={required}>{formatMessage({id: 'component.upstream.other.none'})}</Select.Option>
{[
{
name: formatMessage({ id: 'page.upstream.step.select.upstream.select.option' }),
id: '',
id: 'Custom',
},
...list,
].map((item) => (
Expand All @@ -66,4 +64,4 @@ const Component: React.FC<Props> = ({ shouldUpdate, onChange, list = [], disable
)
}

export default Component
export default UpstreamSelector
5 changes: 0 additions & 5 deletions web/src/components/Upstream/components/active-check/Host.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,12 @@ const Component: React.FC<Props> = ({ readonly }) => {
return (
<Form.Item
label={formatMessage({ id: 'component.upstream.fields.checks.active.host' })}
required
tooltip={formatMessage({ id: 'component.upstream.fields.checks.active.host.tooltip' })}
style={{ marginBottom: 0 }}
>
<Form.Item
name={['checks', 'active', 'host']}
rules={[
{
required: true,
message: formatMessage({ id: 'component.upstream.fields.checks.active.host.required' }),
},
{
pattern: new RegExp(
/^\\*?[0-9a-zA-Z-._]+$/,
Expand Down
Loading