Initialize feature flags before extensions manager

[cwperks]

Signed-off-by: Craig Perkins cwperx@amazon.com

Description

While testing with the experimental extensions feature flag, I found that the NoopExtensionsManager was being instantiated because the feature flags were not initialized by the time that extensions manager is being instantiated. This is a small PR to re-order Node.java to ensure the FeatureFlags are initialized before ExtensionsManager instantiation.

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
• Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

[cwperks]

@ryanbogan @saratvemulapalli @dbwiddis Small PR on the order of initializing feature flags before instantiating the ExtensionsManager.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: UNSTABLE ❕
• TEST FAILURES:

      1 org.opensearch.action.admin.cluster.node.tasks.ResourceAwareTasksTests.testTaskResourceTrackingDuringTaskCancellation

• URL: https://build.ci.opensearch.org/job/gradle-check/10198/
• CommitID: e42b21d
  Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/10201/
• CommitID: 0ba0e8c
  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?

[cwperks]

@ryanbogan Is this a flaky test failure? Can the gradle check be re-run?

Tests with failures:
 - org.opensearch.indices.replication.SegmentReplicationRelocationIT.testRelocateWhileContinuouslyIndexingAndWaitingForRefresh

[ryanbogan]

@ryanbogan Is this a flaky test failure? Can the gradle check be re-run?

Tests with failures:
 - org.opensearch.indices.replication.SegmentReplicationRelocationIT.testRelocateWhileContinuouslyIndexingAndWaitingForRefresh

I think that one is flaky

[saratvemulapalli]

Thanks @cwperks !!

Ideally a test should have caught this problem. @ryanbogan / @cwperks do you want to contribute a test for opensearch.yml having feature flag set and expecting ExtensionsManager to run?

[cwperks]

@saratvemulapalli Are there any example ITs for extensions? I am looking into a setup similar to this: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java#L57-L64

Do you think its possible to use internalCluster().getInstance(ExtensionManager.class) to get the extension manager that was instantiated to see if its a NoopExtensionManager vs an ExtensionManager?

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: TIMEOUT ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/10229/
• CommitID: f1afe52
  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?

[saratvemulapalli]

@saratvemulapalli Are there any example ITs for extensions? I am looking into a setup similar to this: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreIT.java#L57-L64

Do you think its possible to use internalCluster().getInstance(ExtensionManager.class) to get the extension manager that was instantiated to see if its a NoopExtensionManager vs an ExtensionManager?

@cwperks Thats a good question. It should be possible to do it as we inject it[1].

[1] https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/node/Node.java#L1010

[cwperks]

@saratvemulapalli @ryanbogan @andrross I added a test for this. When adding the test, I had to remove the feature flag check around binding the extensions manager otherwise the call to internalCluster().getInstance(ExtensionsManager.class) would always return ExtensionsManager and not the actual instance that was bound to the node. Please let me know if there is a better way to do this.

This means that when the experimental feature flag is not enabled, there will always be a NoopExtensionsManager that is binded. I figured that may alright, but want to double-check with the maintainers.

The test fails with the previous ordering.

[saratvemulapalli]

Thanks @cwperks !!

[saratvemulapalli]

@saratvemulapalli @ryanbogan @andrross I added a test for this. When adding the test, I had to remove the feature flag check around binding the extensions manager otherwise the call to internalCluster().getInstance(ExtensionsManager.class) would always return ExtensionsManager and not the actual instance that was bound to the node. Please let me know if there is a better way to do this.

This means that when the experimental feature flag is not enabled, there will always be a NoopExtensionsManager that is binded. I figured that may alright, but want to double-check with the maintainers.

The test fails with the previous ordering.

That should be good. thanks for taking care of the test.

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: FAILURE ❌
• URL: https://build.ci.opensearch.org/job/gradle-check/10484/
• CommitID: c006875
  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?

[cwperks]

Merged the latest changes in from main to re-run CI

[github-actions]

Gradle Check (Jenkins) Run Completed with:

• RESULT: SUCCESS ✅
• URL: https://build.ci.opensearch.org/job/gradle-check/10488/
• CommitID: 5e6dd6c