From e8bbcba31d5a32f33490b4530c35f31459061e45 Mon Sep 17 00:00:00 2001 From: dblock Date: Tue, 27 Aug 2024 14:36:50 -0400 Subject: [PATCH] Added schema for cluster routing awareness. Signed-off-by: dblock --- CHANGELOG.md | 1 + TESTING_GUIDE.md | 2 ++ spec/namespaces/cluster.yaml | 19 ++++++++-- spec/schemas/cluster.weighted_routing.yaml | 23 ++++++++++++ .../cluster/routing/awareness/weights.yaml | 36 +++++++++++++++++++ tools/src/tester/ChapterEvaluator.ts | 9 ++--- tools/src/tester/ChapterOutput.ts | 6 +++- tools/src/tester/StoryOutputs.ts | 2 +- tools/src/tester/types/eval.types.ts | 11 ++++-- tools/tests/tester/ChapterOutput.test.ts | 15 +++++++- tools/tests/tester/StoryOutputs.test.ts | 7 +++- 11 files changed, 118 insertions(+), 13 deletions(-) create mode 100644 spec/schemas/cluster.weighted_routing.yaml create mode 100644 tests/default/cluster/routing/awareness/weights.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index b63c1c14c..a0ba1fcf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Added 404 responses to `/_alias/{name}` and `/{index}/_alias/{name}` ([#519](https://github.com/opensearch-project/opensearch-api-specification/pull/519)) - Added `asynchronous_search` ([#525](https://github.com/opensearch-project/opensearch-api-specification/pull/525)) - Added `DELETE /_plugins/_ml/tasks/{task_id}` ([#530](https://github.com/opensearch-project/opensearch-api-specification/pull/530)) +- Added request and response schemas for `/_cluster/routing/awareness/{attribute}/weights` ([#524](https://github.com/opensearch-project/opensearch-api-specification/pull/524)) ### Changed diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index 852d3844b..1b47e8686 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -201,6 +201,8 @@ Consider the following chapters in [ml/model_groups](tests/default/ml/model_grou ``` As you can see, the `output` field in the first chapter saves the `model_group_id` from the response body. This value is then used in the subsequent chapters to query and delete the model group. +You can also supply defaults in both output and input values, e.g. `version: payload.version ? 0` or `${weights._version ? 0}` used in [cluster/decommission/awareness.yaml](tests/default/cluster/decommission/awareness.yaml). + You can also reuse output in payload expectations. See [tests/plugins/index_state_management/nodes/plugins/index_state_management.yaml](tests/plugins/index_state_management/nodes/plugins/index_state_management.yaml) for an example. ### Managing Versions diff --git a/spec/namespaces/cluster.yaml b/spec/namespaces/cluster.yaml index c503c375b..562b115bf 100644 --- a/spec/namespaces/cluster.yaml +++ b/spec/namespaces/cluster.yaml @@ -195,6 +195,8 @@ paths: url: https://opensearch.org/docs/latest/api-reference/cluster-api/cluster-awareness/#example-weighted-round-robin-search parameters: - $ref: '#/components/parameters/cluster.put_weighted_routing::path.attribute' + requestBody: + $ref: '#/components/requestBodies/cluster.put_weighted_routing' responses: '200': $ref: '#/components/responses/cluster.put_weighted_routing@200' @@ -465,6 +467,11 @@ paths: $ref: '#/components/responses/cluster.remote_info@200' components: requestBodies: + cluster.put_weighted_routing: + content: + application/json: + schema: + $ref: '../schemas/cluster.weighted_routing.yaml#/components/schemas/Weights' cluster.allocation_explain: content: application/json: @@ -647,7 +654,11 @@ components: required: - persistent - transient - cluster.get_weighted_routing@200: {} + cluster.get_weighted_routing@200: + content: + application/json: + schema: + $ref: '../schemas/cluster.weighted_routing.yaml#/components/schemas/WeightsResponse' cluster.health@200: content: application/json: @@ -692,7 +703,11 @@ components: - acknowledged - persistent - transient - cluster.put_weighted_routing@200: {} + cluster.put_weighted_routing@200: + content: + application/json: + schema: + type: object cluster.remote_info@200: content: application/json: diff --git a/spec/schemas/cluster.weighted_routing.yaml b/spec/schemas/cluster.weighted_routing.yaml new file mode 100644 index 000000000..c4579ba8e --- /dev/null +++ b/spec/schemas/cluster.weighted_routing.yaml @@ -0,0 +1,23 @@ +openapi: 3.1.0 +info: + title: Schemas of Weighted Routing Category + description: Schemas of weighted routing category. + version: 1.0.0 +paths: {} +components: + schemas: + Weights: + type: object + properties: + _version: + $ref: '_common.yaml#/components/schemas/VersionNumber' + weights: + type: object + WeightsResponse: + allOf: + - $ref: '#/components/schemas/Weights' + - type: object + properties: + discovered_cluster_manager: + type: boolean + diff --git a/tests/default/cluster/routing/awareness/weights.yaml b/tests/default/cluster/routing/awareness/weights.yaml new file mode 100644 index 000000000..788373149 --- /dev/null +++ b/tests/default/cluster/routing/awareness/weights.yaml @@ -0,0 +1,36 @@ +$schema: ../../../../../json_schemas/test_story.schema.yaml + +description: Test cluster routing settings. +version: '>= 2.16' +prologues: + - path: /_cluster/settings + method: PUT + request: + payload: + transient: + cluster.routing.allocation.awareness.attributes: zone + cluster.routing.allocation.awareness.force.zone.values: + - zone-a + - zone-b +chapters: + - synopsis: Get zone routing weights. + id: weights + path: /_cluster/routing/awareness/{attribute}/weights + method: GET + parameters: + attribute: zone + output: + version: payload._version ? -1 + - synopsis: Update zone routing weights. + path: /_cluster/routing/awareness/{attribute}/weights + method: PUT + parameters: + attribute: zone + request: + payload: + weights: + zone-a: '1' + zone-b: '0' + _version: ${weights.version} + response: + status: 200 diff --git a/tools/src/tester/ChapterEvaluator.ts b/tools/src/tester/ChapterEvaluator.ts index 6feb7d0a0..b31e8991b 100644 --- a/tools/src/tester/ChapterEvaluator.ts +++ b/tools/src/tester/ChapterEvaluator.ts @@ -7,7 +7,7 @@ * compatible open source license. */ -import { type Chapter, type ActualResponse, type Payload } from './types/story.types' +import { type Chapter, type ActualResponse, type Payload, Story } from './types/story.types' import { type ChapterEvaluation, type Evaluation, Result, EvaluationWithOutput } from './types/eval.types' import { type ParsedOperation } from './types/spec.types' import { overall_result } from './helpers' @@ -64,7 +64,7 @@ export default class ChapterEvaluator { async #evaluate(chapter: Chapter, operation: ParsedOperation, story_outputs: StoryOutputs, retries?: number): Promise { const response = await this._chapter_reader.read(chapter, story_outputs) const params = this.#evaluate_parameters(chapter, operation) - const request = this.#evaluate_request(chapter, operation) + const request = this.#evaluate_request(chapter, operation, story_outputs) const status = this.#evaluate_status(chapter, response) const payload_schema_evaluation = status.result === Result.PASSED ? this.#evaluate_payload_schema(chapter, response, operation) : { result: Result.SKIPPED } const output_values_evaluation: EvaluationWithOutput = status.result === Result.PASSED ? ChapterOutput.extract_output_values(response, chapter.output) : { evaluation: { result: Result.SKIPPED } } @@ -119,12 +119,13 @@ export default class ChapterEvaluator { })) } - #evaluate_request(chapter: Chapter, operation: ParsedOperation): Evaluation { + #evaluate_request(chapter: Chapter, operation: ParsedOperation, story_outputs: StoryOutputs): Evaluation { if (chapter.request?.payload === undefined) return { result: Result.PASSED } const content_type = chapter.request.content_type ?? APPLICATION_JSON const schema = operation.requestBody?.content[content_type]?.schema if (schema == null) return { result: Result.FAILED, message: `Schema for "${content_type}" request body not found in the spec.` } - return this._schema_validator.validate(schema, chapter.request?.payload ?? {}) + const payload = story_outputs.resolve_value(chapter.request?.payload) ?? {} + return this._schema_validator.validate(schema, payload) } #evaluate_status(chapter: Chapter, response: ActualResponse): Evaluation { diff --git a/tools/src/tester/ChapterOutput.ts b/tools/src/tester/ChapterOutput.ts index d8fd6d115..e3c9ae851 100644 --- a/tools/src/tester/ChapterOutput.ts +++ b/tools/src/tester/ChapterOutput.ts @@ -35,7 +35,11 @@ export class ChapterOutput { if (response.payload === undefined) { return { evaluation: { result: Result.ERROR, message: 'No payload found in response, but expected output: ' + path } } } - value = _.get(response, path) + const rhs = path.replaceAll(' ', '').split('?', 2) + let default_value: any = parseFloat(rhs[1]) + if (Number.isNaN(default_value)) default_value = rhs[1] + value = _.get(response, rhs[0]) + if (value === undefined) value = default_value if (value === undefined) { return { evaluation: { result: Result.ERROR, message: `Expected to find non undefined value at \`${path}\`.` } } } diff --git a/tools/src/tester/StoryOutputs.ts b/tools/src/tester/StoryOutputs.ts index 41b8d9e71..08171e451 100644 --- a/tools/src/tester/StoryOutputs.ts +++ b/tools/src/tester/StoryOutputs.ts @@ -50,7 +50,7 @@ export class StoryOutputs { resolve_string (str: string): any { const ref = OutputReference.parse(str) if (ref) { - return this.get_output_value(ref.chapter_id, ref.output_name) + return this.get_output_value(ref.chapter_id, ref.output_name) ?? ref.default_value } else { return str } diff --git a/tools/src/tester/types/eval.types.ts b/tools/src/tester/types/eval.types.ts index eacb2c62a..9474836c9 100644 --- a/tools/src/tester/types/eval.types.ts +++ b/tools/src/tester/types/eval.types.ts @@ -87,15 +87,20 @@ export enum Result { export class OutputReference { chapter_id: string output_name: string - private constructor (chapter_id: string, output_name: string) { + default_value?: string + private constructor (chapter_id: string, output_name: string, default_value?: string) { this.chapter_id = chapter_id this.output_name = output_name + this.default_value = default_value } static parse (str: string): OutputReference | undefined { if (str.startsWith('${') && str.endsWith('}')) { - const spl = str.slice(2, -1).split('.', 2) - return { chapter_id: spl[0], output_name: spl[1] } + const spl = str.slice(2, -1).replaceAll(' ', '').split('.', 2) + const rhs = spl[1].split('?', 2) + let default_value: any = parseFloat(rhs[1]) + if (Number.isNaN(default_value)) default_value = rhs[1] + return { chapter_id: spl[0], output_name: rhs[0], default_value } } return undefined } diff --git a/tools/tests/tester/ChapterOutput.test.ts b/tools/tests/tester/ChapterOutput.test.ts index f5ace6ed7..06be1beac 100644 --- a/tools/tests/tester/ChapterOutput.test.ts +++ b/tools/tests/tester/ChapterOutput.test.ts @@ -36,7 +36,8 @@ describe('with an object response', () => { { d: 2 }, { e: 3 } ] - } + }, + zero: 0 }) test('returns nested values', () => { @@ -68,6 +69,18 @@ describe('with an object response', () => { }) }) + test('uses a default value', () => { + expect(ChapterOutput.extract_output_values(response, { x: 'payload.a.b.x[0] ? -1' })).toEqual( + passed_output({ x: -1 }) + ) + }) + + test('does not use a default value on a numeric zero', () => { + expect(ChapterOutput.extract_output_values(response, { x: 'payload.zero ? -1' })).toEqual( + passed_output({ x: 0 }) + ) + }) + test('errors on invalid source', () => { expect(ChapterOutput.extract_output_values(response, { x: 'foobar' })).toEqual({ evaluation: { diff --git a/tools/tests/tester/StoryOutputs.test.ts b/tools/tests/tester/StoryOutputs.test.ts index 3017b7a66..450ddfd33 100644 --- a/tools/tests/tester/StoryOutputs.test.ts +++ b/tools/tests/tester/StoryOutputs.test.ts @@ -13,13 +13,18 @@ import { StoryOutputs } from 'tester/StoryOutputs' const story_outputs = new StoryOutputs({ chapter_id: new ChapterOutput({ x: 1, - y: 2 + y: 2, + n: null, + z: 0 }) }) test('resolve_string', () => { expect(story_outputs.resolve_string('${chapter_id.x}')).toEqual(1) expect(story_outputs.resolve_string('${invalid_id.x}')).toBeUndefined() + expect(story_outputs.resolve_string('${chapter_id.n ? x}')).toEqual('x') + expect(story_outputs.resolve_string('${chapter_id.n ? 0}')).toEqual(0) + expect(story_outputs.resolve_string('${chapter_id.z ? -1}')).toEqual(0) expect(story_outputs.resolve_string('some_str')).toEqual('some_str') })