PR#1258: Run the integration tests from the current branch - fm-orchestrator

fm-orchestrator

#1258 Run the integration tests from the current branch

Merged 4 years ago by mprahl. Opened 4 years ago by mikeb.

mikeb/fm-orchestrator pr-integration-tests into master

Run the integration tests from the current branch

Mike Bonnet • 4 years ago

095fab0

openshift/integration/koji/pipelines/templates/mbs-build.Jenkinsfile

file modified

		`@@ -42,6 +42,7 @@`
		`timestamps()`
		`timeout(time: 120, unit: 'MINUTES')`
		`buildDiscarder(logRotator(numToKeepStr: '10'))`
		`+ skipDefaultCheckout()`
		`}`
		`environment {`
		`PIPELINE_NAMESPACE = readFile("/run/secrets/kubernetes.io/serviceaccount/namespace").trim()`
		`@@ -267,6 +268,8 @@`
		`openshift.withCluster() {`
		`openshift.withProject(params.MBS_INTEGRATION_TEST_BUILD_CONFIG_NAMESPACE) {`
		`def build = c3i.buildAndWait("bc/${params.MBS_INTEGRATION_TEST_BUILD_CONFIG_NAME}",`
		`+ '-e', "MBS_GIT_REPO=${params.MBS_GIT_REPO}",`
		`+ '-e', "MBS_GIT_REF=${env.PR_NO ? params.MBS_GIT_REF : env.MBS_GIT_COMMIT}",`
		`'-e', "MBS_BACKEND_IMAGE=${env.BACKEND_IMAGE_REF}",`
		`'-e', "MBS_FRONTEND_IMAGE=${env.FRONTEND_IMAGE_REF}",`
		`'-e', "TEST_IMAGES='${env.BACKEND_IMAGE_REF} ${env.FRONTEND_IMAGE_REF}'",`

openshift/integration/koji/pipelines/templates/mbs-integration-test-template.yaml

file modified

		`@@ -108,6 +108,10 @@`
		`type: JenkinsPipeline`
		`jenkinsPipelineStrategy:`
		`env:`
		`+ - name: MBS_GIT_REPO`
		`+ value: "${MBS_GIT_REPO}"`
		`+ - name: MBS_GIT_REF`
		`+ value: "${MBS_GIT_REF}"`
		`- name: MBS_BACKEND_IMAGE`
		`value: "${MBS_BACKEND_IMAGE}"`
		`- name: MBS_FRONTEND_IMAGE`

openshift/integration/koji/pipelines/templates/mbs-integration-test.Jenkinsfile

file modified

+16

		`@@ -41,6 +41,7 @@`
		`timestamps()`
		`timeout(time: 60, unit: 'MINUTES')`
		`buildDiscarder(logRotator(numToKeepStr: '10'))`
		`+ skipDefaultCheckout()`
		`}`
		`environment {`
		`// Jenkins BUILD_TAG could be too long (> 63 characters) for OpenShift to consume`
		`@@ -52,6 +53,21 @@`
		`script {`
		`// Don't set ENVIRONMENT_LABEL in the environment block! Otherwise you will get 2 different UUIDs.`
		`env.ENVIRONMENT_LABEL = "test-${env.TEST_ID}"`
		`+`
		`+ // check out specified branch/commit`
		`+ checkout([$class: 'GitSCM',`
		`+ branches: [[name: params.MBS_GIT_REF]],`
		`+ userRemoteConfigs: [[url: params.MBS_GIT_REPO, refspec: '+refs/heads/:refs/remotes/origin/ +refs/pull//head:refs/remotes/origin/pull//head']],`
		`+ ])`
		`+`
		`+ // get current commit ID`
		`+ // FIXME: Due to a bug discribed in https://issues.jenkins-ci.org/browse/JENKINS-45489,`
		`+ // the return value of checkout() is unreliable.`
		`+ // Not working: env.MBS_GIT_COMMIT = scmVars.GIT_COMMIT`
		`+ env.MBS_GIT_COMMIT = sh(returnStdout: true, script: 'git rev-parse HEAD').trim()`
		`+ echo "Running integration tests for ${params.MBS_GIT_REF}, commit=${env.MBS_GIT_COMMIT}"`
		`+`
		`+ currentBuild.displayName = "${params.MBS_GIT_REF}: ${env.MBS_GIT_COMMIT.take(7)}"`
		`}`
		`}`
		`}`

mikeb commented 4 years ago

Previously, the integration test pipeline was configured to always use the tests present on the
master branch. When running the pre-merge pipeline for a PR, this would result in potentially
out-dated tests being run. This change runs the tests from the current branch, allowing the code
and the tests to evolve together.

This also disables the default checkout of the master branch, which is unnecessary when calling
checkout() from the pipeline.

mikeb commented 4 years ago

@mprahl @rayson What do you think?

mprahl commented 4 years ago

@mikeb I like this approach, but what are the security risks involved with this? Could we potentially use the tests from the master branch if the PR comes from a fork, but if it's from a branch on the same repo, then use the tests from that branch? That way it'd be limited to maintainers of MBS, which can be trusted.

mikeb commented 4 years ago

@mprahl I don't think this is a security risk any more than any other PR is. It doesn't allow the structure of the pipelines to be changed, just the content of the tests. It still relies on review of the PR to make sure it's not doing anything malicious.

FWIW, I don't know how to tell if a PR is coming from a fork or from the main repo.

Edited 4 years ago by mikeb

mprahl commented 4 years ago

:thumbsup:

mprahl commented 4 years ago

Commit 2db62eb fixes this pull-request