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

Updates integTest behavior to run against local and remote cluster #1088

Merged

Conversation

DarshitChanpura
Copy link
Member

Recent CI build failed with:


* Where:
Build file '/tmp/tmp19lh39y9/index-management/build.gradle' line: 441

* What went wrong:
Execution failed for task ':integTest'.
> `cluster{::integTest}` failed to wait for cluster health yellow after 40 SECONDS
    Unexpected end of file from server

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
> Get more help at https://help.gradle.org.

BUILD FAILED in 6m 23s

This is because the previous PR: #1076 assumed that the password will always be admin for integTest task since a custom configuration is applied for setup. However, that is not the case when integTest is triggered via opensearch-build as it uses a remote-cluster.
Hence, this PR defaults integTest task user password to admin for local cluster and conditionally accepts the password if running against remote cluster.

CheckList:

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Copy link

codecov bot commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ba24f86) 74.91% compared to head (be40dc9) 74.93%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1088      +/-   ##
============================================
+ Coverage     74.91%   74.93%   +0.02%     
  Complexity     2813     2813              
============================================
  Files           367      367              
  Lines         16522    16522              
  Branches       2363     2363              
============================================
+ Hits          12377    12381       +4     
+ Misses         2843     2840       -3     
+ Partials       1302     1301       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@bowenlan-amzn bowenlan-amzn left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@r1walz
Copy link
Collaborator

r1walz commented Feb 5, 2024

Hi, @DarshitChanpura. QQ: As per #1064, there an effort to reduce usage of "admin" or other custom strings for providing password inputs and it's recommended to use OPENSEARCH_INITIAL_ADMIN_PASSWORD env/system variable. Should we incorporate that here as well? I'm unsure how this flow works. Please, provide clarification.

Moreover, I see that logs for jenkins/index-management/with-security is filled with following errors:

[2024-02-05T13:31:20,070][ERROR][o.o.s.s.h.n.SecuritySSLNettyHttpServerTransport] [node_name_9200] Exception during establishing a SSL connection: io.netty.handler.ssl.NotSslRecordException: not an SSL/TLS record: 474554202f5f636c75737465722f6865616c74683f776169745f666f725f6e6f6465733d3e3d3126776169745f666f725f7374617475733d79656c6c6f7720485454502f312e310d0a417574686f72697a6174696f6e3a20426173696320595752746157343662586c5464484a76626d645159584e7a643239795a4445794d79453d0d0a557365722d4167656e743a204a6176612f32312e302e310d0a486f73743a206c6f63616c686f73743a393230300d0a4163636570743a202a2f2a0d0a436f6e6e656374696f6e3a206b6565702d616c6976650d0a0d0a

Could this be the issue affecting test setup?

Thanks.

@DarshitChanpura
Copy link
Member Author

DarshitChanpura commented Feb 5, 2024

Hi, @DarshitChanpura. QQ: As per #1064, there an effort to reduce usage of "admin" or other custom strings for providing password inputs and it's recommended to use OPENSEARCH_INITIAL_ADMIN_PASSWORD env/system variable. Should we incorporate that here as well? I'm unsure how this flow works. Please, provide clarification.

This is because the previous PR: #1076 assumed that the password will always be admin for integTest task since a custom configuration is applied for setup. However, that is not the case when integTest is triggered via opensearch-build it uses a remote-cluster. OPENSEARCH_INITIAL_ADMIN_PASSWORD is irrelevant here since the demo configuration is not used to setup security. Instead, a custom configuration is applied.
Hence, this PR defaults integTest task user password to admin for local cluster and conditionally accepts the password if running against remote cluster. I have mentioned the same in PR description as well

Moreover, I see that logs for jenkins/index-management/with-security is filled with following errors:
Could this be the issue affecting test setup?

Its because SSL verification failed. -k option bypasses SSL verification when using cURL. Yes, this has to do with test setup, however it is not a deal-breaker.

Please note, I'm not a maintainer here so I have limited knowledge about workflows. I'm helping out to ensure that CI is unblocked due to changes in default admin credentials that were introduced in security plugin. If you have any questions about workflow setup, the maintainers would be able to help answer them the best.

@r1walz
Copy link
Collaborator

r1walz commented Feb 5, 2024

Thanks for detailing my understanding. LGTM 👍 @DarshitChanpura ty

@r1walz r1walz merged commit 08b3152 into opensearch-project:main Feb 6, 2024
31 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 6, 2024
…1088)

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com>
(cherry picked from commit 08b3152)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
r1walz pushed a commit that referenced this pull request Feb 7, 2024
…1088) (#1090)

(cherry picked from commit 08b3152)

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: bowenlan-amzn <bowenlan23@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x v2.12.0 Issues targeting release v2.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants