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

Implement a basic Jenkins job for running OpenSearch Dashboards integ test #1368

Closed
wants to merge 17 commits into from

Conversation

tianleh
Copy link
Member

@tianleh tianleh commented Dec 17, 2021

Description

It sets up the basic structure as incremental progress including the input parameters and test execution. It may still need more features like uploading testing results which I plan to do as follow ups. Eventually this job will be called post bundle build job.

Issues Resolved

resolves #1322

Test

A successful run http://jenki-jenki-fpgmrv2ryxko-1366042710.us-east-1.elb.amazonaws.com/job/test-osd-tianleh/11/

Check List

  • 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: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
@tianleh
Copy link
Member Author

tianleh commented Dec 17, 2021

Screenshots of the sample run
Screen Shot 2021-12-17 at 3 16 16 AM
Screen Shot 2021-12-17 at 3 16 33 AM

@codecov-commenter
Copy link

codecov-commenter commented Dec 17, 2021

Codecov Report

Merging #1368 (99fee2d) into main (59b954c) will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1368      +/-   ##
============================================
+ Coverage     93.94%   94.02%   +0.07%     
  Complexity       11       11              
============================================
  Files           136      138       +2     
  Lines          2974     3013      +39     
  Branches          8        8              
============================================
+ Hits           2794     2833      +39     
  Misses          172      172              
  Partials          8        8              
Impacted Files Coverage Δ
src/build_workflow/build_args.py 100.00% <0.00%> (ø)
src/build_workflow/build_target.py 100.00% <0.00%> (ø)
src/build_workflow/build_recorder.py 100.00% <0.00%> (ø)
src/build_workflow/builder_from_source.py 100.00% <0.00%> (ø)
src/build_workflow/build_artifact_checks.py 100.00% <0.00%> (ø)
.../jenkins/jobs/PromoteArtifacts_actions_Jenkinsfile 100.00% <0.00%> (ø)
tests/jenkins/jobs/PromoteArtifacts_Jenkinsfile 100.00% <0.00%> (ø)
src/system/properties_file.py 97.43% <0.00%> (+0.06%) ⬆️
...ensearch_dashboards/build_artifact_check_plugin.py 96.55% <0.00%> (+0.12%) ⬆️
..._workflow/opensearch/build_artifact_check_maven.py 95.65% <0.00%> (+0.19%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59b954c...99fee2d. Read the comment docs.

@@ -0,0 +1,83 @@
lib = library(identifier: 'jenkins@20211123', retriever: legacySCM(scm))
Copy link
Member

Choose a reason for hiding this comment

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

What is this identifier, why use fixed date value?

Copy link
Member

Choose a reason for hiding this comment

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

Jenkins caches the library sometimes, so when we make code changes we tend to increment this everywhere.

Comment on lines +69 to +73
node('Jenkins-Agent-al2-x64-c54xlarge-Docker-Host') {
script {
postCleanup()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@dblock dblock Dec 17, 2021

Choose a reason for hiding this comment

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

I wasn't able to extract the whole post part with tests.

Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
Signed-off-by: Tianle Huang <[email protected]>
@tianleh
Copy link
Member Author

tianleh commented Dec 18, 2021

@tianleh
Copy link
Member Author

tianleh commented Dec 18, 2021

Please help take a look again when available @dblock @peternied

@tianleh
Copy link
Member Author

tianleh commented Dec 18, 2021

Created a Github issue to support arm64 #1381

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

Paths are still being computed, take a full path to as input, its simple and lets the caller have control to execute any kind of combination of sources they see fit

When I consider the functionality I think the job parameters at most need to be the following:

  • the path to the test manifest file, relative position in the OpenSearch-Build repository
  • the path to the opensearch distribution manifest file
  • the path to the opensearch-dashboards distribution manifest file

Any other values can be inferred from those files after they've been inspected. If there is more information that is needed then we should see about putting it in the test manifest file as that should describe how to execute the tests.

version: opensearch_dashboards_version
)

sh "./test.sh integ-test manifests/${opensearch_dashboards_version}/opensearch-dashboards-${opensearch_dashboards_version}-test.yml -p opensearch=${basePathOpenSearch} opensearch-dashboards=${basePathOpenSearchDashboards} --test-run-id ${env.BUILD_NUMBER}"
Copy link
Member

Choose a reason for hiding this comment

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

Lets just pass the test manifest path, this is forward compatible with path changes

Suggested change
sh "./test.sh integ-test manifests/${opensearch_dashboards_version}/opensearch-dashboards-${opensearch_dashboards_version}-test.yml -p opensearch=${basePathOpenSearch} opensearch-dashboards=${basePathOpenSearchDashboards} --test-run-id ${env.BUILD_NUMBER}"
sh "./test.sh integ-test ${TEST-MANIFEST} -p opensearch=${basePathOpenSearch} opensearch-dashboards=${basePathOpenSearchDashboards} --test-run-id ${env.BUILD_NUMBER}"

Copy link
Contributor

@abhinavGupta16 abhinavGupta16 left a comment

Choose a reason for hiding this comment

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

Let's add test cases for both the files that added -

  • vars/getPublicUrl.groovy
  • jenkins/opensearch-dashboards/integ-test-opensearch-dashboards.jenkinsfile

name: 'platform',
description: 'Platform of the build of OpenSearch/OpenSearch Dashboards',
trim: true
)
Copy link
Member

@dblock dblock Jan 11, 2022

Choose a reason for hiding this comment

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

I would prefer if the parameters to this job was 1) an OpenSearch Dashboards build URL (e.g. ci.opensearch....) that contains a manifest that would get loaded and all these features such as build ID and architecture and platform were to be extracted, and 2) an OpenSearch URL. Otherwise we keep building assumptions in the URL structure of where things are built, ie. this job "knows" something it shouldn't about the location of builds. Then this code loads the manifest(s) and has everything it needs, it would be shorter and easier to grok.

Use CAPS for variables similarly to other Jenkinsfile.

@tianleh
Copy link
Member Author

tianleh commented Jan 15, 2022

will work on this issue #1482 first so that the release url has the same substructure similar to regular distribution url

@tianleh
Copy link
Member Author

tianleh commented Jan 18, 2022

Tried to add builds/dist in #1490 However, we may need an alternative approach.

@tianleh
Copy link
Member Author

tianleh commented Jan 27, 2022

Need to complete this pre-requested issue first #1492

@tianleh tianleh marked this pull request as draft January 27, 2022 07:39
@tianleh
Copy link
Member Author

tianleh commented Feb 19, 2022

This work is switched to #1653

@tianleh tianleh closed this Feb 19, 2022
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.

Run Dashboards integ test against static endpoint in Jenkins job
6 participants