From 6849081eec290a9138c7437566bc47ee0812b7ab Mon Sep 17 00:00:00 2001 From: Marcus Holl Date: Tue, 23 Oct 2018 17:50:49 +0200 Subject: [PATCH 1/5] fix mkdocs docker image version (#354) --- .travis.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index e0f12eebc..fc29128a2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,13 +6,13 @@ sudo: required services: - docker before_install: -- docker pull squidfunk/mkdocs-material +- docker pull squidfunk/mkdocs-material:3.0.4 script: - mvn clean test --batch-mode - | if [[ "${TRAVIS_PULL_REQUEST}" != "false" ]] then - docker run --rm -it -v ${TRAVIS_BUILD_DIR}:/docs -w /docs/documentation squidfunk/mkdocs-material build --clean --verbose --strict + docker run --rm -it -v ${TRAVIS_BUILD_DIR}:/docs -w /docs/documentation squidfunk/mkdocs-material:3.0.4 build --clean --verbose --strict else # Only in case we are in master branch of the leading SAP repo we would like to deploy, # not from the forks. @@ -22,7 +22,7 @@ script: PRIVATE_KEY="cfg/id_rsa" openssl aes-256-cbc -K $encrypted_12c8071d2874_key -iv $encrypted_12c8071d2874_iv -in cfg/id_rsa.enc -out "${PRIVATE_KEY}" -d chmod a+x gh-pages-deploy.sh - docker run --rm -it --entrypoint "./gh-pages-deploy.sh" -e "TRAVIS_REPO_SLUG=${TRAVIS_REPO_SLUG}" -v ${TRAVIS_BUILD_DIR}:/docs -w /docs squidfunk/mkdocs-material + docker run --rm -it --entrypoint "./gh-pages-deploy.sh" -e "TRAVIS_REPO_SLUG=${TRAVIS_REPO_SLUG}" -v ${TRAVIS_BUILD_DIR}:/docs -w /docs squidfunk/mkdocs-material:3.0.4 else echo "Publishing documentation skipped." fi From 48990bac475e75b27e6782b20de44f06c5cece90 Mon Sep 17 00:00:00 2001 From: Christopher Fenner Date: Wed, 24 Oct 2018 10:13:28 +0200 Subject: [PATCH 2/5] dockerExecute: use docker network to connect sidecar containers (#352) * use docker network to connect containers * add container names * use network-alias instead of name * Update dockerExecute.groovy * Update dockerExecute.groovy * Update dockerExecute.groovy * Update dockerExecute.groovy * Update DockerExecuteTest.groovy * remove obsolete parameter * Update default_pipeline_environment.yml * Update DockerExecuteTest.groovy * Update dockerExecute.groovy * Update DockerExecuteTest.groovy * Update dockerExecute.groovy * update docs --- documentation/docs/steps/dockerExecute.md | 2 +- resources/default_pipeline_environment.yml | 1 - test/groovy/DockerExecuteTest.groovy | 13 +++++++--- vars/dockerExecute.groovy | 30 +++++++++++++++------- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/documentation/docs/steps/dockerExecute.md b/documentation/docs/steps/dockerExecute.md index 599a706ec..5fc13837e 100644 --- a/documentation/docs/steps/dockerExecute.md +++ b/documentation/docs/steps/dockerExecute.md @@ -30,7 +30,7 @@ Proxy environment variables defined on the Jenkins machine are also available in * `containerPortMappings`: Map which defines per docker image the port mappings, like `containerPortMappings: ['selenium/standalone-chrome': [[name: 'selPort', containerPort: 4444, hostPort: 4444]]]` * `dockerEnvVars`: Environment variables to set in the container, e.g. [http_proxy:'proxy:8080'] * `dockerImage`: Name of the docker image that should be used. If empty, Docker is not used and the command is executed directly on the Jenkins system. -* `dockerName`: only relevant for Kubernetes case: Name of the container launching `dockerImage` +* `dockerName`: Kubernetes case: Name of the container launching `dockerImage`, SideCar: Name of the container in local network * `dockerOptions` Docker options to be set when starting the container. It can be a list or a string. * `dockerVolumeBind` Volumes that should be mounted into the container. * `dockerWorkspace`: only relevant for Kubernetes case: specifies a dedicated user home directory for the container which will be passed as value for environment variable `HOME` diff --git a/resources/default_pipeline_environment.yml b/resources/default_pipeline_environment.yml index 1d2502bc8..98df220ae 100644 --- a/resources/default_pipeline_environment.yml +++ b/resources/default_pipeline_environment.yml @@ -232,7 +232,6 @@ steps: 'selenium/standalone-chrome': - containerPort: 4444 hostPort: 4444 - dockerLinkAlias: 'selenium' failOnError: true sidecarImage: 'selenium/standalone-chrome' sidecarName: 'selenium' diff --git a/test/groovy/DockerExecuteTest.groovy b/test/groovy/DockerExecuteTest.groovy index f2da6ecd7..3398d574b 100644 --- a/test/groovy/DockerExecuteTest.groovy +++ b/test/groovy/DockerExecuteTest.groovy @@ -172,6 +172,7 @@ class DockerExecuteTest extends BasePiperTest { void testSidecarDefault(){ jsr.step.dockerExecute( script: nullScript, + dockerName: 'maven', dockerImage: 'maven:3.5-jdk-8-alpine', sidecarEnvVars: ['testEnv':'testVal'], sidecarImage: 'selenium/standalone-chrome', @@ -186,9 +187,14 @@ class DockerExecuteTest extends BasePiperTest { assertThat(docker.imagePullCount, is(2)) assertThat(docker.sidecarParameters, allOf( containsString('--env testEnv=testVal'), - containsString('--volume /dev/shm:/dev/shm') + containsString('--volume /dev/shm:/dev/shm'), + containsString('--network sidecar-'), + containsString('--network-alias testAlias') + )) + assertThat(docker.parameters, allOf( + containsString('--network sidecar-'), + containsString('--network-alias maven') )) - assertThat(docker.parameters, containsString('--link uniqueId:testAlias')) } @Test @@ -217,8 +223,7 @@ class DockerExecuteTest extends BasePiperTest { sidecarEnvVars: ['testEnv':'testVal'], sidecarImage: 'selenium/standalone-chrome', sidecarName: 'selenium', - sidecarVolumeBind: ['/dev/shm':'/dev/shm'], - dockerLinkAlias: 'testAlias', + sidecarVolumeBind: ['/dev/shm':'/dev/shm'] ) { bodyExecuted = true } diff --git a/vars/dockerExecute.groovy b/vars/dockerExecute.groovy index 075469aea..b4843bff0 100644 --- a/vars/dockerExecute.groovy +++ b/vars/dockerExecute.groovy @@ -17,9 +17,9 @@ import groovy.transform.Field 'dockerOptions', 'dockerWorkspace', 'dockerVolumeBind', - 'sidecarName', 'sidecarEnvVars', 'sidecarImage', + 'sidecarName', 'sidecarOptions', 'sidecarWorkspace', 'sidecarVolumeBind' @@ -111,15 +111,27 @@ void call(Map parameters = [:], body) { body() } } else { - def sidecarImage = docker.image(config.sidecarImage) - sidecarImage.pull() - sidecarImage.withRun(getDockerOptions(config.sidecarEnvVars, config.sidecarVolumeBind, config.sidecarOptions)) { c -> - config.dockerOptions = config.dockerOptions?:[] - config.dockerOptions.add("--link ${c.id}:${config.sidecarName}") - image.inside(getDockerOptions(config.dockerEnvVars, config.dockerVolumeBind, config.dockerOptions)) { - echo "[INFO][${STEP_NAME}] Running with sidecar container." - body() + def networkName = "sidecar-${UUID.randomUUID()}" + sh "docker network create ${networkName}" + try{ + def sidecarImage = docker.image(config.sidecarImage) + sidecarImage.pull() + config.sidecarOptions = config.sidecarOptions?:[] + if(config.sidecarName) + config.sidecarOptions.add("--network-alias ${config.sidecarName}") + config.sidecarOptions.add("--network ${networkName}") + sidecarImage.withRun(getDockerOptions(config.sidecarEnvVars, config.sidecarVolumeBind, config.sidecarOptions)) { c -> + config.dockerOptions = config.dockerOptions?:[] + if(config.dockerName) + config.dockerOptions.add("--network-alias ${config.dockerName}") + config.dockerOptions.add("--network ${networkName}") + image.inside(getDockerOptions(config.dockerEnvVars, config.dockerVolumeBind, config.dockerOptions)) { + echo "[INFO][${STEP_NAME}] Running with sidecar container." + body() + } } + }finally{ + sh "docker network remove ${networkName}" } } } else { From 18078b3bdb48c611ed5fee1725ee3e1a614bb877 Mon Sep 17 00:00:00 2001 From: Christopher Fenner Date: Wed, 24 Oct 2018 12:09:51 +0200 Subject: [PATCH 3/5] add sources for code coverage (#349) * add sources for code coverage * hand-in mocked script * remove obsolete test case --- pom.xml | 16 ++++++++++++++-- test/groovy/CheckChangeInDevelopmentTest.groovy | 8 ++++++++ test/groovy/DockerExecuteTest.groovy | 11 ----------- test/groovy/TestsPublishResultsTest.groovy | 12 ++++++------ 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/pom.xml b/pom.xml index c33f91185..352a56770 100644 --- a/pom.xml +++ b/pom.xml @@ -132,7 +132,6 @@ - src org.codehaus.mojo @@ -140,8 +139,21 @@ 1.12 - add-test-source + add-groovy-sources generate-sources + + add-source + + + + src + vars + + + + + add-groovy-test-sources + generate-test-sources add-test-source diff --git a/test/groovy/CheckChangeInDevelopmentTest.groovy b/test/groovy/CheckChangeInDevelopmentTest.groovy index cd3be2150..bf994d7b2 100644 --- a/test/groovy/CheckChangeInDevelopmentTest.groovy +++ b/test/groovy/CheckChangeInDevelopmentTest.groovy @@ -41,6 +41,7 @@ class CheckChangeInDevelopmentTest extends BasePiperTest { ChangeManagement cm = getChangeManagementUtils(true) jsr.step.checkChangeInDevelopment( + script: nullScript, cmUtils: cm, changeManagement: [endpoint: 'https://example.org/cm'], failIfStatusIsNotInDevelopment: true) @@ -63,6 +64,7 @@ class CheckChangeInDevelopmentTest extends BasePiperTest { ChangeManagement cm = getChangeManagementUtils(false) jsr.step.checkChangeInDevelopment( + script: nullScript, cmUtils: cm, changeManagement: [endpoint: 'https://example.org/cm']) } @@ -72,6 +74,7 @@ class CheckChangeInDevelopmentTest extends BasePiperTest { ChangeManagement cm = getChangeManagementUtils(false) boolean inDevelopment = jsr.step.checkChangeInDevelopment( + script: nullScript, cmUtils: cm, changeManagement: [endpoint: 'https://example.org/cm'], failIfStatusIsNotInDevelopment: false) @@ -83,6 +86,7 @@ class CheckChangeInDevelopmentTest extends BasePiperTest { ChangeManagement cm = getChangeManagementUtils(true, '0815') jsr.step.checkChangeInDevelopment( + script: nullScript, changeDocumentId: '42', cmUtils: cm, changeManagement: [endpoint: 'https://example.org/cm']) @@ -95,6 +99,7 @@ class CheckChangeInDevelopmentTest extends BasePiperTest { ChangeManagement cm = getChangeManagementUtils(true, '0815') jsr.step.checkChangeInDevelopment( + script: nullScript, cmUtils: cm, changeManagement : [endpoint: 'https://example.org/cm']) @@ -120,6 +125,7 @@ class CheckChangeInDevelopmentTest extends BasePiperTest { } jsr.step.checkChangeInDevelopment( + script: nullScript, cmUtils: cm, changeManagement: [endpoint: 'https://example.org/cm']) } @@ -134,6 +140,7 @@ class CheckChangeInDevelopmentTest extends BasePiperTest { ChangeManagement cm = getChangeManagementUtils(false, null) jsr.step.checkChangeInDevelopment( + script: nullScript, cmUtils: cm, changeManagement: [endpoint: 'https://example.org/cm']) } @@ -148,6 +155,7 @@ class CheckChangeInDevelopmentTest extends BasePiperTest { ChangeManagement cm = getChangeManagementUtils(false, '') jsr.step.checkChangeInDevelopment( + script: nullScript, cmUtils: cm, changeManagement: [endpoint: 'https://example.org/cm']) } diff --git a/test/groovy/DockerExecuteTest.groovy b/test/groovy/DockerExecuteTest.groovy index 3398d574b..ed47da9e1 100644 --- a/test/groovy/DockerExecuteTest.groovy +++ b/test/groovy/DockerExecuteTest.groovy @@ -115,17 +115,6 @@ class DockerExecuteTest extends BasePiperTest { assertTrue(bodyExecuted) } - @Test - void testExecuteInsideDockerNoScript() throws Exception { - jsr.step.dockerExecute(dockerImage: 'maven:3.5-jdk-8-alpine') { - bodyExecuted = true - } - assertEquals('maven:3.5-jdk-8-alpine', docker.getImageName()) - assertTrue(docker.isImagePulled()) - assertEquals('--env http_proxy --env https_proxy --env no_proxy --env HTTP_PROXY --env HTTPS_PROXY --env NO_PROXY', docker.getParameters().trim()) - assertTrue(bodyExecuted) - } - @Test void testExecuteInsideDockerContainerWithParameters() throws Exception { jsr.step.dockerExecute(script: nullScript, diff --git a/test/groovy/TestsPublishResultsTest.groovy b/test/groovy/TestsPublishResultsTest.groovy index 9ae7f42b0..1955a8fc0 100644 --- a/test/groovy/TestsPublishResultsTest.groovy +++ b/test/groovy/TestsPublishResultsTest.groovy @@ -48,7 +48,7 @@ class TestsPublishResultsTest extends BasePiperTest { @Test void testPublishNothingWithDefaultSettings() throws Exception { - jsr.step.testsPublishResults() + jsr.step.testsPublishResults(script: nullScript) // ensure nothing is published assertTrue('WarningsPublisher options not empty', publisherStepOptions.junit == null) @@ -59,7 +59,7 @@ class TestsPublishResultsTest extends BasePiperTest { @Test void testPublishNothingWithAllDisabled() throws Exception { - jsr.step.testsPublishResults(junit: false, jacoco: false, cobertura: false, jmeter: false) + jsr.step.testsPublishResults(script: nullScript, junit: false, jacoco: false, cobertura: false, jmeter: false) // ensure nothing is published assertTrue('WarningsPublisher options not empty', publisherStepOptions.junit == null) @@ -70,7 +70,7 @@ class TestsPublishResultsTest extends BasePiperTest { @Test void testPublishUnitTestsWithDefaultSettings() throws Exception { - jsr.step.testsPublishResults(junit: true) + jsr.step.testsPublishResults(script: nullScript, junit: true) assertTrue('JUnit options are empty', publisherStepOptions.junit != null) // ensure default patterns are set @@ -84,7 +84,7 @@ class TestsPublishResultsTest extends BasePiperTest { @Test void testPublishCoverageWithDefaultSettings() throws Exception { - jsr.step.testsPublishResults(jacoco: true, cobertura: true) + jsr.step.testsPublishResults(script: nullScript, jacoco: true, cobertura: true) assertTrue('JaCoCo options are empty', publisherStepOptions.jacoco != null) assertTrue('Cobertura options are empty', publisherStepOptions.cobertura != null) @@ -99,7 +99,7 @@ class TestsPublishResultsTest extends BasePiperTest { @Test void testPublishJMeterWithDefaultSettings() throws Exception { - jsr.step.testsPublishResults(jmeter: true) + jsr.step.testsPublishResults(script: nullScript, jmeter: true) assertTrue('JMeter options are empty', publisherStepOptions.jmeter != null) assertEquals('JMeter default pattern not set', @@ -113,7 +113,7 @@ class TestsPublishResultsTest extends BasePiperTest { @Test void testPublishUnitTestsWithCustomSettings() throws Exception { - jsr.step.testsPublishResults(junit: [pattern: 'fancy/file/path', archive: true, active: true]) + jsr.step.testsPublishResults(script: nullScript, junit: [pattern: 'fancy/file/path', archive: true, active: true]) assertTrue('JUnit options are empty', publisherStepOptions.junit != null) // ensure default patterns are set From dabbc724ad577d40bc056f17ca2d4eb67aaa555e Mon Sep 17 00:00:00 2001 From: Christopher Fenner Date: Wed, 24 Oct 2018 13:36:30 +0200 Subject: [PATCH 4/5] handlePipelineStepErrors: extract error message to template (#350) * add template for handleStepErrors * add tests * use template * fix indent * fix typo * Update HandlePipelineStepErrorTest.groovy * Update HandlePipelineStepErrorTest.groovy --- resources/com.sap.piper/templates/error.log | 21 +++++ .../groovy/HandlePipelineStepErrorTest.groovy | 82 +++++++++++++++++++ vars/handlePipelineStepErrors.groovy | 48 ++++------- 3 files changed, 119 insertions(+), 32 deletions(-) create mode 100644 resources/com.sap.piper/templates/error.log create mode 100644 test/groovy/HandlePipelineStepErrorTest.groovy diff --git a/resources/com.sap.piper/templates/error.log b/resources/com.sap.piper/templates/error.log new file mode 100644 index 000000000..3058ed649 --- /dev/null +++ b/resources/com.sap.piper/templates/error.log @@ -0,0 +1,21 @@ +---------------------------------------------------------- +--- ERROR OCCURRED IN LIBRARY STEP: ${stepName} +---------------------------------------------------------- + +FOLLOWING PARAMETERS WERE AVAILABLE TO THIS STEP: +*** +${stepParameters} +*** + +ERROR WAS: +*** +${error} +*** + +FURTHER INFORMATION: +* Documentation of library step ${stepName}: https://sap.github.io/jenkins-library/steps/${stepName}/ +* Source code of library step ${stepName}: https://github.com/SAP/jenkins-library/blob/master/vars/${stepName}.groovy +* Library documentation: https://sap.github.io/jenkins-library/ +* Library repository: https://github.com/SAP/jenkins-library + +---------------------------------------------------------- diff --git a/test/groovy/HandlePipelineStepErrorTest.groovy b/test/groovy/HandlePipelineStepErrorTest.groovy new file mode 100644 index 000000000..830c2855e --- /dev/null +++ b/test/groovy/HandlePipelineStepErrorTest.groovy @@ -0,0 +1,82 @@ +#!groovy +import static org.hamcrest.Matchers.is +import static org.hamcrest.Matchers.not +import static org.hamcrest.Matchers.containsString + +import org.junit.Rule +import org.junit.Test +import org.junit.rules.ExpectedException +import org.junit.rules.RuleChain +import static org.junit.Assert.assertThat + +import util.BasePiperTest +import util.JenkinsLoggingRule +import util.JenkinsStepRule +import util.Rules + +class HandlePipelineStepErrorsTest extends BasePiperTest { + private JenkinsStepRule jsr = new JenkinsStepRule(this) + private JenkinsLoggingRule jlr = new JenkinsLoggingRule(this) + private ExpectedException thrown = ExpectedException.none() + + @Rule + public RuleChain rules = Rules + .getCommonRules(this) + .around(jlr) + .around(jsr) + .around(thrown) + + @Test + void testBeginAndEndMessage() { + def isExecuted + jsr.step.handlePipelineStepErrors([ + stepName: 'testStep', + stepParameters: ['something': 'anything'] + ]) { + isExecuted = true + } + // asserts + assertThat(isExecuted, is(true)) + assertThat(jlr.log, containsString('--- BEGIN LIBRARY STEP: testStep')) + assertThat(jlr.log, containsString('--- END LIBRARY STEP: testStep')) + } + + @Test + void testNonVerbose() { + try { + jsr.step.handlePipelineStepErrors([ + stepName: 'testStep', + stepParameters: ['something': 'anything'], + echoDetails: false + ]) { + throw new Exception('TestError') + } + } catch (ignore) { + } finally { + // asserts + assertThat(jlr.log, not(containsString('--- BEGIN LIBRARY STEP: testStep'))) + assertThat(jlr.log, not(containsString('--- END LIBRARY STEP: testStep'))) + assertThat(jlr.log, not(containsString('--- ERROR OCCURRED IN LIBRARY STEP: testStep'))) + } + } + + @Test + void testErrorsMessage() { + def isReported + try { + jsr.step.handlePipelineStepErrors([ + stepName: 'testStep', + stepParameters: ['something': 'anything'] + ]) { + throw new Exception('TestError') + } + } catch (ignore) { + isReported = true + } finally { + // asserts + assertThat(isReported, is(true)) + assertThat(jlr.log, containsString('--- ERROR OCCURRED IN LIBRARY STEP: testStep')) + assertThat(jlr.log, containsString('[something:anything]')) + } + } +} diff --git a/vars/handlePipelineStepErrors.groovy b/vars/handlePipelineStepErrors.groovy index 42a456f33..00bb0ab5d 100644 --- a/vars/handlePipelineStepErrors.groovy +++ b/vars/handlePipelineStepErrors.groovy @@ -1,50 +1,34 @@ +import groovy.text.SimpleTemplateEngine import groovy.transform.Field @Field STEP_NAME = 'handlePipelineStepErrors' - void call(Map parameters = [:], body) { - def stepParameters = parameters.stepParameters //mandatory def stepName = parameters.stepName //mandatory - def echoDetails = parameters.get('echoDetails', true) - + def verbose = parameters.get('echoDetails', true) + def message = '' try { - if (stepParameters == null && stepName == null) error "step handlePipelineStepErrors requires following mandatory parameters: stepParameters, stepName" - if (echoDetails) - echo "--- BEGIN LIBRARY STEP: ${stepName}.groovy ---" + if (verbose) + echo "--- BEGIN LIBRARY STEP: ${stepName} ---" body() - } catch (Throwable err) { - if (echoDetails) - echo """---------------------------------------------------------- ---- ERROR OCCURRED IN LIBRARY STEP: ${stepName} ----------------------------------------------------------- - -FOLLOWING PARAMETERS WERE AVAILABLE TO THIS STEP: -*** -${stepParameters?.toString()} -*** - -ERROR WAS: -*** -${err} -*** - -FURTHER INFORMATION: -* Documentation of library step ${stepName}: https://sap.github.io/jenkins-library/steps/${stepName}/ -* Source code of library step ${stepName}: https://github.com/SAP/jenkins-library/blob/master/vars/${stepName}.groovy -* Library documentation: https://sap.github.io/jenkins-library/ -* Library repository: https://github.com/SAP/jenkins-library - -----------------------------------------------------------""" + if (verbose) + message += SimpleTemplateEngine.newInstance() + .createTemplate(libraryResource('com.sap.piper/templates/error.log')) + .make([ + stepName: stepName, + stepParameters: stepParameters?.toString(), + error: err + ]).toString() throw err } finally { - if (echoDetails) - echo "--- END LIBRARY STEP: ${stepName}.groovy ---" + if (verbose) + message += "--- END LIBRARY STEP: ${stepName} ---" + echo message } } From 713a5fe31909999003278d8a61d550a4a45560fc Mon Sep 17 00:00:00 2001 From: Oliver Feldmann Date: Wed, 24 Oct 2018 14:45:37 +0200 Subject: [PATCH 5/5] Remove return value from mtaBuild docu There is no return value coming from mtaBuild anymore. --- documentation/docs/steps/mtaBuild.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/docs/steps/mtaBuild.md b/documentation/docs/steps/mtaBuild.md index 64a3447dd..58da03080 100644 --- a/documentation/docs/steps/mtaBuild.md +++ b/documentation/docs/steps/mtaBuild.md @@ -45,7 +45,7 @@ The following parameters can also be specified as step parameters using the glob * `applicationName` ## Return value -The file name of the resulting archive is returned with this step. The file name is extracted from the key `ID` defined in `mta.yaml`. +none ## Side effects 1. The file name of the resulting archive is written to the `commonPipelineEnvironment` with variable name `mtarFileName`.