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

Change password based on version in integtest health check #4331

Merged
merged 16 commits into from
Jan 9, 2024

Conversation

derek-ho
Copy link
Contributor

@derek-ho derek-ho commented Jan 9, 2024

Description

The current cluster health endpoint check is always using "admin:admin", this fails on > 2.12.0 since they are starting to be spun up with "admin:myStrongPassword123!". This PR makes that change in the cluster health endpoint.

Issues Resolved

List any issues this PR will resolve, e.g. Closes [...].

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.

@derek-ho
Copy link
Contributor Author

derek-ho commented Jan 9, 2024

Signed-off-by: Derek Ho <[email protected]>
Copy link

codecov bot commented Jan 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8897650) 91.27% compared to head (c2b1374) 91.34%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4331      +/-   ##
==========================================
+ Coverage   91.27%   91.34%   +0.07%     
==========================================
  Files         189      189              
  Lines        6163     6169       +6     
==========================================
+ Hits         5625     5635      +10     
+ Misses        538      534       -4     

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

@prudhvigodithi
Copy link
Member

@prudhvigodithi I also found a reference here: https://github.com/opensearch-project/opensearch-build/blob/main/src/test_workflow/perf_test/perf_test_cluster.py#L115, does this need to be changed as well?

Yes this should be updated for 2.12.0 perf tests.
Adding @peterzhuamazon @gaiksaya.

@gaiksaya
Copy link
Member

gaiksaya commented Jan 9, 2024

@prudhvigodithi I also found a reference here: https://github.com/opensearch-project/opensearch-build/blob/main/src/test_workflow/perf_test/perf_test_cluster.py#L115, does this need to be changed as well?

Yes this should be updated for 2.12.0 perf tests. Adding @peterzhuamazon @gaiksaya.

I don;t think perf test are used anymore. But you can update and keep. For benchmark test module, the changes were in via #4306

@derek-ho
Copy link
Contributor Author

derek-ho commented Jan 9, 2024

@prudhvigodithi I also found a reference here: https://github.com/opensearch-project/opensearch-build/blob/main/src/test_workflow/perf_test/perf_test_cluster.py#L115, does this need to be changed as well?

Yes this should be updated for 2.12.0 perf tests. Adding @peterzhuamazon @gaiksaya.

I don;t think perf test are used anymore. But you can update and keep. For benchmark test module, the changes were in via #4306

Thanks folks! I made the requested changes, can you take another look?

@derek-ho
Copy link
Contributor Author

derek-ho commented Jan 9, 2024

Groovy test failure seems to be due to network failure

@prudhvigodithi
Copy link
Member

Hey @derek-ho Codecov is failing can you please check.

        password = 'admin'
        if self.manifest:
            if semver.compare(self.manifest.build.version, '2.12.0') != -1:
                password = 'myStrongPassword123!'
 Check warning on line 115 in src/test_workflow/perf_test/perf_test_cluster.py


Codecov
/ codecov/patch
src/test_workflow/perf_test/perf_test_cluster.py#L112-L115
Added lines #L112 - L115 were not covered by tests

Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
@peterzhuamazon peterzhuamazon merged commit 6eb6f8b into opensearch-project:main Jan 9, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants