Refresh OpenSearch nodes version in cluster state after upgrade #865

shwetathareja · 2021-06-21T09:27:37Z

Description

PR for fixing the issue #827

Fix:

As soon as OpenSearch node becomes master, it refreshes the nodes version in cluster state by comparing against their channel version. Right now, it doesn't wait for all Elasticsearch nodes to leave the cluster before refreshing the version.
There was bug in DiscoveryNode BWC serialization logic where any ES version < 7.10.2 was getting over-written with 7.10.2

Issues Resolved

#827

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.

opensearch-ci-bot · 2021-06-21T09:29:16Z

❌ DCO Check Failed 1df4237eb82a3374ba42478347b9bcb78cff9b05
Run ./dev-tools/signoff-check.sh remotes/origin/main 1df4237eb82a3374ba42478347b9bcb78cff9b05 to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

opensearch-ci-bot · 2021-06-21T09:30:00Z

✅ Gradle Wrapper Validation success 1df4237eb82a3374ba42478347b9bcb78cff9b05

opensearch-ci-bot · 2021-06-21T09:36:07Z

❌ Gradle Precommit failure 1df4237eb82a3374ba42478347b9bcb78cff9b05
Log 625

dblock · 2021-06-21T15:19:11Z

I like it.

nknize

+1 on the approach so far. Build fails due to simple compile issue. Will review further after integration and unit tests are added and we can merge in a 1.0.0 patch release since this will likely not make it in time for GA.

nknize · 2021-06-28T17:38:30Z

@shwetathareja Any update on getting Integration and Unit tests done? Do you think you could get this in for GA? If not we will need to get a bugfix release out pretty quickly after GA.

CEHENKLE · 2021-06-29T18:07:50Z

@shwetathareja Hey -- we're running out of runway for 1.0.0 and this seems like something we'd want to get in. Can you get your tests in?

shwetathareja · 2021-07-01T02:29:56Z

@nknize @CEHENKLE sorry for the delay, working on it, will raise it in next couple of hours.

opensearch-ci-bot · 2021-07-01T05:46:50Z

❌ DCO Check Failed 54ef87927cffb5a70edd52d2fc4de97e6d928a9d
Run ./dev-tools/signoff-check.sh remotes/origin/main 54ef87927cffb5a70edd52d2fc4de97e6d928a9d to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

opensearch-ci-bot · 2021-07-01T05:48:21Z

✅ Gradle Wrapper Validation success 54ef87927cffb5a70edd52d2fc4de97e6d928a9d

opensearch-ci-bot · 2021-07-01T05:54:34Z

❌ Gradle Precommit failure 54ef87927cffb5a70edd52d2fc4de97e6d928a9d
Log 691

CEHENKLE · 2021-07-01T06:21:36Z

Thanks, @shwetathareja ! I'll look for it in our AM :)

opensearch-ci-bot · 2021-07-01T10:25:05Z

✅ Gradle Wrapper Validation success 12717a5455f85e062276ae861270155a078a7ea6

opensearch-ci-bot · 2021-07-01T10:25:05Z

❌ DCO Check Failed 12717a5455f85e062276ae861270155a078a7ea6
Run ./dev-tools/signoff-check.sh remotes/origin/main 12717a5455f85e062276ae861270155a078a7ea6 to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

opensearch-ci-bot · 2021-07-01T10:29:55Z

✅ Gradle Wrapper Validation success 191133e0770b7629b263687d96b0ced3a6d72a0a

opensearch-ci-bot · 2021-07-01T10:30:49Z

❌ DCO Check Failed 191133e0770b7629b263687d96b0ced3a6d72a0a
Run ./dev-tools/signoff-check.sh remotes/origin/main 191133e0770b7629b263687d96b0ced3a6d72a0a to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

opensearch-ci-bot · 2021-07-01T10:35:09Z

❌ Gradle Precommit failure 12717a5455f85e062276ae861270155a078a7ea6
Log 692

opensearch-ci-bot · 2021-07-01T10:35:09Z

❌ Gradle Precommit failure 191133e0770b7629b263687d96b0ced3a6d72a0a
Log 693

opensearch-ci-bot · 2021-07-01T10:49:51Z

✅ Gradle Wrapper Validation success 34e53070801c94aab59ba7e93262e398167f0ee2

opensearch-ci-bot · 2021-07-01T10:50:49Z

❌ DCO Check Failed 34e53070801c94aab59ba7e93262e398167f0ee2
Run ./dev-tools/signoff-check.sh remotes/origin/main 34e53070801c94aab59ba7e93262e398167f0ee2 to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

opensearch-ci-bot · 2021-07-01T10:55:34Z

✅ Gradle Precommit success 34e53070801c94aab59ba7e93262e398167f0ee2

shwetathareja · 2021-07-01T12:29:41Z

@nknize @adnapibar @dblock please review.

shwetathareja · 2021-07-01T12:36:21Z

start gradle check

adnapibar · 2021-07-01T18:26:11Z

start gradle check

opensearch-ci-bot · 2021-07-01T18:54:30Z

✅ Gradle Check success 34e53070801c94aab59ba7e93262e398167f0ee2
Log 287

Reports 287

adnapibar · 2021-07-01T20:39:02Z

The changes look good but I see the a few bwc tests for mixed cluster are failing (these pass in main)

./gradlew :qa:mixed-cluster:v7.10.2#mixedClusterTest

Tests with failures:
 - org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=cat.health/10_basic/Empty cluster}
 - org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=cat.health/10_basic/Help}
 - org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=cat.health/10_basic/With ts parameter}

adnapibar · 2021-07-01T23:29:45Z

The changes look good but I see the a few bwc tests for mixed cluster are failing (these pass in main)

./gradlew :qa:mixed-cluster:v7.10.2#mixedClusterTest

Tests with failures:
 - org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=cat.health/10_basic/Empty cluster}
 - org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=cat.health/10_basic/Help}
 - org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT.test {p0=cat.health/10_basic/With ts parameter}

Looks like the BWC tests have become flaky and I can also see these in the main branch after a rerun. I have created an issue to investigate and resolve these separately #922

adnapibar

LGTM - couple of minor questions.
Also please fix the DCO check.

adnapibar · 2021-07-01T23:36:56Z

server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java

@@ -367,7 +367,7 @@ public void writeTo(StreamOutput out) throws IOException {
                }
            }
        }
-        if (out.getVersion().before(Version.V_1_0_0)) {
+        if (out.getVersion().before(Version.V_1_0_0) && version.onOrAfter(Version.V_1_0_0)) {


can you please help me understand why we're adding this?

so, DiscoveryNode writeTo serialization method can be triggered from multiple places e.g. when cluster state is being sent from one node to another and BWC version should be set for only OpenSearch nodes. Without this check, it will mess with the version for Elasticsearch nodes in the cluster as well which ideally should be untouched.

adnapibar · 2021-07-02T00:09:31Z

server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java

+                    // there is an edge case if doesn't send JoinRequest and connection is established,
+                    // then it can continue to report version as 7.10.2 instead of actual OpenSearch version. So,
+                    // removing the node from cluster state to prevent stale version reporting and let it reconnect.
+                    if (node.getVersion().equals(LegacyESVersion.V_7_10_2)) {


Can it not be any other legacy version e.g. 6.8.x or 7.10.x which we are upgrading from?

We're only concerned about the OpenSearch nodes in BWC mode, any other ES node that is not yet upgraded should run in mixed cluster - is that correct understanding?

For other ES versions, the version in cluster state and channel version would always be same. So, there is no need for special handling. Its only for OpenSearch nodes which spoof to be 7.10.2 and we don't know if it is real 7.10.2 or OpenSearch node spoofing its version.

CEHENKLE · 2021-07-02T00:16:45Z

@shwetathareja Heya Shweta -- we're holding the build for this fix, so if you could fix your DCO, we're going slam the door on 1.0.0 behind you. So if you can fix that and merge when you get in, we should be good to go in our AM :)

Thanks,
/C

Signed-off-by: Shweta Thareja <tharejas@amazon.com>

opensearch-ci-bot · 2021-07-02T03:50:20Z

✅ Gradle Wrapper Validation success c75f5e3

opensearch-ci-bot · 2021-07-02T03:50:44Z

✅ DCO Check Passed c75f5e3

opensearch-ci-bot · 2021-07-02T03:58:20Z

✅ Gradle Precommit success c75f5e3

adnapibar · 2021-07-02T06:20:10Z

start gradle check

opensearch-ci-bot · 2021-07-02T06:43:45Z

✅ Gradle Check success c75f5e3
Log 298

Reports 298

…search-project#865) Signed-off-by: Shweta Thareja <tharejas@amazon.com> Co-authored-by: Shweta Thareja <tharejas@amazon.com>

#926) Signed-off-by: Shweta Thareja <tharejas@amazon.com> Co-authored-by: shwetathareja <shwetathareja@live.com>

This change removes code that was written to support rolling upgrade from Elasticsearch 7.10.2 to OpenSearch 1.x, as part of PR #865 Signed-off-by: Rabi Panda <adnapibar@gmail.com> Signed-off-by: Rabi Panda <adnapibar@gmail.com>

…ch-project#865) Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

shwetathareja requested review from nknize, dblock and itiyamas June 21, 2021 09:27

nknize added bug Something isn't working v1.0.0 Version 1.0.0 rolling-upgrade Issues related to rolling upgrades labels Jun 24, 2021

nknize reviewed Jun 24, 2021

View reviewed changes

nknize added Severity-Blocker and removed Severity-Blocker labels Jun 25, 2021

shwetathareja marked this pull request as ready for review July 1, 2021 10:34

adnapibar reviewed Jul 2, 2021

View reviewed changes

Refresh OpenSearch nodes version in cluster state after upgrade

c75f5e3

Signed-off-by: Shweta Thareja <tharejas@amazon.com>

shwetathareja force-pushed the version-fix-cat-apis-after-upgrade branch from 34e5307 to c75f5e3 Compare July 2, 2021 03:48

adnapibar approved these changes Jul 2, 2021

View reviewed changes

CEHENKLE merged commit 8082604 into opensearch-project:main Jul 2, 2021

tlfeng mentioned this pull request Jul 2, 2021

[1.x] Refresh OpenSearch nodes version in cluster state after upgrade (#865) #926

Merged

5 tasks

adnapibar pushed a commit that referenced this pull request Jul 2, 2021

Refresh OpenSearch nodes version in cluster state after upgrade (#865) (

b383355

#926) Signed-off-by: Shweta Thareja <tharejas@amazon.com> Co-authored-by: shwetathareja <shwetathareja@live.com>

adnapibar pushed a commit that referenced this pull request Jul 2, 2021

Refresh OpenSearch nodes version in cluster state after upgrade (#865) (

6a5f9b0

#926) Signed-off-by: Shweta Thareja <tharejas@amazon.com> Co-authored-by: shwetathareja <shwetathareja@live.com>

nknize mentioned this pull request Jul 6, 2021

[BUG]_cat/nodes return stale OpenSearch version after upgrades from ES 7.x/6.8 #827

Closed

shwetathareja deleted the version-fix-cat-apis-after-upgrade branch July 11, 2021 09:54

adnapibar mentioned this pull request Nov 12, 2022

Remove changes for legacy mixed-cluster support #5228

Merged

2 tasks

ritty27 pushed a commit to ritty27/OpenSearch that referenced this pull request May 12, 2024

Consolidate and cleanup the server version checks for tests (opensear…

a1bf415

…ch-project#865) Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

Refresh OpenSearch nodes version in cluster state after upgrade #865

Refresh OpenSearch nodes version in cluster state after upgrade #865

Conversation

shwetathareja commented Jun 21, 2021 • edited Loading

Description

Issues Resolved

Check List

opensearch-ci-bot commented Jun 21, 2021

opensearch-ci-bot commented Jun 21, 2021

opensearch-ci-bot commented Jun 21, 2021

dblock commented Jun 21, 2021

nknize left a comment

Choose a reason for hiding this comment

nknize commented Jun 28, 2021

CEHENKLE commented Jun 29, 2021

shwetathareja commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

CEHENKLE commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

shwetathareja commented Jul 1, 2021

shwetathareja commented Jul 1, 2021

adnapibar commented Jul 1, 2021

opensearch-ci-bot commented Jul 1, 2021

adnapibar commented Jul 1, 2021

adnapibar commented Jul 1, 2021 • edited Loading

adnapibar left a comment

Choose a reason for hiding this comment

adnapibar Jul 1, 2021

Choose a reason for hiding this comment

shwetathareja Jul 2, 2021

Choose a reason for hiding this comment

adnapibar Jul 2, 2021

Choose a reason for hiding this comment

adnapibar Jul 2, 2021

Choose a reason for hiding this comment

shwetathareja Jul 2, 2021

Choose a reason for hiding this comment

CEHENKLE commented Jul 2, 2021

opensearch-ci-bot commented Jul 2, 2021

opensearch-ci-bot commented Jul 2, 2021

opensearch-ci-bot commented Jul 2, 2021

adnapibar commented Jul 2, 2021

opensearch-ci-bot commented Jul 2, 2021

shwetathareja commented Jun 21, 2021 •

edited

Loading

adnapibar commented Jul 1, 2021 •

edited

Loading