From fced6d42d293925331f02650e41af2b01962856d Mon Sep 17 00:00:00 2001 From: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com> Date: Wed, 21 Jun 2023 10:39:17 -0400 Subject: [PATCH] [Backport 2.x] Pass localNode info to all plugins on node start (#8192) * [Enhancement] Pass localNode info to all plugins on node start (#7919) * Updates Node start to now pass localNode info so that it is available directly to a plugin instead of hacky implementation Signed-off-by: Darshit Chanpura * Adds this PR to changelog Signed-off-by: Darshit Chanpura * Makes onNodeStarted call plugin agnostic Signed-off-by: Darshit Chanpura * Marking onNodeStarted() as deprecated Signed-off-by: Darshit Chanpura * Adds a test to verify the behaviour of new signature of onNodeStarted Signed-off-by: Darshit Chanpura * Updates test to also check for nodeId Signed-off-by: Darshit Chanpura * Adds instructions to use the overloaded implementation of onNodeStarted and adds a test to check that current plugins are not affect with this implementation Signed-off-by: Darshit Chanpura --------- Signed-off-by: Darshit Chanpura Signed-off-by: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com> (cherry picked from commit 40d758387bd9227314a64a73035260829bfc3c10) * Adds a CHANGELOG entry Signed-off-by: Darshit Chanpura --------- Signed-off-by: Darshit Chanpura --- CHANGELOG.md | 1 + .../main/java/org/opensearch/node/Node.java | 2 +- .../org/opensearch/plugins/ClusterPlugin.java | 13 ++++ .../plugins/ClusterPluginTests.java | 78 +++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/org/opensearch/plugins/ClusterPluginTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index c2dfc7028a1ea..7a22b02bdf060 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Rename QueryPhase actors like Suggest, Rescore to be processors rather than phase ([#8025](https://github.com/opensearch-project/OpenSearch/pull/8025)) - [Snapshot Interop] Add Changes in Create Snapshot Flow for remote store interoperability. ([#8071](https://github.com/opensearch-project/OpenSearch/pull/8071)) - Allow insecure string settings to warn-log usage and advise to migration of a newer secure variant ([#5496](https://github.com/opensearch-project/OpenSearch/pull/5496)) +- Pass localNode info to all plugins on node start ([#7919](https://github.com/opensearch-project/OpenSearch/pull/7919) ### Deprecated diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index eb1b97647896d..101a8f15b2558 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -1358,7 +1358,7 @@ public void onTimeout(TimeValue timeout) { logger.info("started"); - pluginsService.filterPlugins(ClusterPlugin.class).forEach(ClusterPlugin::onNodeStarted); + pluginsService.filterPlugins(ClusterPlugin.class).forEach(plugin -> plugin.onNodeStarted(clusterService.localNode())); return this; } diff --git a/server/src/main/java/org/opensearch/plugins/ClusterPlugin.java b/server/src/main/java/org/opensearch/plugins/ClusterPlugin.java index b0ed91dd4c468..c2e147b86d17f 100644 --- a/server/src/main/java/org/opensearch/plugins/ClusterPlugin.java +++ b/server/src/main/java/org/opensearch/plugins/ClusterPlugin.java @@ -32,6 +32,7 @@ package org.opensearch.plugins; +import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.routing.allocation.ExistingShardsAllocator; import org.opensearch.cluster.routing.allocation.allocator.ShardsAllocator; import org.opensearch.cluster.routing.allocation.decider.AllocationDecider; @@ -86,7 +87,19 @@ default Map getExistingShardsAllocators() { /** * Called when the node is started + * + * DEPRECATED: Use {@link #onNodeStarted(DiscoveryNode)} for newer implementations. */ + @Deprecated default void onNodeStarted() {} + /** + * Called when node is started. DiscoveryNode argument is passed to allow referring localNode value inside plugin + * + * @param localNode local Node info + */ + default void onNodeStarted(DiscoveryNode localNode) { + onNodeStarted(); + } + } diff --git a/server/src/test/java/org/opensearch/plugins/ClusterPluginTests.java b/server/src/test/java/org/opensearch/plugins/ClusterPluginTests.java new file mode 100644 index 0000000000000..c90d4e2789e82 --- /dev/null +++ b/server/src/test/java/org/opensearch/plugins/ClusterPluginTests.java @@ -0,0 +1,78 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.plugins; + +import org.opensearch.cluster.node.DiscoveryNode; +import org.opensearch.test.OpenSearchSingleNodeTestCase; + +import java.util.Collection; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +public class ClusterPluginTests extends OpenSearchSingleNodeTestCase { + + @Override + protected Collection> getPlugins() { + return pluginList(DummyClusterPlugin.class, DummyClusterPlugin2.class); + } + + public void testOnNodeStarted_shouldContainLocalNodeInfo() { + + DiscoveryNode localNode = DummyClusterPlugin.getLocalNode(); + + assertTrue(localNode != null); + // TODO Figure out if there is a way to check ephemeralId + assertTrue(localNode.getId().equals(node().getNodeEnvironment().nodeId())); + } + + public void testOnNodeStarted_shouldCallDeprecatedMethod() { + DummyClusterPlugin2 dummyClusterPlugin2 = mock(DummyClusterPlugin2.class); + dummyClusterPlugin2.onNodeStarted(); + verify(dummyClusterPlugin2, times(1)).onNodeStarted(); + + DiscoveryNode localNode = DummyClusterPlugin2.getLocalNode(); + assertTrue(localNode != null); + } + +} + +final class DummyClusterPlugin extends Plugin implements ClusterPlugin { + + private static volatile DiscoveryNode localNode; + + public DummyClusterPlugin() {} + + @Override + public void onNodeStarted(DiscoveryNode localNode) { + DummyClusterPlugin.localNode = localNode; + } + + public static DiscoveryNode getLocalNode() { + return localNode; + } +} + +class DummyClusterPlugin2 extends Plugin implements ClusterPlugin { + + private static volatile DiscoveryNode localNode; + + public DummyClusterPlugin2() {} + + @Override + public void onNodeStarted() { + localNode = mock(DiscoveryNode.class); + } + + public static DiscoveryNode getLocalNode() { + return localNode; + } + +}