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

Fix PluginInfo bwc for opensearch_version field #12543

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

abseth-amzn
Copy link
Contributor

@abseth-amzn abseth-amzn commented Mar 6, 2024

Description

Updated opensearch_version field in PluginInfo json to represent a string of single version as opposed to a list of single version to ensure bwc.

Without this fix

/_cluster/stats

"plugins" : [
        {
          "name" : "analysis-icu",
          "version" : "3.0.0-SNAPSHOT",
          "opensearch_version" : [
            "3.0.0"
          ],
          "java_version" : "11",
          "description" : "The ICU Analysis plugin integrates the Lucene ICU module into OpenSearch, adding ICU-related analysis components.",
          "classname" : "org.opensearch.plugin.analysis.icu.AnalysisICUPlugin",
          "custom_foldername" : "",
          "extended_plugins" : [ ],
          "has_native_controller" : false
        }
      ],

With fix:

    "plugins" : [
      {
        "name" : "analysis-icu",
        "version" : "3.0.0-SNAPSHOT",
        "opensearch_version" : "3.0.0",
        "java_version" : "11",
        "description" : "The ICU Analysis plugin integrates the Lucene ICU module into OpenSearch, adding ICU-related analysis components.",
        "classname" : "org.opensearch.plugin.analysis.icu.AnalysisICUPlugin",
        "custom_foldername" : "",
        "extended_plugins" : [ ],
        "has_native_controller" : false
      }
    ],

Related Issues

Resolves #[12528]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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: Abhilasha Seth <abseth@amazon.com>
Copy link
Contributor

github-actions bot commented Mar 6, 2024

Compatibility status:

Checks if related components are compatible with change 4bd718d

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/flow-framework.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/performance-analyzer.git]

Copy link
Contributor

github-actions bot commented Mar 6, 2024

❕ Gradle check result for ed67d25: UNSTABLE

  • TEST FAILURES:
      1 org.opensearch.remotestore.RemoteStoreStatsIT.testDownloadStatsCorrectnessSinglePrimaryMultipleReplicaShards

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Copy link

codecov bot commented Mar 6, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 71.46%. Comparing base (11836d0) to head (4bd718d).
Report is 2 commits behind head on main.

Files Patch % Lines
...c/main/java/org/opensearch/plugins/PluginInfo.java 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #12543      +/-   ##
============================================
+ Coverage     71.32%   71.46%   +0.13%     
- Complexity    59887    59972      +85     
============================================
  Files          4982     4982              
  Lines        282152   282152              
  Branches      40940    40940              
============================================
+ Hits         201239   201627     +388     
+ Misses        64187    63762     -425     
- Partials      16726    16763      +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I think this fixes the regression, thanks. A string with multiple numbers won't break future compatibility, either, so I am good with this as is.

@shwetathareja any must have's from you?

Signed-off-by: Abhilasha Seth <abseth@amazon.com>
Copy link
Contributor

github-actions bot commented Mar 7, 2024

❌ Gradle check result for 4bd718d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@dblock
Copy link
Member

dblock commented Mar 7, 2024

❌ Gradle check result for 4bd718d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

org.opensearch.upgrades.IndexingIT.testIndexingWithSegRep
#12336

Copy link
Contributor

github-actions bot commented Mar 7, 2024

❌ Gradle check result for 4bd718d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@dblock
Copy link
Member

dblock commented Mar 7, 2024

❌ Gradle check result for 4bd718d: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

org.opensearch.join.query.ChildQuerySearchIT.testHasChildInnerHitsHighlighting {p0={"search.concurrent_segment_search.enabled":"false"}}
org.opensearch.join.query.ChildQuerySearchIT.classMethod

#10884

Copy link
Contributor

github-actions bot commented Mar 7, 2024

✅ Gradle check result for 4bd718d: SUCCESS

@dblock dblock requested a review from shwetathareja March 7, 2024 18:33
@dblock dblock added the backport 2.x Backport to 2.x branch label Mar 7, 2024
@kotwanikunal
Copy link
Member

@shwetathareja / @abseth-amzn Please resolve the conversations for a merge.

@kotwanikunal
Copy link
Member

@abseth-amzn Can you please add a test to cover the exception scenario as well?

@abseth-amzn
Copy link
Contributor Author

abseth-amzn commented Mar 12, 2024

@abseth-amzn Can you please add a test to cover the exception scenario as well?

PluginInfo enforces the range size to be exactly one -

if (opensearchVersionRanges.size() != 1) {

Hence its not possible to simulate the empty ranges situation (in a test) that would throw the exception.

@kotwanikunal kotwanikunal merged commit a702f6a into opensearch-project:main Mar 12, 2024
37 of 38 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 12, 2024
* Fix PluginInfo bwc for opensearch_version field

Signed-off-by: Abhilasha Seth <abseth@amazon.com>

* Update how empty range list is handled

Signed-off-by: Abhilasha Seth <abseth@amazon.com>

---------

Signed-off-by: Abhilasha Seth <abseth@amazon.com>
(cherry picked from commit a702f6a)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
kotwanikunal pushed a commit that referenced this pull request Mar 12, 2024
* Fix PluginInfo bwc for opensearch_version field



* Update how empty range list is handled



---------


(cherry picked from commit a702f6a)

Signed-off-by: Abhilasha Seth <abseth@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Mar 18, 2024
…2543)

* Fix PluginInfo bwc for opensearch_version field

Signed-off-by: Abhilasha Seth <abseth@amazon.com>

* Update how empty range list is handled

Signed-off-by: Abhilasha Seth <abseth@amazon.com>

---------

Signed-off-by: Abhilasha Seth <abseth@amazon.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…2543)

* Fix PluginInfo bwc for opensearch_version field

Signed-off-by: Abhilasha Seth <abseth@amazon.com>

* Update how empty range list is handled

Signed-off-by: Abhilasha Seth <abseth@amazon.com>

---------

Signed-off-by: Abhilasha Seth <abseth@amazon.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch bug Something isn't working Plugins skip-changelog v2.13.0 Issues and PRs related to version 2.13.0
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[BUG] GET /_cluster/stats and GET /_nodes bcw broken
4 participants