-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Integration test that checks for settings upgrade #1482
Integration test that checks for settings upgrade #1482
Conversation
Signed-off-by: Megha Sai Kavikondala <kavmegha@amazon.com>
Can one of the admins verify this patch? |
start gradle check |
✅ Gradle Wrapper Validation success a35a739 |
✅ Gradle Precommit success a35a739 |
builder.field("search.remote.foo.skip_unavailable", true); | ||
builder.field("search.remote.foo.seeds", Collections.singletonList("localhost:9200")); | ||
builder.field("search.remote.foo.proxy", "localhost:9200"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to understand, are these settings deprecated in newer versions of OpenSearch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the newer version, the search.remote settings have been deprecated through 'Setting.Property.Deprecated' value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
Looks like we are re-using most of the code from https://github.com/opensearch-project/OpenSearch/blob/main/qa/full-cluster-restart/src/test/java/org/opensearch/upgrades/FullClusterRestartSettingsUpgradeIT.java.
Isn't this already testing these remote settings and are also deprecated? or Am I missing something here?
Changes made by deleting the TestSettingsIT file and adding new lines in FullClusterRestartSettingsUpgradeIT.java
start gradle check |
✅ Gradle Wrapper Validation success 0082131 |
✅ Gradle Precommit success 0082131 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @meghasaik !
* Made changes. Signed-off-by: Megha Sai Kavikondala <kavmegha@amazon.com> * Signed-off-by: Megha Sai Kavikondala <kavmegha@amazon.com> Changes made by deleting the TestSettingsIT file and adding new lines in FullClusterRestartSettingsUpgradeIT.java Signed-off-by: Megha Sai Kavikondala <kavmegha@amazon.com>
…- [ BACKPORT-1.x ] (#1601) * Integration test that checks for settings upgrade (#1482) * Made changes. Signed-off-by: Megha Sai Kavikondala <kavmegha@amazon.com> * Signed-off-by: Megha Sai Kavikondala <kavmegha@amazon.com> Changes made by deleting the TestSettingsIT file and adding new lines in FullClusterRestartSettingsUpgradeIT.java Signed-off-by: Megha Sai Kavikondala <kavmegha@amazon.com> * Signed-off-by: Megha Sai Kavikondala <kavmegha@amazon.com> Informative error messages related to empty index name.[Backport]
Signed-off-by: Megha Sai Kavikondala kavmegha@amazon.com
Description
[Describe what this change achieves]
An Integration test which checks for the settings upgrade by setting the legacy settings for the previous version of ODFE/ES in the old cluster and after upgrading it to the new version, checks if the settings fallback to the previously set settings.
Issues Resolved
#670
Check List
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.