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

[Enhancement] Pass localNode info to all plugins on node start #7919

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Change `com.amazonaws.sdk.stsEndpointOverride` to `aws.stsEndpointOverride` ([7372](https://github.com/opensearch-project/OpenSearch/pull/7372/))
- Align range and default value for deletes_pct_allowed in merge policy ([#7730](https://github.com/opensearch-project/OpenSearch/pull/7730))
- Rename QueryPhase actors like Suggest, Rescore to be processors rather than phase ([#8025](https://github.com/opensearch-project/OpenSearch/pull/8025))
- Pass localNode info to all plugins on node start ([#7919](https://github.com/opensearch-project/OpenSearch/pull/7919))

### Deprecated

Expand Down Expand Up @@ -138,4 +139,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Security

[Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.x...HEAD
[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.8...2.x
[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.8...2.x
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 @@ -1356,7 +1356,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
peternied marked this conversation as resolved.
Show resolved Hide resolved
default void onNodeStarted() {}
peternied marked this conversation as resolved.
Show resolved Hide resolved

/**
* 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() {
peternied marked this conversation as resolved.
Show resolved Hide resolved

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;
}

}