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

Jenkins Job to run OSDInteg in Parallel #3465

Merged
merged 6 commits into from
May 4, 2023

Conversation

Divyaasm
Copy link
Collaborator

@Divyaasm Divyaasm commented May 3, 2023

Description

The existing OSDIntegTest Jenkins Job is updated to run in parallel.

  • To run the Integtests for selected components, update the test manifest for OSD with the OSD component plugins.
  • Make sure the ftrepo is checked after the commit in the workflow to individually run tests for the list of components
  • PR needs to be merged before getting this PR in.
  • Add mechanism to auotocut GH issue for failed components refer issue
  • Follow up with the Issue to improve the overall performace for Integtests both for OS and OSD.

Issues Resolved

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: Divya Madala <divyaasm@amazon.com>
Signed-off-by: Divya Madala <divyaasm@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2023

Codecov Report

Merging #3465 (df9c68b) into main (1c6da01) will decrease coverage by 0.38%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3465      +/-   ##
==========================================
- Coverage   91.81%   91.44%   -0.38%     
==========================================
  Files         181      181              
  Lines        5268     5305      +37     
==========================================
+ Hits         4837     4851      +14     
- Misses        431      454      +23     

see 4 files with indirect coverage changes

Signed-off-by: Divya Madala <divyaasm@amazon.com>
Signed-off-by: Divya Madala <divyaasm@amazon.com>
Copy link
Member

@peterzhuamazon peterzhuamazon left a comment

Choose a reason for hiding this comment

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

Hi @Divyaasm , Good start to match OSD with OS behavior.
Add some comments.
Thanks.

jenkins/opensearch-dashboards/integ-test.jenkinsfile Outdated Show resolved Hide resolved
jenkins/opensearch-dashboards/integ-test.jenkinsfile Outdated Show resolved Hide resolved
string(
name: 'TEST_MANIFEST',
description: 'Test manifest under the manifests folder, e.g. 2.0.0/opensearch-dashboards-2.0.0-test.yml.',
trim: true
)
string(
name: 'BUILD_MANIFEST_URL',
description: 'The build manifest URL, e.g. https://ci.opensearch.org/ci/dbc/distribution-build-opensearch-dashboards/2.5.0/5367/linux/x64/tar/builds/opensearch-dashboards/manifest.yml',
description: 'The build manifest URL OpenSearch Dashboards, e.g. "https://ci.opensearch.org/ci/dbc/distribution-build-opensearch-dashboards/2.5.0/5367/linux/x64/tar/builds/opensearch-dashboards/manifest.yml".',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description: 'The build manifest URL OpenSearch Dashboards, e.g. "https://ci.opensearch.org/ci/dbc/distribution-build-opensearch-dashboards/2.5.0/5367/linux/x64/tar/builds/opensearch-dashboards/manifest.yml".',
description: 'The build manifest URL for OpenSearch Dashboards, e.g. https://ci.opensearch.org/ci/dbc/distribution-build-opensearch-dashboards/2.5.0/5367/linux/x64/tar/builds/opensearch-dashboards/manifest.yml',

Copy link
Member

Choose a reason for hiding this comment

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

Add the same comment change on OS as well. Thanks.

}
}
}
stage('integ-test') {
// Required running on agent directly here to trigger docker stages in agent node, not trigger docker within docker container
Copy link
Member

Choose a reason for hiding this comment

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

Did not understand the wording here for "not trigger docker within docker container"

Copy link
Member

Choose a reason for hiding this comment

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

That is copied from the OS part, where we have to make sure the run is happened directly on an agent not a docker container, so that the agent can trigger more docker container while it is within an agent, this cannot happen if you start in a docker container.

Copy link
Member

Choose a reason for hiding this comment

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

Add the same comment change on OS as well. Thanks.

echo "componentList: ${componentList}"

for (component_check in componentList) {
if (! componentDefaultList.contains(component_check)) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (! componentDefaultList.contains(component_check)) {
if (!componentDefaultList.contains(component_check)) {

Copy link
Member

Choose a reason for hiding this comment

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

Add the same comment change on OS as well. Thanks.

docker.withRegistry('https://public.ecr.aws/') {
docker.image(docker_images["$distribution"]).inside(docker_args["$distribution"]) {
try {
stage("Run Integtest ${local_component}") {
Copy link
Member

Choose a reason for hiding this comment

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

nit: Everything here is integtest so we can avoid writing the same everywhere

Suggested change
stage("Run Integtest ${local_component}") {
stage("${local_component}") {

Copy link
Member

Choose a reason for hiding this comment

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

Add the same comment change on OS as well. Thanks.

Comment on lines 179 to 184
echo "Component Name: ${local_component}"
// Jenkins tend to not clean up workspace at times even though ws clean is called
// Due to docker is mounting the agent directory so it can communicated with the agent
// This sometimes causes the workspace to retain last run test-results and ends with build failures
// https://github.com/opensearch-project/opensearch-build/blob/6ed1ce3c583233eae4fe1027969d778cfc7660f7/src/test_workflow/test_recorder/test_recorder.py#L99
sh("echo ${local_component} with index ${local_component_index} will sleep ${wait_seconds} seconds to reduce load && sleep ${wait_seconds}")
Copy link
Member

Choose a reason for hiding this comment

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

Back to back 2 echo statements are unnecessary. Remove one

Copy link
Member

Choose a reason for hiding this comment

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

Add the same comment change on OS as well. Thanks.

tests/jenkins/TestOpenSearchDashboardsIntegTest.groovy Outdated Show resolved Hide resolved
jenkins/opensearch-dashboards/integ-test.jenkinsfile Outdated Show resolved Hide resolved
Signed-off-by: Divya Madala <divyaasm@amazon.com>
Copy link
Member

@gaiksaya gaiksaya left a comment

Choose a reason for hiding this comment

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

Added some nits. Please address those and create an issue to onboard GH issue creation for OSD.
Another enhancement in line: #3461
Thanks!

}
}
}
stage('integ-test') {
// Required running on agent directly here to trigger docker stages in agent node, not trigger docker within docker container
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Required running on agent directly here to trigger docker stages in agent node, not trigger docker within docker container
// Need to run this directly on agent node here in order to trigger stages with docker container and avoid docker within docker situation

Copy link
Member

Choose a reason for hiding this comment

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

Add the same comment change on OS as well. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Let's not touch the OS files in this PR. These are just comments nits that can come in new PR. Keep the change to minimum in case we need to revert

jenkins/opensearch-dashboards/integ-test.jenkinsfile Outdated Show resolved Hide resolved
Signed-off-by: Divya Madala <divyaasm@amazon.com>
@peterzhuamazon
Copy link
Member

peterzhuamazon commented May 4, 2023

After talking to @gaiksaya I will merge this PR, and all comment changes on OS part will be added in new PR by @Divyaasm . Thanks.

@peterzhuamazon peterzhuamazon merged commit 936f2dd into opensearch-project:main May 4, 2023
@Divyaasm Divyaasm deleted the osdjenkins branch May 5, 2023 21:59
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.

4 participants