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

Generalize Backwards Compatibility tests so we can test from any version to any version #2253

Merged
merged 4 commits into from
Nov 18, 2022

Conversation

peternied
Copy link
Member

@peternied peternied commented Nov 9, 2022

Description

With an issue reported indicating that there are serialization issue between 1.3 and 2.X, making sure that we can reproduce the errors. This new workflow(s) will make sure that we aren't breaking BWC with changes we are adding to 2.X and will give us the flexibility to test certain migration workflows.

Fixing an issue where the BWC tests were not actually building or executing causing the clusters to spin up and then immediately spin down. We will need to invest more energy into running multiple kinds of security plugin specific scenarios through the test system.

Issues Resolved

Check List

  • New functionality includes testing
  • New functionality has been documented
  • 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.

@peternied peternied force-pushed the flexible-bwc branch 2 times, most recently from e75a9c2 to d2efd08 Compare November 10, 2022 21:00
@peternied peternied changed the title [Draft] Generalize Backwards Compatibility tests so we can test from any version to any version Generalize Backwards Compatibility tests so we can test from any version to any version Nov 10, 2022
@peternied peternied marked this pull request as ready for review November 10, 2022 21:10
@peternied peternied requested a review from a team November 10, 2022 21:10
@peternied
Copy link
Member Author

The build is totally broken because a JDK18 features is being used - waiting on the following issue to be merged to resolve this
opensearch-project/OpenSearch#5205

@codecov-commenter
Copy link

codecov-commenter commented Nov 10, 2022

Codecov Report

Merging #2253 (7d7b262) into main (93fe633) will increase coverage by 0.09%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##               main    #2253      +/-   ##
============================================
+ Coverage     61.02%   61.12%   +0.09%     
- Complexity     3267     3271       +4     
============================================
  Files           259      259              
  Lines         18337    18337              
  Branches       3248     3248              
============================================
+ Hits          11191    11209      +18     
+ Misses         5561     5541      -20     
- Partials       1585     1587       +2     
Impacted Files Coverage Δ
...iance/ComplianceIndexingOperationListenerImpl.java 63.23% <0.00%> (+1.47%) ⬆️
...ic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java 59.30% <0.00%> (+3.48%) ⬆️
.../auth/http/jwt/keybyoidc/SelfRefreshingKeySet.java 68.30% <0.00%> (+8.45%) ⬆️
...t/keybyoidc/AuthenticatorUnavailableException.java 20.00% <0.00%> (+20.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

.github/actions/create-bwc-build/action.yaml Outdated Show resolved Hide resolved
.github/actions/run-bwc-suite/action.yaml Show resolved Hide resolved
.github/workflows/bwc-tests.yml Outdated Show resolved Hide resolved
bwc-test/build.gradle Show resolved Hide resolved
bwc-test/build.gradle Show resolved Hide resolved
…ion to any version

With an issue reported indicating that there are serialization issue between 1.3 and 2.X, making sure that we can reproduce the errors. This new workflow(s) will make sure that we aren't breaking BWC with changes we are adding to 2.X and will give us the flexibility to test certain migration workflows.

Fixing an issue where the BWC tests were not actually building or executing causing the clusters to spin up and then immediately spin down. We will need to invest more energy into running multiple kinds of security plugin specific scenarios through the test system.

Signed-off-by: Peter Nied <[email protected]>
@peternied
Copy link
Member Author

Note; CI is blocked until the following issue(s) are resolved:

.github/actions/create-bwc-build/action.yaml Show resolved Hide resolved
.github/actions/create-bwc-build/action.yaml Outdated Show resolved Hide resolved
.github/actions/create-bwc-build/action.yaml Show resolved Hide resolved
.github/actions/create-bwc-build/action.yaml Outdated Show resolved Hide resolved
.github/actions/run-bwc-suite/action.yaml Show resolved Hide resolved
.github/actions/run-bwc-suite/action.yaml Show resolved Hide resolved
.github/workflows/bwc-tests.yml Show resolved Hide resolved
Copy link
Member

@cwperks cwperks left a comment

Choose a reason for hiding this comment

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

Thank you @peternied! I took a first pass and really like the flexibility this gives us in BWC testing. Left a few thoughts. There's a few descriptions that look incomplete which should be addressed, but otherwise this looks good to me.

.github/actions/run-bwc-suite/action.yaml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
bwc-test/build.gradle Outdated Show resolved Hide resolved
.github/workflows/bwc-tests.yml Outdated Show resolved Hide resolved
.github/workflows/bwc-tests.yml Outdated Show resolved Hide resolved
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

Much cleaner and compartmentalized!!

Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
@peternied
Copy link
Member Author

@cwperks @DarshitChanpura @scrawfor99 CI has finally been sorted out since we've got a 2.5.0 build, please take another look.

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

Ty @peternied for resolving this pain!

@cwperks
Copy link
Member

cwperks commented Nov 18, 2022

@peternied Quick q: Is it safe to ignore these Windows errors? Failed to create working dir using hard links. Falling back to copy org.opensearch.gradle.testclusters.OpenSearchNode$LinkCreationException: Failed to create hard link D:\a\security\security\bwc-test\build\testclusters\securityBwcCluster0-2\distro\3.0.0-ARCHIVE\bin\opensearch pointing to C:\Users\runneradmin\.gradle\caches\transforms-3\c38e0a71e65414d6057b5385bc6304be\transformed\opensearch-3.0.0-windows-x64.zip\opensearch-3.0.0\bin\opensearch

@peternied
Copy link
Member Author

@cwperks Interesting, since they are falling back seems safe to me. Took a look at the related code, seems like maybe its slowing the process down a little. I think we could dig in more in the future if we were looking for a speed boost

https://github.com/opensearch-project/OpenSearch/blob/a0d30739402ab361d2a4c0fc6970ffbd2fe07039/buildSrc/src/main/java/org/opensearch/gradle/testclusters/OpenSearchNode.java#L1048

@peternied peternied merged commit dc7d21c into opensearch-project:main Nov 18, 2022
@peternied peternied deleted the flexible-bwc branch November 18, 2022 22:01
@stephen-crawford
Copy link
Contributor

This looks really good to me!

@stephen-crawford stephen-crawford added the backport 1.3 backport to 1.3 branch label Dec 1, 2022
@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.3 1.3
# Navigate to the new working tree
cd .worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-2253-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 dc7d21c716250001926af9ce5d5937d221122ff9
# Push it to GitHub
git push --set-upstream origin backport/backport-2253-to-1.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.3

Then, create a pull request where the base branch is 1.3 and the compare/head branch is backport/backport-2253-to-1.3.

peternied added a commit to pawel-gudel-eliatra/security that referenced this pull request Jul 10, 2023
…ion to any version (opensearch-project#2253)

* Generalize Backwards Compatibility tests so we can test from any version to any version

With an issue reported indicating that there are serialization issue between 1.3 and 2.X, making sure that we can reproduce the errors. This new workflow(s) will make sure that we aren't breaking BWC with changes we are adding to 2.X and will give us the flexibility to test certain migration workflows.

Fixing an issue where the BWC tests were not actually building or executing causing the clusters to spin up and then immediately spin down. We will need to invest more energy into running multiple kinds of security plugin specific scenarios through the test system.

Signed-off-by: Peter Nied <[email protected]>
kaituo added a commit to kaituo/anomaly-detection-1 that referenced this pull request Sep 8, 2023
Issue:
- In versions between 1.1 and 2.10, the task :adBwcCluster#twoThirdsUpgradedClusterTask fails.
- Symptoms mirror those in [OpenSearch Issue #5076](opensearch-project/OpenSearch#5076).
- Logs show nodes from old and new versions failing to join as a cluster, resulting in a "master not found" exception.

Resolution:
- Upgraded the bwc version to 1.3.2, aligning with other plugins which use 1.3+ as their baseline.
  - [Cross-Cluster Replication PR opensearch-project#469](opensearch-project/cross-cluster-replication#469)
  - [Security PR #2253](opensearch-project/security#2253)
  - [ML Commons PR opensearch-project#681](opensearch-project/ml-commons#681)
- Post-upgrade, the twoThirdsUpgradedClusterTask runs successfully, suggesting potential incompatibility between versions 1.1 and 2.10.

Testing:
- Executed `./gradlew bwcTestSuite -Dtests.security.manager=false` and all tests passed.

Signed-off-by: Kaituo Li <[email protected]>
kaituo added a commit to kaituo/anomaly-detection-1 that referenced this pull request Sep 8, 2023
Issue:
- In versions between 1.1 and 2.10, the task :adBwcCluster#twoThirdsUpgradedClusterTask fails.
- Symptoms mirror those in [OpenSearch Issue #5076](opensearch-project/OpenSearch#5076).
- Logs show nodes from old and new versions failing to join as a cluster, resulting in a "master not found" exception.

Resolution:
- Upgraded the bwc version to 1.3.2, aligning with other plugins which use 1.3+ as their baseline.
  - [Cross-Cluster Replication PR opensearch-project#469](opensearch-project/cross-cluster-replication#469)
  - [Security PR #2253](opensearch-project/security#2253)
  - [ML Commons PR opensearch-project#681](opensearch-project/ml-commons#681)
- Post-upgrade, the twoThirdsUpgradedClusterTask runs successfully, suggesting potential incompatibility between versions 1.1 and 2.10.

Testing:
- Executed `./gradlew bwcTestSuite -Dtests.security.manager=false` and all tests passed.

Signed-off-by: Kaituo Li <[email protected]>
jackiehanyang pushed a commit to opensearch-project/anomaly-detection that referenced this pull request Sep 8, 2023
…sue (#1029)

Issue:
- In versions between 1.1 and 2.10, the task :adBwcCluster#twoThirdsUpgradedClusterTask fails.
- Symptoms mirror those in [OpenSearch Issue #5076](opensearch-project/OpenSearch#5076).
- Logs show nodes from old and new versions failing to join as a cluster, resulting in a "master not found" exception.

Resolution:
- Upgraded the bwc version to 1.3.2, aligning with other plugins which use 1.3+ as their baseline.
  - [Cross-Cluster Replication PR #469](opensearch-project/cross-cluster-replication#469)
  - [Security PR #2253](opensearch-project/security#2253)
  - [ML Commons PR #681](opensearch-project/ml-commons#681)
- Post-upgrade, the twoThirdsUpgradedClusterTask runs successfully, suggesting potential incompatibility between versions 1.1 and 2.10.

Testing:
- Executed `./gradlew bwcTestSuite -Dtests.security.manager=false` and all tests passed.

Signed-off-by: Kaituo Li <[email protected]>
opensearch-trigger-bot bot pushed a commit to opensearch-project/anomaly-detection that referenced this pull request Sep 8, 2023
…sue (#1029)

Issue:
- In versions between 1.1 and 2.10, the task :adBwcCluster#twoThirdsUpgradedClusterTask fails.
- Symptoms mirror those in [OpenSearch Issue #5076](opensearch-project/OpenSearch#5076).
- Logs show nodes from old and new versions failing to join as a cluster, resulting in a "master not found" exception.

Resolution:
- Upgraded the bwc version to 1.3.2, aligning with other plugins which use 1.3+ as their baseline.
  - [Cross-Cluster Replication PR #469](opensearch-project/cross-cluster-replication#469)
  - [Security PR #2253](opensearch-project/security#2253)
  - [ML Commons PR #681](opensearch-project/ml-commons#681)
- Post-upgrade, the twoThirdsUpgradedClusterTask runs successfully, suggesting potential incompatibility between versions 1.1 and 2.10.

Testing:
- Executed `./gradlew bwcTestSuite -Dtests.security.manager=false` and all tests passed.

Signed-off-by: Kaituo Li <[email protected]>
(cherry picked from commit 3c1a830)
jackiehanyang pushed a commit to opensearch-project/anomaly-detection that referenced this pull request Sep 8, 2023
…sue (#1029) (#1030)

Issue:
- In versions between 1.1 and 2.10, the task :adBwcCluster#twoThirdsUpgradedClusterTask fails.
- Symptoms mirror those in [OpenSearch Issue #5076](opensearch-project/OpenSearch#5076).
- Logs show nodes from old and new versions failing to join as a cluster, resulting in a "master not found" exception.

Resolution:
- Upgraded the bwc version to 1.3.2, aligning with other plugins which use 1.3+ as their baseline.
  - [Cross-Cluster Replication PR #469](opensearch-project/cross-cluster-replication#469)
  - [Security PR #2253](opensearch-project/security#2253)
  - [ML Commons PR #681](opensearch-project/ml-commons#681)
- Post-upgrade, the twoThirdsUpgradedClusterTask runs successfully, suggesting potential incompatibility between versions 1.1 and 2.10.

Testing:
- Executed `./gradlew bwcTestSuite -Dtests.security.manager=false` and all tests passed.

Signed-off-by: Kaituo Li <[email protected]>
(cherry picked from commit 3c1a830)

Co-authored-by: Kaituo Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants