Skip to content
This repository was archived by the owner on Sep 17, 2024. It is now read-only.

[CI] tear down the workspace #885

Merged
v1v merged 3 commits into from
Mar 15, 2021
Merged

[CI] tear down the workspace #885

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
15d078e
[CI] tear down the workspace
v1v Mar 15, 2021
53245f1
Update .ci/Jenkinsfile
v1v Mar 15, 2021
4630b71
withGoEnv closure for the try/finally
v1v Mar 15, 2021
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
36 changes: 26 additions & 10 deletions .ci/Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,14 @@ def generateFunctionalTestStep(Map args = [:]){
if (isPR() || isUpstreamTrigger(filter: 'PR-')) {
tags += pullRequestFilter
}
def workerLabels = "${platform} && immutable && docker"

return {
node("${platform} && immutable && docker") {
try {
deleteDir()
unstash 'source'
withGoEnv(version: "${GO_VERSION}"){
node("${workerLabels}") {
deleteDir()
unstash 'source'
withGoEnv(version: "${GO_VERSION}"){
try {
if(isInstalled(tool: 'docker', flag: '--version')) {
dockerLogin(secret: "${DOCKER_ELASTIC_SECRET}", registry: "${DOCKER_REGISTRY}")
}
Expand All @@ -339,12 +340,27 @@ def generateFunctionalTestStep(Map args = [:]){
}
}
}
}
} catch(e) {
error(e.toString())
} finally {
junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/outputs/TEST-*.xml")
} finally {
Copy link
Copy Markdown
Member Author

@v1v v1v Mar 15, 2021

Choose a reason for hiding this comment

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

We only need the finally, then error is redundant

junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/outputs/TEST-*.xml")
archiveArtifacts allowEmptyArchive: true, artifacts: "${BASE_DIR}/outputs/TEST-*.xml"
tearDown(labels: workerLabels)
}
}
}
}
}

/**
* Tear down the setup for the static workers.
*/
def tearDown(Map args = [:]){
Copy link
Copy Markdown
Contributor

@mdelapenya mdelapenya Mar 15, 2021

Choose a reason for hiding this comment

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

Wdyt about having a specific step in the library for cleaning up Go dependencies?

Copy link
Copy Markdown
Member Author

@v1v v1v Mar 15, 2021

Choose a reason for hiding this comment

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

IMO the build system should be the one ensuring the workspace is in good shape. Then the CI should only caring to delete the workspace if required.

Though, it seems go has got some opinionated approach and the build system does not provide those tools, based on that, we could create a specific step in the library to help with.

There are some questions regarding:

  • the new step will always run within the Go environmental context?
  • if running as a post declarative step, will it work?

Do you think we just need to raise an issue regarding this improvement?

Copy link
Copy Markdown
Contributor

@mdelapenya mdelapenya Mar 15, 2021

Choose a reason for hiding this comment

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

I was/am concerned about having the very same method in different pipelines (Beats and here) and probably in many of the other Go projects that we maintain.

I agree that the build system should keep the workspace and dependencies in a good shape, but as you know there is nothing as maven/gradle to do so.

Regarding your questions:

  • I think the Go environment should be the same, just in case the clean method could change between different Go versions
  • I've been reading about go clean. Do you think it could be a best practice to invoke it before tests run? The same we do mvn clean test in the Maven world. If so then we should not think about the post step issue, as the environment would always be right. Even better, we could enforce the clean in the set Go env step 🤔

catchError(buildResult: 'SUCCESS', stageResult: 'SUCCESS') {
dir("${BASE_DIR}"){
sh(label: 'Remove the entire module cache', script: 'go clean -modcache', returnStatus: true)
}
if (isStaticWorker(labels: args.labels)) {
dir("${WORKSPACE}") {
deleteDir()
}
}
}
Expand Down