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

Add proxy support to RemoteClusterConnection #33062

Merged
merged 4 commits into from
Aug 25, 2018

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Aug 22, 2018

This adds support for connecting to a remote cluster through
a tcp proxy. A remote cluster can configured with an additional
search.remote.$clustername.proxy setting. This proxy will be used
to connect to remote nodes for every node connection established.
We still try to sniff the remote clsuter and connect to nodes directly
through the proxy which has to support some kind of routing to these nodes.
Yet, this routing mechanism requires the handshake request to include some
kind of information where to route to which is not yet implemented. The effort
to use the hostname and an optional node attribute for routing is tracked
in #32517

Closes #31840

This adds support for connecting to a remote cluster through
a tcp proxy. A remote cluster can configured with an additional
`search.remote.$clustername.proxy` setting. This proxy will be used
to connect to remote nodes for every node connection established.
We still try to sniff the remote clsuter and connect to nodes directly
through the proxy which has to support some kind of routing to these nodes.
Yet, this routing mechanism requires the handshake request to include some
kind of information where to route to which is not yet implemented. The effort
to use the hostname and an optional node attribute for routing is tracked
in elastic#32517

Closes elastic#31840
@s1monw s1monw added >enhancement :Distributed Coordination/Network Http and internode communication implementations v7.0.0 v6.5.0 labels Aug 22, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Contributor

@Tim-Brooks Tim-Brooks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments.

@@ -77,25 +97,37 @@ protected RemoteClusterAware(Settings settings) {
this.clusterNameResolver = new ClusterNameExpressionResolver(settings);
}

protected static Map<String, List<Supplier<DiscoveryNode>>> buildRemoteClustersSeeds(Settings settings) {
protected static Map<String, Tuple<String, List<Supplier<DiscoveryNode>>>> buildRemoteClustersSeeds(Settings settings) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a doc comment here? With the new work, it is really tough to figure out what is going on in this method.

throw new IllegalArgumentException("nodeMap must be non-empty");
}

return new StubbableTransport(MockTransportService.newMockTransport(Settings.EMPTY, Version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not extend and override StubbableTransport like this? Ideally maybe the class should be final. It provides methods to attach lambdas for "stubbing" the behavior (although I think the method will need to be made public). The method is either addConnectBehavior or you can add a setDefaultConnectbehavior. Similar to setDefaultSendBehavior.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The theoretical idea here is to try to move away from overriding methods like crazy at the transport level. So if refactorings need to happen, we can (hopefully) just move the stubs to different locations, opposed to dealing with a billion different tightly couple to the Transport interface tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ good call. I made the class final.

@s1monw s1monw requested a review from Tim-Brooks August 24, 2018 08:31
@s1monw
Copy link
Contributor Author

s1monw commented Aug 24, 2018

@tbrooks8 I pushed changes for your feedback

Copy link
Contributor

@Tim-Brooks Tim-Brooks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@s1monw s1monw merged commit 3376922 into elastic:master Aug 25, 2018
s1monw added a commit that referenced this pull request Aug 25, 2018
This adds support for connecting to a remote cluster through
a tcp proxy. A remote cluster can configured with an additional
`search.remote.$clustername.proxy` setting. This proxy will be used
to connect to remote nodes for every node connection established.
We still try to sniff the remote clsuter and connect to nodes directly
through the proxy which has to support some kind of routing to these nodes.
Yet, this routing mechanism requires the handshake request to include some
kind of information where to route to which is not yet implemented. The effort
to use the hostname and an optional node attribute for routing is tracked
in #32517

Closes #31840
@s1monw s1monw deleted the ccs_proxy_mode branch August 25, 2018 19:25
dnhatn added a commit that referenced this pull request Aug 26, 2018
* master:
  Add proxy support to RemoteClusterConnection (#33062)
  TEST: Skip assertSeqNos for closed shards (#33130)
  TEST: resync operation on replica should acquire shard permit (#33103)
  Switch remaining x-pack tests to new style Requests (#33108)
  Switch remaining tests to new style Requests (#33109)
  Switch remaining ml tests to new style Requests (#33107)
  Build: Line up IDE detection logic
  Security index expands to a single replica (#33131)
  HLRC: request/response homogeneity and JavaDoc improvements (#33133)
  Checkstyle!
  [Test] Fix sporadic failure in MembershipActionTests
  Revert "Do NOT allow termvectors on nested fields (#32728)"
  [Rollup] Move toAggCap() methods out of rollup config objects (#32583)
  Fix race condition in scheduler engine test
dnhatn added a commit that referenced this pull request Aug 26, 2018
* 6.x:
  Allow engine to recover from translog upto a seqno (#33032)
  TEST: Skip assertSeqNos for closed shards (#33130)
  TEST: resync operation on replica should acquire shard permit (#33103)
  Add proxy support to RemoteClusterConnection (#33062)
  Build: Line up IDE detection logic
  Security index expands to a single replica (#33131)
  Suppress more tests
  HLRC: request/response homogeneity and JavaDoc improvements (#33133)
  [Rollup] Move toAggCap() methods out of rollup config objects (#32583)
  Muted all these tests due to #33128
  Fix race condition in scheduler engine test
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 26, 2018
* master:
  Fix a mappings update test (elastic#33146)
  Reload Secure Settings REST specs & docs (elastic#32990)
  Refactor CachingUsernamePassword realm (elastic#32646)
  Add proxy support to RemoteClusterConnection (elastic#33062)
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Mar 20, 2019
In elastic#33062 we introduced the `cluster.remote.*.proxy` setting for proxied
connections to remote clusters, but left it deliberately undocumented since it
needed followup work so that it could work with SNI. However, since elastic#32517 is
now closed we can add this documentation and remove the comment about its lack
of documentation.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Mar 28, 2019
In elastic#33062 we introduced the `cluster.remote.*.proxy` setting for proxied
connections to remote clusters, but left it deliberately undocumented since it
needed followup work so that it could work with SNI. However, since elastic#32517 is
now closed we can add this documentation and remove the comment about its lack
of documentation.
DaveCTurner added a commit that referenced this pull request Mar 28, 2019
In #33062 we introduced the `cluster.remote.*.proxy` setting for proxied
connections to remote clusters, but left it deliberately undocumented since it
needed followup work so that it could work with SNI. However, since #32517 is
now closed we can add this documentation and remove the comment about its lack
of documentation.
DaveCTurner added a commit that referenced this pull request Mar 28, 2019
In #33062 we introduced the `cluster.remote.*.proxy` setting for proxied
connections to remote clusters, but left it deliberately undocumented since it
needed followup work so that it could work with SNI. However, since #32517 is
now closed we can add this documentation and remove the comment about its lack
of documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants