Skip to content

Commit 5e46f60

Browse files
juzhiyuanLiteSun
andauthored
fix: show correct health checker (#1784)
Co-authored-by: litesun <[email protected]>
1 parent f57ad4e commit 5e46f60

File tree

29 files changed

+297
-293
lines changed

29 files changed

+297
-293
lines changed

web/cypress/integration/route/can-skip-upstream-when-select-service-id.spec.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ context('Can select service_id skip upstream in route', () => {
6161
cy.get(this.domSelector.name).type(this.data.routeName);
6262
cy.contains('Next').click();
6363
cy.get(this.domSelector.upstreamSelector).click();
64-
cy.contains('None').should('not.exist');
64+
cy.get('.ant-select-item-option-disabled > .ant-select-item-option-content').contains('None');
6565

6666
cy.contains('Previous').click();
6767
cy.wait(500);
@@ -91,31 +91,34 @@ context('Can select service_id skip upstream in route', () => {
9191
cy.contains(this.data.routeName).siblings().contains('Configure').click();
9292
cy.get(this.domSelector.serviceSelector).click();
9393
cy.contains('None').click();
94+
cy.get(this.domSelector.notification).should('contain', 'Please check the configuration of binding service');
95+
cy.get(this.domSelector.notificationCloseIcon).click();
96+
9497
cy.contains('Next').click();
95-
cy.get(this.domSelector.upstream_id).click();
96-
cy.contains('None').should('not.exist');
98+
cy.wait(500);
99+
cy.get('[data-cy=upstream_selector]').click();
97100
cy.contains(this.data.upstreamName).click();
98101
cy.contains('Next').click();
99102
cy.contains('Next').click();
100103
cy.contains('Submit').click();
101104
cy.contains(this.data.submitSuccess);
102105
});
103106

104-
it('should delete upstream, service and route', function () {
105-
cy.visit('/');
106-
cy.contains('Service').click();
107-
cy.contains(this.data.serviceName).siblings().contains('Delete').click();
108-
cy.contains('button', 'Confirm').click();
109-
cy.get(this.domSelector.notification).should('contain', this.data.deleteServiceSuccess);
110-
107+
it('should delete route, service and upstream', function () {
111108
cy.visit('/');
112109
cy.contains('Route').click();
113110
cy.contains(this.data.routeName).siblings().contains('More').click();
114111
cy.contains('Delete').click();
115112
cy.get(this.domSelector.deleteAlert).should('be.visible').within(() => {
116113
cy.contains('OK').click();
117114
});
115+
118116
cy.get(this.domSelector.notification).should('contain', this.data.deleteRouteSuccess);
117+
cy.visit('/');
118+
cy.contains('Service').click();
119+
cy.contains(this.data.serviceName).siblings().contains('Delete').click();
120+
cy.contains('button', 'Confirm').click();
121+
cy.get(this.domSelector.notification).should('contain', this.data.deleteServiceSuccess);
119122

120123
cy.visit('/');
121124
cy.contains('Upstream').click();

web/cypress/integration/route/create-route-with-upstream.spec.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ context('Create Route with Upstream', () => {
5454
cy.get(this.domSelector.input).should('be.disabled');
5555
// should enable Upstream input boxes after selecting Custom mode
5656
cy.get(this.domSelector.upstreamSelector).click();
57-
cy.contains('Custom').click();
58-
cy.get(this.domSelector.input).should('not.be.disabled');
57+
cy.contains('.ant-select-item-option-content', 'Custom').click();
5958

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

7978
cy.get(this.domSelector.name).should('value', this.data.routeName);
80-
cy.contains('Next').click({ force: true });
79+
cy.contains('Next').click({
80+
force: true
81+
});
8182

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

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

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

web/cypress/integration/service/edit-service-with-upstream.spec.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,14 @@ context('Edit Service with Upstream', () => {
6363
cy.contains('Search').click();
6464
cy.contains(this.data.serviceName).siblings().contains('Configure').click();
6565

66-
cy.get(this.domSelector.nodes_0_host).click({ force: true }).should('value', this.data.ip1);
66+
cy.wait(500);
67+
cy.get(this.domSelector.nodes_0_host).click({
68+
force: true
69+
}).should('value', this.data.ip1);
6770
cy.get(this.domSelector.input).should('be.disabled');
6871

6972
cy.get(this.domSelector.upstreamSelector).click();
70-
cy.contains('Custom').click();
73+
cy.contains('.ant-select-item-option-content', 'Custom').click();
7174
cy.get(this.domSelector.nodes_0_host).should('not.be.disabled').clear().type(this.data.ip2);
7275
cy.get(this.domSelector.nodes_0_port).type(this.data.port);
7376
cy.get(this.domSelector.nodes_0_weight).type(this.data.weight);

web/src/components/Upstream/UpstreamForm.tsx

Lines changed: 67 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,12 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
import { Divider, Form, Switch } from 'antd';
17+
import { Divider, Form, notification, Switch } from 'antd';
1818
import React, { useState, forwardRef, useImperativeHandle, useEffect } from 'react';
1919
import { useIntl } from 'umi';
2020
import type { FormInstance } from 'antd/es/form';
2121

2222
import PanelSection from '@/components/PanelSection';
23-
import { transformRequest } from '@/pages/Upstream/transform';
2423
import PassiveCheck from './components/passive-check';
2524
import ActiveCheck from './components/active-check'
2625
import Nodes from './components/Nodes'
@@ -31,7 +30,7 @@ import UpstreamSelector from './components/UpstreamSelector';
3130
import Retries from './components/Retries';
3231
import PassHost from './components/PassHost';
3332
import TLSComponent from './components/TLS';
34-
import { transformUpstreamDataFromRequest } from './service';
33+
import { convertToRequestData } from './service';
3534

3635
type Upstream = {
3736
name?: string;
@@ -46,14 +45,18 @@ type Props = {
4645
// FIXME: use proper typing
4746
ref?: any;
4847
required?: boolean;
48+
neverReadonly?: boolean
4949
};
5050

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

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

7780
useImperativeHandle(ref, () => ({
78-
getData: () => transformRequest(form.getFieldsValue()),
81+
getData: () => convertToRequestData(form.getFieldsValue()),
7982
}));
8083

81-
useEffect(() => {
82-
const formData = transformRequest(form.getFieldsValue()) || {};
83-
const { upstream_id } = form.getFieldsValue();
84+
const resetForm = (upstream_id: string) => {
85+
if (upstream_id === undefined) {
86+
return
87+
}
8488

89+
if (!neverReadonly) {
90+
setReadonly(!["Custom", "None"].includes(upstream_id) || disabled);
91+
}
92+
93+
/**
94+
* upstream_id === None <==> required === false
95+
* No need to bind Upstream object.
96+
* When creating Route and binds with a Service, no need to configure Upstream in Route.
97+
*/
8598
if (upstream_id === 'None') {
8699
setHiddenForm(true);
87-
if (required) {
88-
requestAnimationFrame(() => {
89-
form.resetFields();
90-
setHiddenForm(false);
91-
});
92-
}
93-
} else {
94-
if (upstream_id) {
95-
requestAnimationFrame(() => {
96-
const targetData = list.find((item) => item.id === upstream_id) as UpstreamComponent.ResponseData
97-
if (targetData) {
98-
form.setFieldsValue(transformUpstreamDataFromRequest(targetData));
99-
}
100-
});
101-
}
102-
if (!required && !Object.keys(formData).length) {
103-
requestAnimationFrame(() => {
104-
form.setFieldsValue({ upstream_id: 'None' });
105-
setHiddenForm(true);
106-
});
107-
}
100+
form.resetFields()
101+
form.setFieldsValue({ upstream_id: 'None' })
102+
return
103+
}
104+
105+
setHiddenForm(false)
106+
107+
// NOTE: Use Ant Design's form object to set data automatically
108+
if (upstream_id === "Custom") {
109+
return
110+
}
111+
112+
// NOTE: Set data from Upstream List (Upstream Selector)
113+
if (list.length === 0) {
114+
return
115+
}
116+
form.resetFields()
117+
const targetData = list.find((item) => item.id === upstream_id) as UpstreamComponent.ResponseData
118+
if (targetData) {
119+
form.setFieldsValue(targetData);
108120
}
109-
setReadonly(Boolean(upstream_id) || disabled);
110-
}, [list]);
121+
}
122+
123+
/**
124+
* upstream_id
125+
* - None: No need to bind Upstream to a resource (e.g Service).
126+
* - Custom: Users could input values on UpstreamForm
127+
* - Upstream ID from API
128+
*/
129+
useEffect(() => {
130+
const upstream_id = form.getFieldValue('upstream_id');
131+
resetForm(upstream_id)
132+
}, [form.getFieldValue('upstream_id'), list]);
111133

112134
const ActiveHealthCheck = () => (
113135
<React.Fragment>
@@ -192,19 +214,26 @@ const UpstreamForm: React.FC<Props> = forwardRef(
192214
}
193215
</Form.Item>
194216
<Divider orientation="left" plain />
195-
<Form.Item label={formatMessage({ id: 'page.upstream.step.healthyCheck.passive' })} name={['custom', 'checks', 'passive']} valuePropName="checked">
217+
<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' })}>
196218
<Switch disabled={readonly} />
197219
</Form.Item>
198-
<Form.Item shouldUpdate noStyle>
220+
<Form.Item shouldUpdate={(prev, next) => prev.custom?.checks?.passive !== next.custom?.checks?.passive} noStyle>
199221
{
200222
() => {
201223
const passive = form.getFieldValue(['custom', 'checks', 'passive'])
224+
const active = form.getFieldValue(['custom', 'checks', 'active'])
202225
if (passive) {
203226
/*
204227
* When enable passive check, we should enable active check, too.
205228
* When we use form.setFieldsValue to enable active check, error throws.
206229
* We choose to alert users first, and need users to enable active check manually.
207230
*/
231+
if (!active) {
232+
notification.warn({
233+
message: formatMessage({ id: 'component.upstream.other.health-check.invalid' }),
234+
description: formatMessage({ id: 'component.upstream.other.health-check.passive-only' })
235+
})
236+
}
208237
return <PassiveHealthCheck />
209238
}
210239
return null
@@ -225,32 +254,8 @@ const UpstreamForm: React.FC<Props> = forwardRef(
225254
list={list}
226255
disabled={disabled}
227256
required={required}
228-
shouldUpdate={(prev, next) => {
229-
setReadonly(Boolean(next.upstream_id));
230-
if (prev.upstream_id !== next.upstream_id) {
231-
const id = next.upstream_id;
232-
if (id) {
233-
const targetData = list.find((item) => item.id === id) as UpstreamComponent.ResponseData
234-
if (targetData) {
235-
form.setFieldsValue(transformUpstreamDataFromRequest(targetData));
236-
}
237-
form.setFieldsValue({
238-
upstream_id: id,
239-
});
240-
}
241-
}
242-
return prev.upstream_id !== next.upstream_id;
243-
}}
244-
onChange={(upstream_id) => {
245-
setReadonly(Boolean(upstream_id));
246-
setHiddenForm(Boolean(upstream_id === 'None'));
247-
const targetData = list.find((item) => item.id === upstream_id) as UpstreamComponent.ResponseData
248-
if (targetData) {
249-
form.setFieldsValue(transformUpstreamDataFromRequest(targetData));
250-
}
251-
if (upstream_id === '') {
252-
form.resetFields();
253-
}
257+
onChange={(nextUpstreamId) => {
258+
resetForm(nextUpstreamId);
254259
}}
255260
/>
256261
)}

web/src/components/Upstream/components/PassHost.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* limitations under the License.
1616
*/
1717
import React from 'react'
18-
import { Form, Input, Select } from 'antd'
18+
import { Form, Input, notification, Select } from 'antd'
1919
import { useIntl } from 'umi'
2020
import type { FormInstance } from 'antd/lib/form'
2121

@@ -79,6 +79,14 @@ const Component: React.FC<Props> = ({ form, readonly }) => {
7979
</Form.Item>
8080
);
8181
}
82+
83+
if (form.getFieldValue('pass_host') === 'node' && (form.getFieldValue('nodes') || []).length !== 1) {
84+
notification.warning({
85+
message: formatMessage({id: 'component.upstream.other.pass_host-with-multiple-nodes.title'}),
86+
description: formatMessage({id: 'component.upstream.other.pass_host-with-multiple-nodes'})
87+
})
88+
form.setFieldsValue({pass_host: 'pass'})
89+
}
8290
return null;
8391
}}
8492
</Form.Item>

web/src/components/Upstream/components/UpstreamSelector.tsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,16 @@ type Props = {
2727
list?: Upstream[];
2828
disabled?: boolean;
2929
required?: boolean;
30-
shouldUpdate: (prev: any, next: any) => void;
3130
onChange: (id: string) => void
3231
}
3332

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

3736
return (
3837
<Form.Item
3938
label={formatMessage({ id: 'page.upstream.step.select.upstream' })}
4039
name="upstream_id"
41-
shouldUpdate={shouldUpdate as any}
4240
>
4341
<Select
4442
showSearch
@@ -49,11 +47,11 @@ const Component: React.FC<Props> = ({ shouldUpdate, onChange, list = [], disable
4947
item?.children.toLowerCase().includes(input.toLowerCase())
5048
}
5149
>
52-
{Boolean(!required) && <Select.Option value={'None'}>None</Select.Option>}
50+
<Select.Option value="None" disabled={required}>{formatMessage({id: 'component.upstream.other.none'})}</Select.Option>
5351
{[
5452
{
5553
name: formatMessage({ id: 'page.upstream.step.select.upstream.select.option' }),
56-
id: '',
54+
id: 'Custom',
5755
},
5856
...list,
5957
].map((item) => (
@@ -66,4 +64,4 @@ const Component: React.FC<Props> = ({ shouldUpdate, onChange, list = [], disable
6664
)
6765
}
6866

69-
export default Component
67+
export default UpstreamSelector

web/src/components/Upstream/components/active-check/Host.tsx

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,12 @@ const Component: React.FC<Props> = ({ readonly }) => {
2727
return (
2828
<Form.Item
2929
label={formatMessage({ id: 'component.upstream.fields.checks.active.host' })}
30-
required
3130
tooltip={formatMessage({ id: 'component.upstream.fields.checks.active.host.tooltip' })}
3231
style={{ marginBottom: 0 }}
3332
>
3433
<Form.Item
3534
name={['checks', 'active', 'host']}
3635
rules={[
37-
{
38-
required: true,
39-
message: formatMessage({ id: 'component.upstream.fields.checks.active.host.required' }),
40-
},
4136
{
4237
pattern: new RegExp(
4338
/^\\*?[0-9a-zA-Z-._]+$/,

0 commit comments

Comments
 (0)