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

Meta: Implement results publisher #207

Closed
4 tasks done
Tracked by #122
camerski opened this issue Aug 12, 2021 · 16 comments
Closed
4 tasks done
Tracked by #122

Meta: Implement results publisher #207

camerski opened this issue Aug 12, 2021 · 16 comments
Assignees
Labels
enhancement New Enhancement triaged This issue has been reviewed by the triage team

Comments

@camerski
Copy link
Contributor

camerski commented Aug 12, 2021

Subtask of #177

Implement a results publisher that writes test logs and results to the same location that the bundle manifest came from (local filesystem or S3).

@saratvemulapalli
Copy link
Member

saratvemulapalli commented Sep 2, 2021

Test Reporting

Test Reporting consists of TestRecorder and TestPublisher.

P0

  • Implement TestRecorder which will record all test outputs.
  • Implement TestPublisher which will publish results to S3.

Here is the low level design proposal for each of the pieces

TestRecorder:

  • It is instantiated once per test suite run.
  • TestRecorder is called after each test on a component is completed.
  • Uses TemporaryDirectory to store the results and cleans it up when it goes out of scope.
class TestRecorder:
    def __init__(self, location, test_type, test_run_id, keep):
         self.location = location (Use temp directory with {keep})
         self.test_type = test_type
         self.test_run_id = test_run_id

    def record_test_logs(
        self, component_name, component_config, exit_status, stdout, stderr, results, is_remote_cluster):
        """
        Record the outcome of test run on a component in {self.location}/{self.test_run_id}/{self.test_type}/{component_name}/{component_config}/test_outcome/
        :param component_name: The name of the component that ran tests.
        :param exit_code: One of SUCCESS, FAILED, ERROR. SUCCESS means the tests ran and all of them passed.
        FAILED means the tests ran and at least one failed. ERROR means the test suite did not run correctly.
        :param stdout: A string containing the stdout stream from the test process.
        :param stderr: A string containing the stderr stream from the test process.
        :param results: A generator that yields tuples containing test results files, in the form (absolute_path, relative_path).
        :param is_remote_cluster: A boolean to tell if the cluster being recorded is a remote cluster
        This method generates test_result_manifest.yml and stores it under {self.location}/{self.test_run_id}/{self.test_type}/{component_name}/{component_config}/test_outcome/
        """
       ...

    def record_test_outcome(self, component_name, component_test_config, exit_code, stdout, stderr, results):
        """
        Record the outcome of a test run.
        :param component_name: The name of the component that ran tests.
        :param exit_code: One of SUCCESS, FAILED, ERROR. SUCCESS means the tests ran and all of them passed.
        FAILED means the tests ran and at least one failed. ERROR means the test suite did not run correctly.
        :param component_test_config: component_config under consideration for test eg: with/without-security.
        :param exit_code: Integer value of the exit code.
        :param stdout: A string containing the stdout stream from the test process.
        :param stderr: A string containing the stderr stream from the test process.
        :param results: A generator that yields tuples containing test results files, in the form (absolute_path, relative_path).
        """

test_result_manifest.yml contains:

test_type:integ_test
component:sql
exit_status: success
test_run_id: 1

TestPublisher:

class TestPublisher:
    def __init__(self, bundle_manifest, test_recorder):
        self.bundle_manifest = bundle_manifest
        self.test_recorder = test_recorder
       
    def to_s3(self, s3_bucket):
    """
    Publishes tests results to S3 pulling information from {self.test_recorder}
    """
    ...
    
    def _get_base_path(self):
    """
    Returns the base path to store logs: /builds/bundles/<bundle-version>/<build-id>/<arch-id>/tests/
    """
    ...

Proposal for test directory structure

This is the directory structure we will use in S3.
/builds/bundles/<bundle-version>/<build-id>/<arch>/tests/<test-run-id>/<test-suite>/<component>/<test-component

Build Directory structure:
/builds/bundles/<bundle-version>/<build-id>/<arch>/<build-artifacts>

@owaiskazi19
Copy link
Member

owaiskazi19 commented Nov 4, 2021

Looks like the directory structure has been changed in S3. Proposing a new tentative structure here for publishing logs in S3:
/bundles/<bundle-version>/<build-id>/<arch>/tests/<test-run-id>/<test-suite>/<component>/<test-component>. Let me know your thoughts on this @saratvemulapalli @peternied

@peternied
Copy link
Member

peternied commented Nov 4, 2021

How about /bundles/<bundle-version>/<build-id>/<arch>/tests -> /{basePath}/test-results/{run-id}/

That base path has the arch, platform, build number and all that good stuff in there. The change that was make for the bundle workflow would be a clean way to update the test system

@dblock
Copy link
Member

dblock commented Nov 4, 2021

Let's not call paths camelCase ;)

@peternied
Copy link
Member

That proposal with examples:

Build Manifest

https://ci.opensearch.org/ci/dbc/bundle-build/1.2.0/292/linux/x64/builds/manifest.yml

Dist Manifest

https://ci.opensearch.org/ci/dbc/bundle-build/1.2.0/292/linux/x64/dist/manifest.yml

Test Results

https://ci.opensearch.org/ci/dbc/bundle-build/1.2.0/292/linux/x64/test-results/1/core/integration-test-report.html

@dblock
Copy link
Member

dblock commented Nov 4, 2021

I would just call it "tests" and "integ-tests" to match other names.

https://ci.opensearch.org/ci/dbc/bundle-build/1.2.0/292/linux/x64/tests/1/integ-test.html

@owaiskazi19
Copy link
Member

owaiskazi19 commented Nov 4, 2021

Just to be sure will we be storing tests under bundle-build?

@peternied
Copy link
Member

https://ci.opensearch.org/ci/dbc/bundle-build/1.2.0/292/linux/x64/ is the base path, you don't need to worry about what it is, the jenkinsfile should feed it into the test system so it can be published

@peternied
Copy link
Member

RE: tests vs test-results
So within the opensearch-build repo, there is already a folder called tests where as builds and dist have no collisions. If we want simpler names, results work without too much redundancy so something like .../x64/results/1/integ-test.html

@owaiskazi19
Copy link
Member

owaiskazi19 commented Nov 4, 2021

The way test recorder and publisher are integrated right now is, test recorder is storing results under tests/test-run-id/integ-test/<component-name>/<security-type>/<remote/local>-cluster-logs.
TestPublisher will pick up those logs and store under basePath/tests/test-run-id/integ-test/<component-name>/<security-type>/<remote/local>-cluster-logsin S3. Let me know if this looks good.

@dblock
Copy link
Member

dblock commented Nov 4, 2021

Note that in #878 I am getting rid of the S3 dependency. We don't have build code that uploads to S3, this is done in Jenkins. Similarly we need to only worry about publishing test results locally and the Jenkins job can copy those results to S3 as needed.

@owaiskazi19
Copy link
Member

owaiskazi19 commented Nov 4, 2021

Thanks for the update. Does that mean #341 will be handled by Jenkins as the test results are already getting published locally by TestRecorder under tests/test-run-id/integ-test/<component-name>/<security-type>/<remote/local>-cluster-logs? If so, is it fair to say that we can close #341 once Jenkins job copy the results to S3.

@dblock
Copy link
Member

dblock commented Nov 4, 2021

@peternied?

@owaiskazi19 We still need something in Jenkinsfile to publish those results, not sure which issue that is

@peternied
Copy link
Member

I think the actual uploading should be done via Jenkins, the system itself doesn't need this knowledge. I think it would look something like what is already in the build workflow

@saratvemulapalli
Copy link
Member

Nice, I like it @owaiskazi19. Also make sure the test manifest file is also included in the path, I believe the tests generate this via TestRecorder.

@peternied is there an issue for updating the Jenkins job to support uploading files to artifact bucket?

@saratvemulapalli
Copy link
Member

Closing this.
Feel free to re-open if anything else is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New Enhancement triaged This issue has been reviewed by the triage team
Projects
None yet
Development

No branches or pull requests

6 participants