Replace 'master' with 'cluster_manager' in 'GET Cat Nodes' API #2441

tlfeng · 2022-03-11T06:27:04Z

Description

Replace 'master' with 'cluster_manager' in the table header of GET Cat Nodes API, to promote inclusive language.
Add cm as the alias for the table header cluster_manager
Add master as the alias for the table header cluster_manager, for keeping compatibility when using GET _cat/nodes?v&h=master to show the specific column only.
Add YAML rest test

Current response of 'GET Cat Nodes' API:

http://localhost:9200/_cat/nodes?v

ip        heap.percent ram.percent cpu load_1m load_5m load_15m node.role master name
127.0.0.1           10         100  22    4.28                  dimr      *      runTask-0

Proposed response of 'GET Cat Health' API:

http://localhost:9200/_cat/nodes?v

ip        heap.percent ram.percent cpu load_1m load_5m load_15m node.role cluster_manager name
127.0.0.1           10         100  22    4.28                  dimr      *               runTask-0

Explanation on the API response compatibility
Because "cat API is a human-readable interface" (https://opensearch.org/docs/latest/opensearch/rest-api/cat/index/), and are not intended for use by software program, the old name "master" is not retained.

To keep the same API response with nodes in 1.x version, users can specify "header" in the request parameter, then the API response will remain the same for nodes in both 1.x and 2.x versions.
For example:

GET _cat/nodes?v&h=version&master

version master
2.0.0   *

Testing

./gradlew ':rest-api-spec:yamlRestTest' \
--tests "org.opensearch.test.rest.ClientYamlTestSuiteIT" \
-Dtests.method="test {p0=cat.nodes/10_basic/*}"

Issues Resolved

Part of #1549

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Tianli Feng <ftianli@amazon.com>

opensearch-ci-bot · 2022-03-11T06:28:02Z

Can one of the admins verify this patch?

opensearch-ci-bot · 2022-03-11T06:48:27Z

❌ Gradle Check failure 9cadbce
Log 3258

Reports 3258

tlfeng · 2022-03-11T17:41:20Z

In log 3258:
Because the table header of "_cat/nodes" will be changed in this PR, a method org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.readVersionsFromCatNodes(OpenSearchClientYamlSuiteTestCase.java:337) used for Yaml REST testing can not read the proper value for both old and new versions of OpenSearch.

> Task :qa:mixed-cluster:v1.4.0#mixedClusterTest

REPRODUCE WITH: ./gradlew ':qa:mixed-cluster:v1.4.0#mixedClusterTest' --tests "org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=search/110_field_collapsing/field collapsing and rescore}" -Dtests.seed=2308CADB0B4AEF70 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=no -Dtests.timezone=Europe/San_Marino -Druntime.java=17

org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT > test {p0=search/110_field_collapsing/field collapsing and rescore} FAILED
    java.lang.AssertionError: invalid line: 2.0.0 length: 1
        at __randomizedtesting.SeedInfo.seed([2308CADB0B4AEF70:AB5CF501A5B68288]:0)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.readVersionsFromCatNodes(OpenSearchClientYamlSuiteTestCase.java:337)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.initAndResetContext(OpenSearchClientYamlSuiteTestCase.java:145)
...

Signed-off-by: Tianli Feng <ftianli@amazon.com>

…ility Signed-off-by: Tianli Feng <ftianli@amazon.com>

Signed-off-by: Tianli Feng <ftianli@amazon.com>

opensearch-ci-bot · 2022-03-13T07:04:20Z

❌ Gradle Check failure 45e04ce
Log 3305

Reports 3305

tlfeng · 2022-03-13T07:08:01Z

In log 3305:

> Task :qa:mixed-cluster:v1.4.0#mixedClusterTest

REPRODUCE WITH: ./gradlew ':qa:mixed-cluster:v1.4.0#mixedClusterTest' --tests "org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT" -Dtests.method="test {p0=cat.nodes/10_basic/Test cat nodes output - before 2.0.0}" -Dtests.seed=C7989F860D60857C -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-LY -Dtests.timezone=Australia/Broken_Hill -Druntime.java=17

org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT > test {p0=cat.nodes/10_basic/Test cat nodes output - before 2.0.0} FAILED
    java.lang.AssertionError: Failure at [cat.nodes/10_basic:11]: field [$body] was expected to match the provided regex but didn't
    Expected: ^  ip                     \s+  heap\.percent   \s+  ram\.percent \s+ cpu      \s+ load_1m            \s+ load_5m            \s+ load_15m           \s+ node\.role              \s+  master   \s+   name  \n
       ((\d{1,3}\.){3}\d{1,3}  \s+  \d+             \s+  \d*          \s+ (-)?\d* \s+  ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ ((-)?\d*(\.\d+)?)? \s+ (-|[cdhilmrstvw]{1,11}) \s+  [-*x]    \s+   (\S+\s?)+     \n)+  $
         but: was "ip        heap.percent ram.percent cpu load_1m load_5m load_15m node.role cluster_manager name\n127.0.0.1           31          46  16   36.50   56.22    48.62 dimr      *               v1.4.0-2\n127.0.0.1           21          46  16   36.50   56.22    48.62 dimr      -               v1.4.0-1\n127.0.0.1           32          46  16   36.50   56.22    48.62 dimr      -               v1.4.0-0\n127.0.0.1           19          46  16   36.50   56.22    48.62 dimr      -               v1.4.0-3\n"
        at __randomizedtesting.SeedInfo.seed([C7989F860D60857C:4FCCA05CA39CE884]:0)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.executeSection(OpenSearchClientYamlSuiteTestCase.java:442)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.test(OpenSearchClientYamlSuiteTestCase.java:415)

opensearch-ci-bot · 2022-03-13T07:29:31Z

❌ Gradle Check failure 7f0ad8f
Log 3309

Reports 3309

Signed-off-by: Tianli Feng <ftianli@amazon.com>

opensearch-ci-bot · 2022-03-13T08:08:36Z

❌ Gradle Check failure cf385f5
Log 3313

Reports 3313

Signed-off-by: Tianli Feng <ftianli@amazon.com>

opensearch-ci-bot · 2022-03-14T05:11:37Z

❌ Gradle Check failure efa7a75372870d03457cb96bcf18bd5402167558
Log 3328

Reports 3328

opensearch-ci-bot · 2022-03-14T05:11:53Z

❌ Gradle Check failure fb7ec1a
Log 3329

Reports 3329

tlfeng · 2022-03-14T05:14:27Z

In log 3328:

REPRODUCE WITH: ./gradlew ':qa:mixed-cluster:v1.2.5#mixedClusterTest' --tests "org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT" -Dtests.method="test {p0=indices.get_field_mapping/20_missing_field/Return empty object if field doesn't exist, but index does}" -Dtests.seed=D61FE7F6821A0034 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en-PH -Dtests.timezone=Europe/Simferopol -Druntime.java=17

org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT > test {p0=indices.get_field_mapping/20_missing_field/Return empty object if field doesn't exist, but index does} FAILED
    java.lang.AssertionError: Failure at [indices.get_field_mapping/20_missing_field:20]: field [test_index.mappings] is null
        at __randomizedtesting.SeedInfo.seed([D61FE7F6821A0034:5E4BD82C2CE66DCC]:0)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.executeSection(OpenSearchClientYamlSuiteTestCase.java:442)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.test(OpenSearchClientYamlSuiteTestCase.java:415)

It's reported in #2440

In log 3329:

> Task :server:internalClusterTest

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.discovery.StableMasterDisruptionIT.testStaleMasterNotHijackingMajority" -Dtests.seed=1656830A3FB8F3E7 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en-IN -Dtests.timezone=America/Puerto_Rico -Druntime.java=17

org.opensearch.discovery.StableMasterDisruptionIT > testStaleMasterNotHijackingMajority FAILED
    java.lang.AssertionError: node_t2: [Tuple [v1=node_t1, v2=null]]
        at __randomizedtesting.SeedInfo.seed([1656830A3FB8F3E7:4950CE051E632CEB]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.opensearch.discovery.StableMasterDisruptionIT.lambda$testStaleMasterNotHijackingMajority$5(StableMasterDisruptionIT.java:253)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1060)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1033)
        at org.opensearch.discovery.StableMasterDisruptionIT.testStaleMasterNotHijackingMajority(StableMasterDisruptionIT.java:250)

It's reported in issue #1565. Maybe caused by too many gradle check runs in CI.

tlfeng · 2022-03-15T05:16:51Z

start gradle check

opensearch-ci-bot · 2022-03-15T06:04:24Z

✅ Gradle Check success fb7ec1a
Log 3387

Reports 3387

andrross · 2022-03-15T22:00:11Z

Because "cat API is a human-readable interface" (https://opensearch.org/docs/latest/opensearch/rest-api/cat/index/), and are not intended for use by software program, the old name "master" is not retained.

Just because the documentation describes this as a human readable does not mean that users don't write software that parses the result :) Given that the response is unstructured text, I think this is probably the right way to go, but I suspect someone somewhere will be broken by this change.

tlfeng · 2022-03-15T23:04:21Z

Just because the documentation describes this as a human readable does not mean that users don't write software that parses the result :) Given that the response is unstructured text, I think this is probably the right way to go, but I suspect someone somewhere will be broken by this change.

😁 Hi @andrross, you are so correct. I can never realized that the test framework of OpenSearch itself is parsing the result of the CAT Nodes API, until running the BWC test. https://github.com/opensearch-project/OpenSearch/blob/1.2.4/test/framework/src/main/java/org/opensearch/test/rest/yaml/OpenSearchClientYamlSuiteTestCase.java#L326 😂
It motivated me to add master as the alias for getting the output of cluster_manager column, when using GET _cat/nodes?v&h=master (in the commit 45e04ce).

For example:
The response of GET _cat/nodes?v&h=version&master after this PR will be the same as the current:
(for a single node cluster)

version master
2.0.0   *

I will add this to the PR description as well.
I think with necessary documentation about the "alias", users can find this proper way of specifying headers in the request parameter to keep their code of parsing CAT Nodes API for both 1.x and 2.x versions.

Replace master with cluster_manager in cat nodes api

9cadbce

Signed-off-by: Tianli Feng <ftianli@amazon.com>

tlfeng requested a review from a team as a code owner March 11, 2022 06:27

tlfeng mentioned this pull request Mar 11, 2022

Add alternative usages for the REST APIs and configurations that contain "master" terminology #1549

Closed

12 tasks

tlfeng added enhancement Enhancement or improvement to existing feature or request v2.0.0 Version 2.0.0 documentation pending Tracks issues which have PRs merged but documentation changes pending >breaking Identifies a breaking change. labels Mar 11, 2022

tlfeng marked this pull request as draft March 11, 2022 17:38

Tianli Feng added 3 commits March 12, 2022 22:06

Merge branch 'main' into replace-master-in-cat-nodes

a8ff7a2

Signed-off-by: Tianli Feng <ftianli@amazon.com>

Add master as the table header alias for cluster_manager for compatib…

45e04ce

…ility Signed-off-by: Tianli Feng <ftianli@amazon.com>

Add cm as the alias for table header cluster_manager

7f0ad8f

Signed-off-by: Tianli Feng <ftianli@amazon.com>

fix yaml rest rest by using node_selector to filter nodes in 2.x version

cf385f5

Signed-off-by: Tianli Feng <ftianli@amazon.com>

Fix nodes.info yaml rest test during mixedclustertest

fb7ec1a

Signed-off-by: Tianli Feng <ftianli@amazon.com>

tlfeng force-pushed the replace-master-in-cat-nodes branch from efa7a75 to fb7ec1a Compare March 14, 2022 04:26

tlfeng marked this pull request as ready for review March 14, 2022 05:22

dblock requested a review from andrross March 15, 2022 15:48

kartg approved these changes Mar 16, 2022

View reviewed changes

tlfeng mentioned this pull request Mar 16, 2022

Replace 'discovered_master' with 'discovered_cluster_manager' in 'GET Cat Health' API #2438

Merged

5 tasks

andrross approved these changes Mar 18, 2022

View reviewed changes

tlfeng merged commit 641350b into opensearch-project:main Mar 18, 2022

tlfeng deleted the replace-master-in-cat-nodes branch March 18, 2022 22:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace 'master' with 'cluster_manager' in 'GET Cat Nodes' API #2441

Replace 'master' with 'cluster_manager' in 'GET Cat Nodes' API #2441

tlfeng commented Mar 11, 2022 •

edited

Loading

opensearch-ci-bot commented Mar 11, 2022

opensearch-ci-bot commented Mar 11, 2022

tlfeng commented Mar 11, 2022

opensearch-ci-bot commented Mar 13, 2022

tlfeng commented Mar 13, 2022

opensearch-ci-bot commented Mar 13, 2022

opensearch-ci-bot commented Mar 13, 2022

opensearch-ci-bot commented Mar 14, 2022

opensearch-ci-bot commented Mar 14, 2022

tlfeng commented Mar 14, 2022 •

edited

Loading

tlfeng commented Mar 15, 2022

opensearch-ci-bot commented Mar 15, 2022

andrross commented Mar 15, 2022

tlfeng commented Mar 15, 2022 •

edited

Loading

Replace 'master' with 'cluster_manager' in 'GET Cat Nodes' API #2441

Replace 'master' with 'cluster_manager' in 'GET Cat Nodes' API #2441

Conversation

tlfeng commented Mar 11, 2022 • edited Loading

Description

Issues Resolved

Check List

opensearch-ci-bot commented Mar 11, 2022

opensearch-ci-bot commented Mar 11, 2022

tlfeng commented Mar 11, 2022

opensearch-ci-bot commented Mar 13, 2022

tlfeng commented Mar 13, 2022

opensearch-ci-bot commented Mar 13, 2022

opensearch-ci-bot commented Mar 13, 2022

opensearch-ci-bot commented Mar 14, 2022

opensearch-ci-bot commented Mar 14, 2022

tlfeng commented Mar 14, 2022 • edited Loading

tlfeng commented Mar 15, 2022

opensearch-ci-bot commented Mar 15, 2022

andrross commented Mar 15, 2022

tlfeng commented Mar 15, 2022 • edited Loading

tlfeng commented Mar 11, 2022 •

edited

Loading

tlfeng commented Mar 14, 2022 •

edited

Loading

tlfeng commented Mar 15, 2022 •

edited

Loading