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

Add a field 'discovered_cluster_manager' in 'GET Cluster Health' API that shows identical value with field 'discovered_master' #2437

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Mar 11, 2022

Description

  • Add a new field discovered_cluster_manager to the response of GET _cluster/health API, which has got the same value with the existing discovered_master field, aims to replace the non-inclusive term "master" in the API response field.
  • Add yaml rest test to validate discovered_cluster_manager and discovered_master field have got the same value.

Current response of GET _cluster/health API

{
  "cluster_name" : "opensearch",
  "status" : "green",
  "timed_out" : false,
  "number_of_nodes" : 1,
  "number_of_data_nodes" : 1,
  "discovered_master" : true,
...
}

Proposed response of GET _cluster/health API

{
  "cluster_name" : "opensearch",
  "status" : "green",
  "timed_out" : false,
  "number_of_nodes" : 1,
  "number_of_data_nodes" : 1,
  "discovered_master" : true,
  "discovered_cluster_manager" : true,
...
}

Testing:

  ./gradlew ':rest-api-spec:yamlRestTest' \
  --tests "org.opensearch.test.rest.ClientYamlTestSuiteIT" \
  -Dtests.method="test {p0=cluster.health/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.

@tlfeng 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 tlfeng requested a review from a team as a code owner March 11, 2022 04:04
@tlfeng tlfeng changed the title Add a field discovered_cluster_manager in get cluster health api Add a new field 'discovered_cluster_manager' in 'GET Cluster Health' API, which having the same value with 'discovered_master' Mar 11, 2022
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@tlfeng tlfeng changed the title Add a new field 'discovered_cluster_manager' in 'GET Cluster Health' API, which having the same value with 'discovered_master' Add a field 'discovered_cluster_manager' in 'GET Cluster Health' API that shows identical value with field 'discovered_master' Mar 11, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 12d6f676976220d00e1ca4d2684ea2a74495b321
Log 3250

Reports 3250

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@tlfeng tlfeng force-pushed the cluster-manager-cluster-health branch from 12d6f67 to 0148e0e Compare March 11, 2022 04:45
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0148e0e
Log 3252

Reports 3252

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 11, 2022

In log 3252:

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=9FC2465A92C8B157 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=uk-UA -Dtests.timezone=Australia/Hobart -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:13]: expected [2xx] status code but api [indices.get_field_mapping] returned [404 Not Found] [{}]
        at __randomizedtesting.SeedInfo.seed([9FC2465A92C8B157:179679803C34DCAF]: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 not reproducible locally. Opened issue #2440

@dblock
Copy link
Member

dblock commented Mar 11, 2022

start gradle check

@dblock dblock requested a review from kartg March 11, 2022 18:59
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0148e0e
Log 3266

Reports 3266

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 11, 2022

Log 3266 shows the same test failure as above, now I begin to doubt if that is caused by the code change.. 😂 But it's not reproducible by the single command ./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=9FC2465A92C8B157
Update: It's tracked in issue #2440

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 15, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0148e0e
Log 3388

Reports 3388

…ster-health

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure c05d14c
Log 3422

Reports 3422

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 15, 2022

In log 3422:

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards" -Dtests.seed=72EABB7906B3AA3D -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en-IN -Dtests.timezone=America/Tegucigalpa -Druntime.java=17
   2> java.lang.AssertionError: AcknowledgedResponse failed - not acked
     Expected: <true>
          but: was <false>
        at __randomizedtesting.SeedInfo.seed([72EABB7906B3AA3D:5E9BB19817AD80DF]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:127)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:131)
        at org.opensearch.test.TestCluster.wipeIndices(TestCluster.java:167)
        at org.opensearch.test.TestCluster.wipe(TestCluster.java:90)
        at org.opensearch.test.OpenSearchIntegTestCase.afterInternal(OpenSearchIntegTestCase.java:601)
        at org.opensearch.test.OpenSearchIntegTestCase.cleanUpCluster(OpenSearchIntegTestCase.java:2235)
...

It's reported in issue #1561

@tlfeng tlfeng requested review from dreamer-89 and removed request for kartg March 16, 2022 02:33
Copy link
Member

@dreamer-89 dreamer-89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@dreamer-89
Copy link
Member

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure c05d14c
Log 3432

Reports 3432

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 16, 2022

In log 3432:

> Task :qa:remote-clusters:integTest

REPRODUCE WITH: ./gradlew ':qa:remote-clusters:integTest' --tests "org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks" -Dtests.seed=A9A05CBF01AE3B80 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-DZ -Dtests.timezone=America/Cayman -Druntime.java=17

WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/home/ubuntu/.gradle/wrapper/dists/gradle-7.3.3-all/4295vidhdd9hd3gbjyw1xqxpo/gradle-7.3.3/lib/plugins/gradle-testing-base-7.3.3.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release
org.opensearch.cluster.remote.test.RemoteClustersIT > testHAProxyModeConnectionWorks FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([A9A05CBF01AE3B80:AED689743A284DDD]:0)
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertTrue(Assert.java:53)
        at org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks(RemoteClustersIT.java:125)

It's reported in issue #1703

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 16, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure c05d14c
Log 3436

Reports 3436

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 16, 2022

In log 3436:

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards" -Dtests.seed=B0C5A531120FFD2 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=es-PR -Dtests.timezone=America/Coral_Harbour -Druntime.java=17

org.opensearch.cluster.allocation.ClusterRerouteIT > testDelayWithALargeAmountOfShards FAILED
    java.lang.AssertionError: AcknowledgedResponse failed - not acked
    Expected: <true>
         but: was <false>
        at __randomizedtesting.SeedInfo.seed([B0C5A531120FFD2:277D50B2003ED530]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:127)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:131)
        at org.opensearch.test.TestCluster.wipeIndices(TestCluster.java:167)
        at org.opensearch.test.TestCluster.wipe(TestCluster.java:90)
        at org.opensearch.test.OpenSearchIntegTestCase.afterInternal(OpenSearchIntegTestCase.java:601)
        at org.opensearch.test.OpenSearchIntegTestCase.cleanUpCluster(OpenSearchIntegTestCase.java:2235)

It's reported in issue #1561

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 55a62d4
Log 3484

Reports 3484

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 17, 2022

In log 3484:
It's not reproducible locally.

> Task :server:internalClusterTest

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.aliases.IndexAliasesIT.testSameAlias" -Dtests.seed=42844E403B1AF12C -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=cs-CZ -Dtests.timezone=America/Argentina/Salta -Druntime.java=17

org.opensearch.aliases.IndexAliasesIT > testSameAlias FAILED
    java.lang.AssertionError: AcknowledgedResponse failed - not acked
    Expected: <true>
         but: was <false>
        at __randomizedtesting.SeedInfo.seed([42844E403B1AF12C:C0C7FB8A9CF2AB0A]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:127)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:115)
        at org.opensearch.aliases.IndexAliasesIT.lambda$testSameAlias$54(IndexAliasesIT.java:820)
        at org.opensearch.aliases.IndexAliasesIT.assertAliasesVersionIncreases(IndexAliasesIT.java:1489)
        at org.opensearch.aliases.IndexAliasesIT.assertAliasesVersionIncreases(IndexAliasesIT.java:1480)
        at org.opensearch.aliases.IndexAliasesIT.testSameAlias(IndexAliasesIT.java:818)

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 28c671a
Log 3496

Reports 3496

@tlfeng tlfeng merged commit 1c2bdd8 into opensearch-project:main Mar 18, 2022
@tlfeng tlfeng deleted the cluster-manager-cluster-health branch March 18, 2022 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking Identifies a breaking change. documentation pending Tracks issues which have PRs merged but documentation changes pending enhancement Enhancement or improvement to existing feature or request v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants