Support multinode cluster logging #4097

monusingh-1 · 2023-10-03T05:16:35Z

Description

Allow logging of multiple nodes if a component uses multi node configuration to run integtest.

Fixes following when there are multiple clusters being created for integ test run:
FileExistsError: [Errno 17] File exists: '/var/jenkins/workspace/integ-test@4/test-results/5327/integ-test/cross-cluster-replication/with-security/local-cluster-logs/opensearch-service-logs'

Issues Resolved

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

#3448

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: monusingh-1 <msnghgw@amazon.com>

monusingh-1 · 2023-10-09T18:58:47Z

Will work on fixing integration tests

Signed-off-by: monusingh-1 <msnghgw@amazon.com>

peterzhuamazon · 2023-10-10T16:42:51Z

Hi @monusingh-1 could you add some quick log or list of dir to showcase this function work in action?

Thanks.

monusingh-1 · 2023-10-10T17:16:23Z

2023-10-10 17:14:14 INFO     ===============================================
2023-10-10 17:14:14 INFO     Running integration tests for cross-cluster-replication with-security
2023-10-10 17:14:14 INFO     ===============================================
2023-10-10 17:14:14 INFO     Executing "ls" in /tmp/tmpo7i2z2u7/cross-cluster-replication
2023-10-10 17:14:14 INFO     Sending SIGKILL to PID 145104
2023-10-10 17:14:14 INFO     Process killed with exit code None
2023-10-10 17:14:14 INFO     Recording local cluster logs for cross-cluster-replication with test configuration as with-security at /home/ec2-user/monu_test/os_build/test-results/b5334a012b9646618ccd4fe9dbd345eb/integ-test/cross-cluster-replication/with-security/local-cluster-logs/id-0
2023-10-10 17:14:14 INFO     Cleanup /tmp/tmpo7i2z2u7/1/local-test-cluster/opensearch-2.10.0 content after the test
2023-10-10 17:14:15 INFO     Sending SIGKILL to PID 145500
2023-10-10 17:14:15 INFO     Process killed with exit code None
2023-10-10 17:14:15 INFO     Recording local cluster logs for cross-cluster-replication with test configuration as with-security at /home/ec2-user/monu_test/os_build/test-results/b5334a012b9646618ccd4fe9dbd345eb/integ-test/cross-cluster-replication/with-security/local-cluster-logs/id-1
2023-10-10 17:14:15 INFO     Cleanup /tmp/tmpo7i2z2u7/2/local-test-cluster/opensearch-2.10.0 content after the test

2023-10-10 17:15:35 INFO     ===============================================
2023-10-10 17:15:35 INFO     Running integration tests for cross-cluster-replication without-security
2023-10-10 17:15:35 INFO     ===============================================
2023-10-10 17:15:35 INFO     Executing "ls" in /tmp/tmpo7i2z2u7/cross-cluster-replication
2023-10-10 17:15:36 INFO     Sending SIGKILL to PID 145904
2023-10-10 17:15:36 INFO     Process killed with exit code None
2023-10-10 17:15:36 INFO     Recording local cluster logs for cross-cluster-replication with test configuration as without-security at /home/ec2-user/monu_test/os_build/test-results/b5334a012b9646618ccd4fe9dbd345eb/integ-test/cross-cluster-replication/without-security/local-cluster-logs/id-2
2023-10-10 17:15:36 INFO     Cleanup /tmp/tmpo7i2z2u7/1/local-test-cluster/opensearch-2.10.0 content after the test
2023-10-10 17:15:36 INFO     Sending SIGKILL to PID 146204
2023-10-10 17:15:36 INFO     Process killed with exit code None
2023-10-10 17:15:36 INFO     Recording local cluster logs for cross-cluster-replication with test configuration as without-security at /home/ec2-user/monu_test/os_build/test-results/b5334a012b9646618ccd4fe9dbd345eb/integ-test/cross-cluster-replication/without-security/local-cluster-logs/id-3
2023-10-10 17:15:36 INFO     Cleanup /tmp/tmpo7i2z2u7/2/local-test-cluster/opensearch-2.10.0 content after the test

peterzhuamazon · 2023-10-10T17:25:13Z

src/test_workflow/test_recorder/test_recorder.py

+        dest_directory = os.path.join(base, "local-cluster-logs/id-" + str(self.number_of_nodes))
+        self.number_of_nodes += 1


The problem with this design as per your log, is that once with-security test complete, the id will not reset and result in id-2/id-3 in without-security dir.

Would suggest reset the number after each run. So it will results in this:

with-security/id-0 id-1 without-security/id-0 id-1

instead of:

with-security/id-0 id-1 without-security/id-2 id-3

Tests are triggered separately for with and without security so we should be good I believe.

The current behavior continues to increase the id throughout different components.

bbarani · 2023-10-10T17:25:22Z

src/test_workflow/test_recorder/test_recorder.py


    def save_test_result_data(self, test_result_data: TestResultData) -> None:
        base = self.parent_class._create_base_folder_structure(test_result_data.component_name, test_result_data.component_test_config)
-        dest_directory = os.path.join(base, "local-cluster-logs")
+        dest_directory = os.path.join(base, "local-cluster-logs/id-" + str(self.number_of_nodes))
+        self.number_of_nodes += 1


This seems to be a generic change. Can you make sure that this doesn't impact other plugins? CC: @peterzhuamazon @gaiksaya

I think it might impact the resports workflows that @zelinh was working on.

Need confirmation on how he retrieve the logs.

Thanks.

I don't think so. The test report just dumps grabs and dumps everything in integ-test folder to manifest. It does not parse each folder for logs I believe. @zelinh can confirm.

Yeah, I was worried this might impact the logs path for component yml file we generated during test record; but i just checked and confirmed that we are walking through the test folder here. The inside file/directory structure should also be recorded properly.
I also checked that we aren't hardcorded anywhere local-cluster-logs so I think this should be fine.

gaiksaya

@rishabh6788 @bbarani Need to know if it is okay to merge this PR now or if we should hold on until 2.11 is done.
Thanks!

bbarani · 2023-10-10T17:54:14Z

@rishabh6788 @bbarani Need to know if it is okay to merge this PR now or if we should hold on until 2.11 is done. Thanks!

I think the tests have been disabled for CCR now. Lets merge it and enable tests for CCR once 2.11.0 is released?

zelinh · 2023-10-10T19:07:34Z

src/test_workflow/test_recorder/test_recorder.py


    def save_test_result_data(self, test_result_data: TestResultData) -> None:
        base = self.parent_class._create_base_folder_structure(test_result_data.component_name, test_result_data.component_test_config)
-        dest_directory = os.path.join(base, "local-cluster-logs")
+        dest_directory = os.path.join(base, "local-cluster-logs/id-" + str(self.number_of_nodes))
+        self.number_of_nodes += 1


Yeah, I was worried this might impact the logs path for component yml file we generated during test record; but i just checked and confirmed that we are walking through the test folder here. The inside file/directory structure should also be recorded properly.
I also checked that we aren't hardcorded anywhere local-cluster-logs so I think this should be fine.

monusingh-1 requested review from dblock, peterzhuamazon, bbarani, gaiksaya, rishabh6788, zelinh, jordarlu, prudhvigodithi, Divyaasm and tianleh as code owners October 3, 2023 05:16

github-actions bot added the valued-contributor label Oct 3, 2023

Support multinode cluster logging

c636f79

Signed-off-by: monusingh-1 <msnghgw@amazon.com>

monusingh-1 force-pushed the test_rec_fix branch from b9e2da6 to c636f79 Compare October 3, 2023 05:19

monusingh-1 marked this pull request as draft October 3, 2023 11:46

Merge branch 'opensearch-project:main' into test_rec_fix

14f7bec

monusingh-1 marked this pull request as ready for review October 9, 2023 18:57

fix test

9fd1110

Signed-off-by: monusingh-1 <msnghgw@amazon.com>

peterzhuamazon reviewed Oct 10, 2023

View reviewed changes

bbarani reviewed Oct 10, 2023

View reviewed changes

gaiksaya approved these changes Oct 10, 2023

View reviewed changes

zelinh approved these changes Oct 10, 2023

View reviewed changes

gaiksaya merged commit dbfc699 into opensearch-project:main Oct 16, 2023
10 checks passed

monusingh-1 deleted the test_rec_fix branch October 17, 2023 05:53

gaiksaya mentioned this pull request Oct 24, 2023

Revert CCR test setting for 2.11 #4164

Merged

zelinh mentioned this pull request Jan 18, 2024

Add opensearch cluster stdout and stderr logs to test results manifest #4352

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support multinode cluster logging #4097

Support multinode cluster logging #4097

monusingh-1 commented Oct 3, 2023 •

edited

Loading

monusingh-1 commented Oct 9, 2023

peterzhuamazon commented Oct 10, 2023

monusingh-1 commented Oct 10, 2023

peterzhuamazon Oct 10, 2023

gaiksaya Oct 10, 2023

monusingh-1 Oct 10, 2023

bbarani Oct 10, 2023

peterzhuamazon Oct 10, 2023

gaiksaya Oct 10, 2023

zelinh Oct 10, 2023

gaiksaya left a comment

bbarani commented Oct 10, 2023 •

edited

Loading

zelinh Oct 10, 2023

		dest_directory = os.path.join(base, "local-cluster-logs/id-" + str(self.number_of_nodes))
		self.number_of_nodes += 1

Support multinode cluster logging #4097

Support multinode cluster logging #4097

Conversation

monusingh-1 commented Oct 3, 2023 • edited Loading

Description

Issues Resolved

monusingh-1 commented Oct 9, 2023

peterzhuamazon commented Oct 10, 2023

monusingh-1 commented Oct 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gaiksaya left a comment

Choose a reason for hiding this comment

bbarani commented Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

monusingh-1 commented Oct 3, 2023 •

edited

Loading

bbarani commented Oct 10, 2023 •

edited

Loading