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

Jump to bottom

[JENKINS-51183] - Introduce Integration testing flow with BOM #20

Closed
wants to merge 14 commits into from
Closed

[JENKINS-51183] - Introduce Integration testing flow with BOM #20

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f5d5968
[JENKINS-51183] - Introduce Integration testing flow with BOM
oleg-nenashev May 7, 2018
c9eee6c
[JENKINS-51183] - Retrigger CI
oleg-nenashev May 11, 2018
76c5e0c
[JENKINS-51183] - Fix the Init Scripts location
oleg-nenashev May 14, 2018
11bdec7
Merge commit 'c9eee6c8f7c3b676f30dee9185d25a77753e915c' into JENKINS-…
oleg-nenashev May 14, 2018
410f446
[JENKINS-51183] - Enable ATH and PCT
oleg-nenashev May 14, 2018
42771f3
Merge branch 'master' into JENKINS-51183
oleg-nenashev May 24, 2018
3dc7ccc
Archive artifacts, disable ATH
oleg-nenashev May 24, 2018
fc7af4b
[JENKINS-51183] - reenable the flag
oleg-nenashev May 24, 2018
a8a9bdc
[JENKINS-51183] - Use relative path in the BOM
oleg-nenashev May 24, 2018
7efa8fd
Merge commit 'cb859635d11ca7c3a2822ee6608588bb3e9b1159' into JENKINS-…
oleg-nenashev May 24, 2018
3159cd4
Minor diagnostics fix for missing provider
oleg-nenashev May 24, 2018
772444c
[JENKINS-51183] - Update all plugins to the recent versions + fix groovy
oleg-nenashev May 24, 2018
7356475
[JENKINS-51183] - Add a Makefile for manual build testing
oleg-nenashev May 24, 2018
23084cf
[JENKINS-51183] - Use the released CWP version
oleg-nenashev May 24, 2018
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
3 changes: 3 additions & 0 deletions Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
if (infra.isRunningOnJenkinsInfra()) {
// ci.jenkins.io
buildPlugin(platforms: ['linux'])

// Integration tests
essentialsTest(baseDir: "src/test/it")
Copy link
Member

Choose a reason for hiding this comment

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

I guess this baseDir could just be the default?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure. In some cases essentials.yml is located in the root (e.g. Core or Git Plugin). CC @raul-arabaolaza . Currently I use root dir as a default

Copy link

Choose a reason for hiding this comment

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

I also use root dir as the default (and is the default location used by pipeline-library) but there is no strong technical reason for that, note that in any case any path can be used as is a param of the step itself

} else if (env.CHANGE_FORK == null) { // TODO pending JENKINS-45970
// to run tests on S3
buildArtifactManagerPluginOnAWS()
Expand Down
56 changes: 56 additions & 0 deletions src/test/it/bom.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
metadata:
# labels and annotations are key: value string attributes
labels:
name: bom-it
# Needed to make PCT happy
groupId: org.jenkins-ci.main
artifactId: jenkins-war
version: 2.118-artifact-manager-s3-SNAPSHOT
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to specify a version, anyway? Could the packager not just pick something arbitrary (999-SNAPSHOT), or generate a version based on the spec.core.version?

Copy link
Member Author

Choose a reason for hiding this comment

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

Current BOM specification seems to require this field. jenkins-infra/pipeline-library#57 actually runs without it, but it will set 1.0-SNAPSHOT as a default now. Makes sense to switch to 999-SNAPSHOT as you propose

spec:
core:
version: 2.118
plugins:
- groupId: "org.jenkins-ci.plugins.workflow"
artifactId: "workflow-api"
Copy link
Member

Choose a reason for hiding this comment

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

So this is duplicating information: the pom.xml already specifies a dependency on workflow-api, and specifies the version. We should not have that twice in the same repo.

I think this is a fundamental flaw in the idea of using a BOM in individual repos for integration tests (as opposed to the overall jenkins-infra/evergreen/configuration/essentials.yaml): there is no clear proposal for how to reconcile duplicated configuration. In jenkinsci/jep#92 (comment) @carlossg claims that bom.yml is the “source of truth” and that pom.xml would be updated by “the tooling” but I do not see how this is going to work. Is pom.xml not going to be versioned any longer? That seems impractical. So, you would need to change version numbers in both places and commit them at once? What is enforcing that they stay aligned?

To me it seems more comfortable for the POM to specify details of artifacts and their versions, and for the BOM to express additional information, if any—perhaps just delete the bom.yml altogether, since the essentials.yml here seems to have all the interesting configuration. For purposes of the custom WAR packager, you can just bundle up the jenkins.war and Jenkins plugins in Maven’s test scope. This trick works fine.

And that would also bypass the “filesystem ref” problem mentioned below.

Copy link
Member Author

@oleg-nenashev oleg-nenashev May 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I could create a "POM to BOM" tool which dumps a YAML with all HPI dependencies in the plugin and adds them to bom.yml (would make sense to rename it to bom-template.yml then).

Copy link
Member

Choose a reason for hiding this comment

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

would make sense to rename it to bom-template.yml then

My point is that there would be nothing in such a template! essentials.yml would have all the information needed for running tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course we can do it if needed, in the most of the cases

version: 2.28-rc333.0675e9e9cb4c
Copy link
Member

Choose a reason for hiding this comment

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

jenkinsci/jep#92 (comment) FTR:

      branch: jglick:VirtualFile-JENKINS-49635

would be useful here in addition to the version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Only for cosmetic reasons, right? When version is set, Custom WAR Packager will ignore other fields anyway

- groupId: "org.jenkins-ci.plugins"
artifactId: "copyartifact"
version: 1.40-rc298.60da56cfabea
Copy link
Member

Choose a reason for hiding this comment

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

1.40

# Maven HPI Plugin improperly resolves dependency trees, needs upper bounds check
- groupId: "org.jenkins-ci.plugins"
artifactId: "structs"
version: 1.14
- groupId: "org.jenkins-ci.plugins.workflow"
artifactId: "workflow-step-api"
version: 2.14
- groupId: "org.jenkins-ci.plugins.workflow"
artifactId: "workflow-job"
version: 2.20
- groupId: "org.jenkins-ci.plugins.workflow"
artifactId: "workflow-basic-steps"
version: 2.7
- groupId: "org.jenkins-ci.plugins.workflow"
artifactId: "workflow-support"
version: 2.18
Copy link
Member

Choose a reason for hiding this comment

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

Currently have an incremental version of this one actually.

- groupId: "org.jenkins-ci.plugins"
artifactId: "junit"
version: 1.24
- groupId: "org.jenkins-ci.plugins"
artifactId: "mailer"
version: 1.21
- groupId: "org.jenkins-ci.plugins"
artifactId: "scm-api"
version: 2.2.7
- groupId: "org.jenkins-ci.plugins"
artifactId: "script-security"
version: 1.44
- groupId: "org.jenkins-ci.plugins"
artifactId: "aws-java-sdk"
version: 1.11.264
environments:
- name: aws
plugins:
#TODO: BOM does not support FileSystem REFs, so somebody needs to add BOM auto-update logic here
Copy link
Member

Choose a reason for hiding this comment

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

Uh, no, updating the BOM would not make sense here, because presumably you wish for the integration tests to be run as part of this plugin’s PR builds. So to verify some change in the plugin against these other tests, you would need to commit the change, then update the version here and commit again, before filing the PR! Does not make sense.

We could allow ${changelist} to be substituted so you would have

          version: 1.0-alpha-1${changelist}

but this would need to be updated after every MRP usage, which is still awkward. Filesystem path references could help, but it needs some exploration what they would look like. One option is

          versionFrom: ../../../.flattened-pom.xml

assuming the local process-resources phase has been run. Alternately,

          versionFrom: ../../..

where the version is inferred from the output of

mvn -Dset.changelist -Dexpression=project.version -Doutput=version help:evaluate
version=`cat version`
rm version
echo $version

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, whatever works. I could easily do it in the custom WAR packager spec, but BOM would require a whatever standardized definition in jenkinsci/jep#92

- groupId: "io.jenkins.plugins"
artifactId: "artifact-manager-s3"
version: 1.0-alpha-1-rc20.60688a35a736
35 changes: 35 additions & 0 deletions src/test/it/essentials.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
packaging:
bom: bom.yml
environment: aws
config:
bundle:
vendor: "Jenkins project"
title: "Jenkins WAR - Artifact Manager S3 Integration Tests"
description: "Demo build, which includes Artifact Manager S3 and plugins using ArtifactManager API for testing"
groovyHooks:
- type: "init"
id: "initScripts"
source:
dir: src/main/groovy
ath:
disabled: true
Copy link
Member

Choose a reason for hiding this comment

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

Why does this attribute need to exist? Why not just comment out the whole ath section?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you can do that if you want

useLocalSnapshots: false
tests:
- "core.ArtifactsTest"
- "plugins.CopyArtifactPluginTest"
- "plugins.CompressArtifactsPluginTest"
- "plugins.WorkflowPluginTest"
- "plugins.WorkflowMultibranchTest"
categories:
- "org.jenkinsci.test.acceptance.junit.SmokeTest"
pct:
disabled: true
useLocalSnapshots: false
jth:
version: 2.38
Copy link
Member

Choose a reason for hiding this comment

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

Delete.

passCustomJenkinsWAR: true
plugins:
- "copyartifact"
- "workflow-job"
- "workflow-basic-steps"
13 changes: 13 additions & 0 deletions src/test/it/initScripts/enableArtifactManager.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import jenkins.model.ArtifactManagerConfiguration
import io.jenkins.plugins.artifact_manager_s3.JCloudsArtifactManager
import io.jenkins.plugins.artifact_manager_s3.JCloudsArtifactManagerFactory

// Predefine storage and prefix when run in test on AWS
if (Boolean.getBoolean("artifact-manager-s3.enabled")) {
println("--- Enabling default artifact storage: ${factory.descriptor.displayName}")

def factory = new JCloudsArtifactManagerFactory();
ArtifactManagerConfiguration.get().artifactManagerFactories.add(factory)
JCloudsArtifactManager.@BLOB_CONTAINER = "artifact-manager-s3"
JCloudsArtifactManager.@PREFIX = "integration-tests/"
}