Skip to content

Commit 004a506

Browse files
authored
Merge pull request #2543 from murgatroid99/grpc-js_outlier_detection_config_parsing
grpc-js: Add config parsing tests and fix outlier detection config parsing
2 parents aa905bf + d7c27fb commit 004a506

File tree

2 files changed

+226
-12
lines changed

2 files changed

+226
-12
lines changed

packages/grpc-js/src/load-balancer-outlier-detection.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ function validateFieldType(
107107
expectedType: TypeofValues,
108108
objectName?: string
109109
) {
110-
if (fieldName in obj && typeof obj[fieldName] !== expectedType) {
110+
if (fieldName in obj && obj[fieldName] !== undefined && typeof obj[fieldName] !== expectedType) {
111111
const fullFieldName = objectName ? `${objectName}.${fieldName}` : fieldName;
112112
throw new Error(
113113
`outlier detection config ${fullFieldName} parse error: expected ${expectedType}, got ${typeof obj[
@@ -123,7 +123,7 @@ function validatePositiveDuration(
123123
objectName?: string
124124
) {
125125
const fullFieldName = objectName ? `${objectName}.${fieldName}` : fieldName;
126-
if (fieldName in obj) {
126+
if (fieldName in obj && obj[fieldName] !== undefined) {
127127
if (!isDuration(obj[fieldName])) {
128128
throw new Error(
129129
`outlier detection config ${fullFieldName} parse error: expected Duration, got ${typeof obj[
@@ -149,7 +149,7 @@ function validatePositiveDuration(
149149
function validatePercentage(obj: any, fieldName: string, objectName?: string) {
150150
const fullFieldName = objectName ? `${objectName}.${fieldName}` : fieldName;
151151
validateFieldType(obj, fieldName, 'number', objectName);
152-
if (fieldName in obj && !(obj[fieldName] >= 0 && obj[fieldName] <= 100)) {
152+
if (fieldName in obj && obj[fieldName] !== undefined && !(obj[fieldName] >= 0 && obj[fieldName] <= 100)) {
153153
throw new Error(
154154
`outlier detection config ${fullFieldName} parse error: value out of range for percentage (0-100)`
155155
);
@@ -201,13 +201,15 @@ export class OutlierDetectionLoadBalancingConfig
201201
}
202202
toJsonObject(): object {
203203
return {
204-
interval: msToDuration(this.intervalMs),
205-
base_ejection_time: msToDuration(this.baseEjectionTimeMs),
206-
max_ejection_time: msToDuration(this.maxEjectionTimeMs),
207-
max_ejection_percent: this.maxEjectionPercent,
208-
success_rate_ejection: this.successRateEjection,
209-
failure_percentage_ejection: this.failurePercentageEjection,
210-
child_policy: [this.childPolicy.toJsonObject()]
204+
outlier_detection: {
205+
interval: msToDuration(this.intervalMs),
206+
base_ejection_time: msToDuration(this.baseEjectionTimeMs),
207+
max_ejection_time: msToDuration(this.maxEjectionTimeMs),
208+
max_ejection_percent: this.maxEjectionPercent,
209+
success_rate_ejection: this.successRateEjection ?? undefined,
210+
failure_percentage_ejection: this.failurePercentageEjection ?? undefined,
211+
child_policy: [this.childPolicy.toJsonObject()]
212+
}
211213
};
212214
}
213215

@@ -238,7 +240,7 @@ export class OutlierDetectionLoadBalancingConfig
238240
validatePositiveDuration(obj, 'base_ejection_time');
239241
validatePositiveDuration(obj, 'max_ejection_time');
240242
validatePercentage(obj, 'max_ejection_percent');
241-
if ('success_rate_ejection' in obj) {
243+
if ('success_rate_ejection' in obj && obj.success_rate_ejection !== undefined) {
242244
if (typeof obj.success_rate_ejection !== 'object') {
243245
throw new Error(
244246
'outlier detection config success_rate_ejection must be an object'
@@ -268,7 +270,7 @@ export class OutlierDetectionLoadBalancingConfig
268270
'success_rate_ejection'
269271
);
270272
}
271-
if ('failure_percentage_ejection' in obj) {
273+
if ('failure_percentage_ejection' in obj && obj.failure_percentage_ejection !== undefined) {
272274
if (typeof obj.failure_percentage_ejection !== 'object') {
273275
throw new Error(
274276
'outlier detection config failure_percentage_ejection must be an object'
Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
/*
2+
* Copyright 2023 gRPC authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
*/
17+
18+
import { experimental } from '../src';
19+
import * as assert from 'assert';
20+
import parseLoadBalancingConfig = experimental.parseLoadBalancingConfig;
21+
22+
/**
23+
* Describes a test case for config parsing. input is passed to
24+
* parseLoadBalancingConfig. If error is set, the expectation is that that
25+
* operation throws an error with a matching message. Otherwise, toJsonObject
26+
* is called on the result, and it is expected to match output, or input if
27+
* output is unset.
28+
*/
29+
interface TestCase {
30+
name: string;
31+
input: object,
32+
output?: object;
33+
error?: RegExp;
34+
}
35+
36+
/* The main purpose of these tests is to verify that configs that are expected
37+
* to be valid parse successfully, and configs that are expected to be invalid
38+
* throw errors. The specific output of this parsing is a lower priority
39+
* concern.
40+
* Note: some tests have an expected output that is different from the output,
41+
* but all non-error tests additionally verify that parsing the output again
42+
* produces the same output. */
43+
const allTestCases: {[lbPolicyName: string]: TestCase[]} = {
44+
pick_first: [
45+
{
46+
name: 'no fields set',
47+
input: {},
48+
output: {
49+
shuffleAddressList: false
50+
}
51+
},
52+
{
53+
name: 'shuffleAddressList set',
54+
input: {
55+
shuffleAddressList: true
56+
}
57+
}
58+
],
59+
round_robin: [
60+
{
61+
name: 'no fields set',
62+
input: {}
63+
}
64+
],
65+
outlier_detection: [
66+
{
67+
name: 'only required fields set',
68+
input: {
69+
child_policy: [{round_robin: {}}]
70+
},
71+
output: {
72+
interval: {
73+
seconds: 10,
74+
nanos: 0
75+
},
76+
base_ejection_time: {
77+
seconds: 30,
78+
nanos: 0
79+
},
80+
max_ejection_time: {
81+
seconds: 300,
82+
nanos: 0
83+
},
84+
max_ejection_percent: 10,
85+
success_rate_ejection: undefined,
86+
failure_percentage_ejection: undefined,
87+
child_policy: [{round_robin: {}}]
88+
}
89+
},
90+
{
91+
name: 'all optional fields undefined',
92+
input: {
93+
interval: undefined,
94+
base_ejection_time: undefined,
95+
max_ejection_time: undefined,
96+
max_ejection_percent: undefined,
97+
success_rate_ejection: undefined,
98+
failure_percentage_ejection: undefined,
99+
child_policy: [{round_robin: {}}]
100+
},
101+
output: {
102+
interval: {
103+
seconds: 10,
104+
nanos: 0
105+
},
106+
base_ejection_time: {
107+
seconds: 30,
108+
nanos: 0
109+
},
110+
max_ejection_time: {
111+
seconds: 300,
112+
nanos: 0
113+
},
114+
max_ejection_percent: 10,
115+
success_rate_ejection: undefined,
116+
failure_percentage_ejection: undefined,
117+
child_policy: [{round_robin: {}}]
118+
}
119+
},
120+
{
121+
name: 'empty ejection configs',
122+
input: {
123+
success_rate_ejection: {},
124+
failure_percentage_ejection: {},
125+
child_policy: [{round_robin: {}}]
126+
},
127+
output: {
128+
interval: {
129+
seconds: 10,
130+
nanos: 0
131+
},
132+
base_ejection_time: {
133+
seconds: 30,
134+
nanos: 0
135+
},
136+
max_ejection_time: {
137+
seconds: 300,
138+
nanos: 0
139+
},
140+
max_ejection_percent: 10,
141+
success_rate_ejection: {
142+
stdev_factor: 1900,
143+
enforcement_percentage: 100,
144+
minimum_hosts: 5,
145+
request_volume: 100
146+
},
147+
failure_percentage_ejection: {
148+
threshold: 85,
149+
enforcement_percentage: 100,
150+
minimum_hosts: 5,
151+
request_volume: 50,
152+
},
153+
child_policy: [{round_robin: {}}]
154+
}
155+
},
156+
{
157+
name: 'all fields populated',
158+
input: {
159+
interval: {
160+
seconds: 20,
161+
nanos: 0
162+
},
163+
base_ejection_time: {
164+
seconds: 40,
165+
nanos: 0
166+
},
167+
max_ejection_time: {
168+
seconds: 400,
169+
nanos: 0
170+
},
171+
max_ejection_percent: 20,
172+
success_rate_ejection: {
173+
stdev_factor: 1800,
174+
enforcement_percentage: 90,
175+
minimum_hosts: 4,
176+
request_volume: 200
177+
},
178+
failure_percentage_ejection: {
179+
threshold: 95,
180+
enforcement_percentage: 90,
181+
minimum_hosts: 4,
182+
request_volume: 60,
183+
},
184+
child_policy: [{round_robin: {}}]
185+
186+
}
187+
}
188+
]
189+
}
190+
191+
describe('Load balancing policy config parsing', () => {
192+
for (const [lbPolicyName, testCases] of Object.entries(allTestCases)) {
193+
describe(lbPolicyName, () => {
194+
for (const testCase of testCases) {
195+
it(testCase.name, () => {
196+
const lbConfigInput = {[lbPolicyName]: testCase.input};
197+
if (testCase.error) {
198+
assert.throws(() => {
199+
parseLoadBalancingConfig(lbConfigInput);
200+
}, testCase.error);
201+
} else {
202+
const expectedOutput = testCase.output ?? testCase.input;
203+
const parsedJson = parseLoadBalancingConfig(lbConfigInput).toJsonObject();
204+
assert.deepStrictEqual(parsedJson, {[lbPolicyName]: expectedOutput});
205+
// Test idempotency
206+
assert.deepStrictEqual(parseLoadBalancingConfig(parsedJson).toJsonObject(), parsedJson);
207+
}
208+
});
209+
}
210+
});
211+
}
212+
});

0 commit comments

Comments
 (0)