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 build utility to check cluster health over ssl #40573

Merged
merged 3 commits into from
Apr 1, 2019

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Mar 28, 2019

By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.

Closes: #38072

By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.
@tvernum tvernum added >test Issues or PRs that are addressing/adding tests :Delivery/Build Build or test infrastructure v7.0.0 v8.0.0 v7.2.0 v6.7.1 labels Mar 28, 2019
@tvernum tvernum requested review from alpar-t and jkakavas March 28, 2019 05:33
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@tvernum
Copy link
Contributor Author

tvernum commented Mar 28, 2019

@elasticmachine run elasticsearch-ci/2

Copy link
Contributor

@alpar-t alpar-t left a comment

Choose a reason for hiding this comment

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

Thanks! I really like this change, much better to have stuff like this in buildSrc implemented in java and with tests.
I left a single comment.

this.password = password;
}

public boolean wait(int durationInMs) throws GeneralSecurityException, InterruptedException, IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we refactor this to separate the wait/retry logic from the actual check ?
This will be useful for testclusters too, where a boolean return code is not needed and the wait/retry logic is provided for all cluster checks, but even if we don't consider that use-case I think separating these would make it much easier to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 to some kind of generic wait and retry utility if we don't already have one.

@mark-vieira
Copy link
Contributor

As it is this utility will only work with via HTTPS if some custom truststore info is provided. Do we think we'll ever have a desire to use this yet not provide a custom truststore?

@tvernum
Copy link
Contributor Author

tvernum commented Mar 29, 2019

@mark-vieira That's not intentional. If you pass https as the protocol and don't configure a custom truststore then it should just rely on the JDK truststore.

@mark-vieira
Copy link
Contributor

I think I was reading the logic wrong. I interpreted it to be that an error would be thrown in that case but you are right, it looks like it should just use the default SSLContext in that scenario. Disregard.

Copy link
Contributor

@alpar-t alpar-t 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

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

LGTM

@colings86 colings86 added v6.7.2 and removed v6.7.1 labels Mar 30, 2019
@tvernum tvernum merged commit c652917 into elastic:master Apr 1, 2019
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Apr 1, 2019
By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.

Resolves: elastic#38072
Backport of: elastic#40573
tvernum added a commit that referenced this pull request Apr 4, 2019
By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.

Backport of: #40573
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Apr 5, 2019
By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.

Backport of: elastic#40573
tvernum added a commit that referenced this pull request Apr 8, 2019
By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.

Backport of: #40573
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Apr 8, 2019
By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.

Resolves: elastic#38072
Backport of: elastic#40573
tvernum added a commit that referenced this pull request Apr 8, 2019
By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.

Backport of: #40573
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
By default, in integ tests we wait for the standalone cluster to start
by using the ant Get task to retrieve the cluster health endpoint.
However the ant task has no facilities for customising the trusted
CAs for a https resource, so if the integ test cluster has TLS enabled
on the http interface (using a custom CA) we need a separate utility
for that purpose.

Resolves: elastic#38072
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team >test Issues or PRs that are addressing/adding tests v6.7.2 v7.0.0 v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create gradle task/utility to wait-for-status with custom SSL trust
7 participants