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.

Sign up for GitHub

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

Jump to bottom

[Transform] Remove node.attr.transform.remote_connect and use new remote node role #54217

Merged
merged 1 commit into from
Mar 25, 2020
Merged

[Transform] Remove node.attr.transform.remote_connect and use new remote node role #54217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 1 addition & 5 deletions x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/Transform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.indices.SystemIndexDescriptor;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.node.Node;
import org.elasticsearch.persistent.PersistentTasksExecutor;
import org.elasticsearch.plugins.PersistentTaskPlugin;
import org.elasticsearch.plugins.Plugin;
Expand Down Expand Up @@ -148,7 +147,6 @@ public class Transform extends Plugin implements SystemIndexPlugin, PersistentTa
* These attributes should never be set directly, use the node setting counter parts instead.
*/
public static final String TRANSFORM_ENABLED_NODE_ATTR = "transform.node";
public static final String TRANSFORM_REMOTE_ENABLED_NODE_ATTR = "transform.remote_connect";

/**
* Setting whether transform (the coordinator task) can run on this node and REST API's are available,
Expand Down Expand Up @@ -355,9 +353,8 @@ public List<Setting<?>> getSettings() {
@Override
public Settings additionalSettings() {
String transformEnabledNodeAttribute = "node.attr." + TRANSFORM_ENABLED_NODE_ATTR;
String transformRemoteEnabledNodeAttribute = "node.attr." + TRANSFORM_REMOTE_ENABLED_NODE_ATTR;

if (settings.get(transformEnabledNodeAttribute) != null || settings.get(transformRemoteEnabledNodeAttribute) != null) {
if (settings.get(transformEnabledNodeAttribute) != null) {
throw new IllegalArgumentException(
"Directly setting transform node attributes is not permitted, please use the documented node settings instead"
);
Expand All @@ -370,7 +367,6 @@ public Settings additionalSettings() {
Settings.Builder additionalSettings = Settings.builder();

additionalSettings.put(transformEnabledNodeAttribute, TRANSFORM_ENABLED_NODE.get(settings));
additionalSettings.put(transformRemoteEnabledNodeAttribute, Node.NODE_REMOTE_CLUSTER_CLIENT.get(settings));

return additionalSettings.build();
}
Expand Down
2 changes: 1 addition & 1 deletion ...n/java/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public static boolean nodeCanRunThisTransform(DiscoveryNode node, TransformTaskP
}

// does the transform require a remote and remote is enabled?
if (params.requiresRemote() && Boolean.parseBoolean(nodeAttributes.get(Transform.TRANSFORM_REMOTE_ENABLED_NODE_ATTR)) == false) {
if (params.requiresRemote() && node.isRemoteClusterClient() == false) {
if (explain != null) {
explain.put(node.getId(), "transform requires a remote connection but remote is disabled");
}
Expand Down
19 changes: 2 additions & 17 deletions x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/TransformTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,12 @@ public void testNodeAttributes() {
Settings.Builder builder = Settings.builder();
boolean transformEnabled = randomBoolean();
boolean transformPluginEnabled = randomBoolean();
boolean remoteClusterClient = randomBoolean();

// randomly use explicit or default setting
if ((transformEnabled && randomBoolean()) == false) {
builder.put("node.transform", transformEnabled);
}

// randomly use explicit or default setting
if ((remoteClusterClient && randomBoolean()) == false) {
builder.put("node.remote_cluster_client", remoteClusterClient);
}

if (transformPluginEnabled == false) {
builder.put("xpack.transform.enabled", transformPluginEnabled);
}
Expand All @@ -42,23 +36,14 @@ public void testNodeAttributes() {
transformPluginEnabled && transformEnabled,
Boolean.parseBoolean(transform.additionalSettings().get("node.attr.transform.node"))
);
assertEquals(
transformPluginEnabled && remoteClusterClient,
Boolean.parseBoolean(transform.additionalSettings().get("node.attr.transform.remote_connect"))
);
}

public void testNodeAttributesDirectlyGiven() {
Settings.Builder builder = Settings.builder();

if (randomBoolean()) {
builder.put("node.attr.transform.node", randomBoolean());
} else {
builder.put("node.attr.transform.remote_connect", randomBoolean());
}
builder.put("node.attr.transform.node", randomBoolean());

Transform transform = createTransform(builder.build());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> transform.additionalSettings());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, transform::additionalSettings);
assertThat(
e.getMessage(),
equalTo("Directly setting transform node attributes is not permitted, please use the documented node settings instead")
Expand Down
23 changes: 10 additions & 13 deletions ...a/org/elasticsearch/xpack/transform/transforms/TransformPersistentTasksExecutorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,19 +243,14 @@ private DiscoveryNodes.Builder buildNodes(
boolean dedicatedTransformNode,
boolean pastDataNode,
boolean transformRemoteNodes,
boolean transformLocanOnlyNodes,
boolean transformLocalOnlyNodes,
boolean currentDataNode
) {

Map<String, String> transformNodeAttributes = new HashMap<>();
transformNodeAttributes.put(Transform.TRANSFORM_ENABLED_NODE_ATTR, "true");
transformNodeAttributes.put(Transform.TRANSFORM_REMOTE_ENABLED_NODE_ATTR, "true");
Map<String, String> transformNodeAttributesDisabled = new HashMap<>();
transformNodeAttributesDisabled.put(Transform.TRANSFORM_ENABLED_NODE_ATTR, "false");
transformNodeAttributesDisabled.put(Transform.TRANSFORM_REMOTE_ENABLED_NODE_ATTR, "true");
Map<String, String> transformNodeAttributesNoRemote = new HashMap<>();
transformNodeAttributesNoRemote.put(Transform.TRANSFORM_ENABLED_NODE_ATTR, "true");
transformNodeAttributesNoRemote.put(Transform.TRANSFORM_REMOTE_ENABLED_NODE_ATTR, "false");

DiscoveryNodes.Builder nodes = DiscoveryNodes.builder();

Expand All @@ -265,7 +260,7 @@ private DiscoveryNodes.Builder buildNodes(
"dedicated-transform-node",
buildNewFakeTransportAddress(),
transformNodeAttributes,
Collections.singleton(DiscoveryNodeRole.MASTER_ROLE),
new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)),
Version.CURRENT
)
);
Expand All @@ -277,7 +272,9 @@ private DiscoveryNodes.Builder buildNodes(
"past-data-node-1",
buildNewFakeTransportAddress(),
transformNodeAttributes,
new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE)),
new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE,
DiscoveryNodeRole.MASTER_ROLE,
DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)),
Version.V_7_7_0
)
);
Expand All @@ -289,7 +286,7 @@ private DiscoveryNodes.Builder buildNodes(
"current-data-node-with-2-tasks",
buildNewFakeTransportAddress(),
transformNodeAttributes,
new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE)),
new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)),
Version.CURRENT
)
)
Expand All @@ -298,18 +295,18 @@ private DiscoveryNodes.Builder buildNodes(
"current-data-node-with-1-tasks",
buildNewFakeTransportAddress(),
transformNodeAttributes,
new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE)),
new HashSet<>(Arrays.asList(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE)),
Version.CURRENT
)
);
}

if (transformLocanOnlyNodes) {
if (transformLocalOnlyNodes) {
nodes.add(
new DiscoveryNode(
"current-data-node-with-0-tasks-transform-remote-disabled",
buildNewFakeTransportAddress(),
transformNodeAttributesNoRemote,
transformNodeAttributes,
new HashSet<>(Arrays.asList(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE)),
Version.CURRENT
)
Expand All @@ -322,7 +319,7 @@ private DiscoveryNodes.Builder buildNodes(
"current-data-node-with-transform-disabled",
buildNewFakeTransportAddress(),
transformNodeAttributesDisabled,
Set.of(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE),
Set.of(DiscoveryNodeRole.DATA_ROLE, DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE),
Version.CURRENT
)
);
Expand Down