Add opensearch cluster stdout and stderr logs to test results manifest #4352
Conversation
rishabh6788
requested review from
dblock,
peterzhuamazon,
bbarani,
gaiksaya,
zelinh,
jordarlu,
prudhvigodithi,
Divyaasm and
tianleh
as code owners
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #4352 +/- ##
==========================================
- Coverage 91.35% 91.34% -0.01%
==========================================
Files 190 190
Lines 6175 6193 +18
==========================================
+ Hits 5641 5657 +16
- Misses 534 536 +2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I'm thinking if the component yml file actually contains the local cluster logs if it's generated. (e.g. https://ci.opensearch.org/ci/dbc/integ-test/2.12.0/9207/linux/arm64/tar/test-results/7043/integ-test/alerting/without-security/alerting.yml)
Can we generate the component yml file in this case?
NVM, I think this might not be applicable. Component yml file not generated because the cluster goes wrong and we won't get to the stage to record component yml.
| os_stdout = '' | ||
| os_stderr = '' | ||
| if base_path.startswith("https://"): | ||
| os_stdout = "/".join([base_path.strip("/"), "test-results", test_number, test_type, component_name, config, "local-cluster-logs/id-0/stdout.txt"]) |
There was a problem hiding this comment.
I think this might not be id-0 all the time. CCR team added this id for the cases that there are multiple clusters so we may need to iterate through all of them. #4097
This would also applied to some dashboards integ tests I believe.
There was a problem hiding this comment.
Just verified with past runs and here is the structure, for all the plugins except CCR the cluster id for with-security runs is id-0 and without-security is id-1. I have updated the code for the same.
For CCR, 4 node cluster is spun up, with-security has id-0, id-1, id-2, id-3 and without has 4 - 7. I think it is fine for now wrt CCR as the logs would definitely come handy, even though it comes from one configuration run, in cases where OS process fails to come up.
There was a problem hiding this comment.
But this structure will also be used for dashboards plugins. From what I'm seeing, with-security will have id-0 and id-1; without-security will have id-2 & id-3. We might still want to have some approaches to accommodate for dashboards test.
There was a problem hiding this comment.
Fixed it. @zelinh
| def get_os_cluster_logs(base_path: str, test_number: str, test_type: str, component_name: str, config: str) -> List[str]: | ||
| os_stdout: str = '' | ||
| os_stderr: str = '' | ||
| cluster_id: str = 'id-0' if config == 'with-security' else 'id-1' |
There was a problem hiding this comment.
This hard code id will not work for the dashboards cases since dashboards will include two local cluster logs; it uses 0&1 for with-security and 2&3 for without-security.
There was a problem hiding this comment.
Fixed this. The stdout and stderr fields in the test report yaml will now be a list showing appropriate cluster-ids for OS and OSD.
Signed-off-by: Rishabh Singh <sngri@amazon.com>
Description
This PR adds opensearch cluster stdout and stderr logs to test results manifest. Now the user will be able to see opensearch cluster logs for their component's integ test run. This will be helpful when the test results show as
N/Aand the users wants to know what went wrong.Issues Resolved
#4335
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.