Skip to content

Conversation

@ywangd
Copy link
Member

@ywangd ywangd commented Jul 31, 2020

When a new cluster starts, the HTTP layer become ready to accept incoming requests while the basic license is still being generated in the background. When a get license request comes in before the license is ready, it can get 404 error. This PR fixes it by either: ensure the cluster is ready before issue the license request or wrap the license request in assertBusy.

Resolves: #59157
Resolves: #60468
Resolves: #60519

@ywangd ywangd added >test Issues or PRs that are addressing/adding tests :Security/License License functionality for commercial features v8.0.0 v7.8.2 v7.10.0 v7.9.1 labels Jul 31, 2020
@ywangd ywangd changed the title [Test] Fix get-license test failure by ensure cluster is ready Fix get-license test failure by ensure cluster is ready Jul 31, 2020
@ywangd ywangd marked this pull request as ready for review August 3, 2020 01:12
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/License)

@elasticmachine elasticmachine added the Team:Security Meta label for security team label Aug 3, 2020
@ywangd ywangd requested a review from tvernum August 3, 2020 01:12

protected boolean isGetLicenseTest() {
String testName = getTestName();
return testName != null && (testName.contains("/get-license/") || testName.contains("\\get-license\\"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this need to support \\ but ccr doesn't?

Copy link
Member Author

Choose a reason for hiding this comment

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

My guess is that ccr tests do not run on Windows? There are other similar methods in this file, e.g. isMachineLearningTest, isTransformTest and they all check for the backslash'd variant. The CI does have machine learning specific jobs so I think they are justified.

For license check, I decided to add it because we do have multijob-windows-compatibility jobs. I am not completely sure that these jobs run the license check. But it's easy to add so I just did it.

@ywangd ywangd merged commit ec1f4f1 into elastic:master Aug 3, 2020
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 3, 2020
When a new cluster starts, the HTTP layer becomes ready to accept incoming 
requests while the basic license is still being populated in the background. 
When a get license request comes in before the license is ready, it can get 
404 error. This PR fixes it by either wrap the license check in assertBusy or 
ensure the license is ready before perform the check.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 3, 2020
When a new cluster starts, the HTTP layer becomes ready to accept incoming 
requests while the basic license is still being populated in the background. 
When a get license request comes in before the license is ready, it can get 
404 error. This PR fixes it by either wrap the license check in assertBusy or 
ensure the license is ready before perform the check.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 3, 2020
When a new cluster starts, the HTTP layer becomes ready to accept incoming 
requests while the basic license is still being populated in the background. 
When a get license request comes in before the license is ready, it can get 
404 error. This PR fixes it by either wrap the license check in assertBusy or 
ensure the license is ready before perform the check.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 3, 2020
ywangd added a commit that referenced this pull request Aug 3, 2020
…60573)

This is a follow up for #60498 to ensure an AssertionError is throw when the license is ready.
The client throws a ResponseException for 404 status code. It needs to be converted to an AssertionError to correctly work with assertBusy.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 3, 2020
…lastic#60573)

This is a follow up for elastic#60498 to ensure an AssertionError is throw when the license is ready.
The client throws a ResponseException for 404 status code. It needs to be converted to an AssertionError to correctly work with assertBusy.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 3, 2020
…lastic#60573)

This is a follow up for elastic#60498 to ensure an AssertionError is throw when the license is ready.
The client throws a ResponseException for 404 status code. It needs to be converted to an AssertionError to correctly work with assertBusy.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 3, 2020
…lastic#60573)

This is a follow up for elastic#60498 to ensure an AssertionError is throw when the license is ready.
The client throws a ResponseException for 404 status code. It needs to be converted to an AssertionError to correctly work with assertBusy.
ywangd added a commit that referenced this pull request Aug 3, 2020
)

When a new cluster starts, the HTTP layer becomes ready to accept incoming 
requests while the basic license is still being populated in the background. 
When a get license request comes in before the license is ready, it can get 
404 error. This PR fixes it by either wrap the license check in assertBusy or 
ensure the license is ready before perform the check.

This is a backport for both #60498 and #60573
ywangd added a commit that referenced this pull request Aug 3, 2020
)

When a new cluster starts, the HTTP layer becomes ready to accept incoming 
requests while the basic license is still being populated in the background. 
When a get license request comes in before the license is ready, it can get 
404 error. This PR fixes it by either wrap the license check in assertBusy or 
ensure the license is ready before perform the check.

This is a backport for both #60498 and #60573
ywangd added a commit that referenced this pull request Aug 3, 2020
)

When a new cluster starts, the HTTP layer becomes ready to accept incoming 
requests while the basic license is still being populated in the background. 
When a get license request comes in before the license is ready, it can get 
404 error. This PR fixes it by either wrap the license check in assertBusy or 
ensure the license is ready before perform the check.

This is a backport for both #60498 and #60573
ywangd added a commit that referenced this pull request Aug 5, 2020
Fix another variant of missing license test failure similar to the cases fixed by #60498.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 5, 2020
Fix another variant of missing license test failure similar to the cases fixed by elastic#60498.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 5, 2020
Fix another variant of missing license test failure similar to the cases fixed by elastic#60498.
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Aug 5, 2020
Fix another variant of missing license test failure similar to the cases fixed by elastic#60498.
ywangd added a commit that referenced this pull request Aug 5, 2020
Fix another variant of missing license test failure similar to the cases fixed by #60498.
ywangd added a commit that referenced this pull request Aug 5, 2020
Fix another variant of missing license test failure similar to the cases fixed by #60498.
ywangd added a commit that referenced this pull request Aug 5, 2020
Fix another variant of missing license test failure similar to the cases fixed by #60498.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Security/License License functionality for commercial features Team:Security Meta label for security team >test Issues or PRs that are addressing/adding tests v7.8.2 v7.9.1 v7.10.0 v8.0.0-alpha1

Projects

None yet

4 participants