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.

Sign up for GitHub

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

Refactor distribution build job #5192

Merged
merged 1 commit into from
Nov 21, 2024
+92 −69
Merged

Refactor distribution build job #5192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 45 additions & 34 deletions jenkins/opensearch-dashboards/distribution-build.jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,44 +872,55 @@ pipeline {
}
}
}
}
}
stage('docker build') {
when {
beforeAgent true
allOf {
expression {
params.BUILD_DOCKER != 'do_not_build_docker'
stage('docker build') {
when {
beforeAgent true
allOf {
expression {
params.BUILD_DOCKER != 'do_not_build_docker'
}
expression {
params.BUILD_PLATFORM.contains('linux')
}
expression {
params.BUILD_DISTRIBUTION.contains('tar')
}
}
}
expression {
params.BUILD_PLATFORM.contains('linux')
options {
timeout(time: 90, unit: 'MINUTES')
}
expression {
params.BUILD_DISTRIBUTION.contains('tar')
agent {
docker {
label AGENT_LINUX_X64
image dockerAgent.image
args dockerAgent.args
registryUrl 'https://public.ecr.aws/'
alwaysPull true
}
}
}
}
agent {
docker {
label AGENT_LINUX_X64
image dockerAgent.image
args dockerAgent.args
registryUrl 'https://public.ecr.aws/'
alwaysPull true
}
}
steps {
script {
echo "env.ARTIFACT_URL_LINUX_X64_TAR: ${env.ARTIFACT_URL_LINUX_X64_TAR}"
echo "env.ARTIFACT_URL_LINUX_ARM64_TAR: ${env.ARTIFACT_URL_LINUX_ARM64_TAR}"
steps {
script {
while(true) {
Copy link
Member

Choose a reason for hiding this comment

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

Would recommend to use inbuilt feature of waitUntil. We can take up it up as an enhancement if you like. Would avoid us adding code for checking and waiting for 120seconds

if (env.ARTIFACT_URL_LINUX_X64_TAR != null && env.ARTIFACT_URL_LINUX_ARM64_TAR != null) {
echo "env.ARTIFACT_URL_LINUX_X64_TAR: ${env.ARTIFACT_URL_LINUX_X64_TAR}"
echo "env.ARTIFACT_URL_LINUX_ARM64_TAR: ${env.ARTIFACT_URL_LINUX_ARM64_TAR}"

buildDockerImage(
inputManifest: "manifests/${INPUT_MANIFEST}",
buildNumber: "${BUILD_NUMBER}",
buildOption: "${BUILD_DOCKER}",
artifactUrlX64: env.ARTIFACT_URL_LINUX_X64_TAR,
artifactUrlArm64: env.ARTIFACT_URL_LINUX_ARM64_TAR
)
buildDockerImage(
inputManifest: "manifests/${INPUT_MANIFEST}",
buildNumber: "${BUILD_NUMBER}",
buildOption: "${BUILD_DOCKER}",
artifactUrlX64: env.ARTIFACT_URL_LINUX_X64_TAR,
artifactUrlArm64: env.ARTIFACT_URL_LINUX_ARM64_TAR
)
break
} else {
echo "Waiting for x64 and arm64 tar builds to complete, sleeping 120 seconds..."
sleep(time: 120, unit: 'SECONDS')
Comment on lines +918 to +919
Copy link
Member

Choose a reason for hiding this comment

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

I had tried something like this https://github.com/gaiksaya/opensearch-build/blob/6d4745692422ba7c7c89a55db110a6789ab97e9c/jenkins/opensearch/test-dist.jenkinsfile#L65-L70

Can you check if this is a better option than that? It will automatically start the build as soon those variables are set. If not the timeout will take care of timing out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both are the same, in this we don't need to set additional env variables as we can leverage the ones already available.

Copy link
Member

Choose a reason for hiding this comment

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

Wont that start the build as soon as the variables are set instead of waiting for next 120 seconds? Also wondering, if both or one of the stages failed, we should skip building docker altogether. However,in this case, it will wait for 90 minutes before timing out. Looks like we cannot check stage status (only job status). Can we add some check for that scenario?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Once the variables are set that means the artifacts are available in S3 and can be used immediately, I don't see an issue here. Also as per logic it will only trigger the docker build once both, x64 and arm64 artifacts are available else timeout eventually.
Stage status check is not trivial and there is no straight-forward way to do the same, will need some hack.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We shouldn't be building docker just for x64 or arm64, either build for both else skip.

Copy link
Member

Choose a reason for hiding this comment

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

Right! My point being, the average build time for tarball x64 and arm64 is 20-30min. So if they fail, we would have to wait more 60min (since docker timeout is 90min) for docker stage to timeout eventually.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, may be you can PR it since you have the changes done in your local. I will cancel or you can edit this as well.
I am okay with both.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On a second look, env.TAR_X64_SUCCESS = 'true' would only be set if the groovy lib that executes the actual workflow returns successfully. In case of failure it will exit out of the job without setting this variable.
So it is same as what I'm doing, in case if success/failure both the solutions work same.
Please correct me if I'm wrong.

Copy link
Member

@gaiksaya gaiksaya Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

How about setting env.TAR_ARM64_STATUS = 'somevalue' in post failure stage? Or any value you see fit?

post {
  success {
      script {
          env.TAR_ARM64_STATUS = 'succeeded'
      }
  }
  failure {
    script {
      env.TAR_ARM64_STATUS = 'failed'
    }
  }
}

Check for both values of env.TAR_ARM64_STATUS in the docker build (succeeded and/or failed) and act accordingly. The main motive of this change is to lower the build time and immediately build docker when tarball is ready. Looks like in case of failure we would be waiting 90min for the job to complete.

Copy link
Member

Choose a reason for hiding this comment

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

Synced up offline, looks like this would be too much of hack. We can see how the change in this PR goes and iterate on it later. Thanks!

}
}
}
}
}
}
}
Expand Down
82 changes: 47 additions & 35 deletions jenkins/opensearch/distribution-build.jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ pipeline {
expression {
params.BUILD_DISTRIBUTION.contains('deb')
}

}
agent { label AGENT_LINUX_ARM64 }
stages {
Expand Down Expand Up @@ -834,41 +834,53 @@ pipeline {
}
}
}
}
}
stage('docker build') {
when {
beforeAgent true
allOf {
expression {
params.BUILD_DOCKER != 'do_not_build_docker'
stage('docker build') {
when {
beforeAgent true
allOf {
expression {
params.BUILD_DOCKER != 'do_not_build_docker'
}
expression {
params.BUILD_PLATFORM.contains('linux')
}
}
}
expression {
params.BUILD_PLATFORM.contains('linux')
options {
timeout(time: 90, unit: 'MINUTES')
}
agent {
docker {
label AGENT_LINUX_X64
image dockerAgent.image
args dockerAgent.args
registryUrl 'https://public.ecr.aws/'
alwaysPull true
}
}
steps {
script {
while (true){
if (env.ARTIFACT_URL_LINUX_X64_TAR != null && env.ARTIFACT_URL_LINUX_ARM64_TAR != null) {
echo "env.ARTIFACT_URL_LINUX_X64_TAR: ${env.ARTIFACT_URL_LINUX_X64_TAR}"
echo "env.ARTIFACT_URL_LINUX_ARM64_TAR: ${env.ARTIFACT_URL_LINUX_ARM64_TAR}"
buildDockerImage(
inputManifest: "manifests/${INPUT_MANIFEST}",
buildNumber: "${BUILD_NUMBER}",
buildOption: "${BUILD_DOCKER}",
artifactUrlX64: env.ARTIFACT_URL_LINUX_X64_TAR,
artifactUrlArm64: env.ARTIFACT_URL_LINUX_ARM64_TAR
)
break
} else {
echo "Waiting for x64 and arm64 tar builds to complete, sleeping 120 seconds..."
sleep(time: 120, unit: 'SECONDS')

}
}

}
}
}
}
agent {
docker {
label AGENT_LINUX_X64
image dockerAgent.image
args dockerAgent.args
registryUrl 'https://public.ecr.aws/'
alwaysPull true
}
}
steps {
script {
echo "env.ARTIFACT_URL_LINUX_X64_TAR: ${env.ARTIFACT_URL_X64_TAR}"
echo "env.ARTIFACT_URL_LINUX_ARM64_TAR: ${env.ARTIFACT_URL_ARM64_TAR}"

buildDockerImage(
inputManifest: "manifests/${INPUT_MANIFEST}",
buildNumber: "${BUILD_NUMBER}",
buildOption: "${BUILD_DOCKER}",
artifactUrlX64: env.ARTIFACT_URL_LINUX_X64_TAR,
artifactUrlArm64: env.ARTIFACT_URL_LINUX_ARM64_TAR
)
}
}
}
Expand Down Expand Up @@ -896,7 +908,7 @@ pipeline {
}
postCleanup()
}

}
}
success {
Expand Down
Loading