Skip to content

Commit

Permalink
[Backport 2.x] Pass localNode info to all plugins on node start (#8192)
Browse files Browse the repository at this point in the history
* [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 <dchanp@amazon.com>

* Adds this PR to changelog

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Makes onNodeStarted call plugin agnostic

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Marking onNodeStarted() as deprecated

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds a test to verify the behaviour of new signature of onNodeStarted

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Updates test to also check for nodeId

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* 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 <dchanp@amazon.com>

---------

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com>
(cherry picked from commit 40d7583)

* Adds a CHANGELOG entry

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

---------

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
  • Loading branch information
DarshitChanpura authored Jun 21, 2023
1 parent efd12df commit fced6d4
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/opensearch/node/Node.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
13 changes: 13 additions & 0 deletions server/src/main/java/org/opensearch/plugins/ClusterPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -86,7 +87,19 @@ default Map<String, ExistingShardsAllocator> 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();
}

}
Original file line number Diff line number Diff line change
@@ -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<Class<? extends Plugin>> 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;
}

}

0 comments on commit fced6d4

Please sign in to comment.