Unify Jenkins pipelines, inline scripts and add full-build launch-config #2620

HannesWell · 2023-04-20T18:31:24Z

This PR is a suggestion to merge the deployment Jenkins pipelines of Xtext as already mentioned in #2224 and to inline the scripts called in them.

The initial reaction was a bit reluctant, nevertheless I think the suggested changes would have the following advantages:

Inlining the scripts make the pipelines simpler to read since there is less indirection
Merging the deployment pipelines leads to less (mostly duplicated) pipeline code to maintain and the few groovy-'tricks' that are necessary are IMHO relatively simple to understand (there are more complicated things at least in the main pipeline.
The Jenkins probably two jobs for different deployments can be merged into one, that runs a nightly deployment each night, controlled by the cron-trigger and if started manually one can select to run a M, RC or GA release.
To test modifications you can replay the pipeline in the Jenkins UI with your modifications applied without the need to push any branch.
The scripts that are currently called in the workflows are relatively complicated and hard to understand
At least the sign and deploy scripts probably cannot run locally since the required environment (variables, credentials, etc.) is probably not available on a local computer
The (bash) scripts can only run on Unix (maybe also on Mac) and on Windows only if one installs something Cygwin

To ease local execution of Xtext's maven build I added a M2E Maven launch config for the full-build.
If you want to have configs for other scenarios (maven only, p2 only) I can of course add them.

Of course the (probably) two Jenkins jobs that use the merged Pipelines have to be merged too.

The two commits should not be squashed. Otherwise git will not record the renaming jenkins/milestone-deploy/Jenkinsfile → jenkins/deploy/Jenkinsfile .

HannesWell · 2023-04-20T18:35:24Z

Jenkinsfile

+              -PuseJenkinsSnapshots \
+              -Dtarget-platform-classifier=xtext-${selectedTargetPlatform()} \
+              -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn \
+              -DaltDeploymentRepository=local::file:./build/maven-repository


In the script the value was -DaltDeploymentRepository=local::default::file:./build/maven-repository, but Maven emitted a deprecation warning, therefore I changed that to the recommended value.

HannesWell · 2023-04-20T18:36:20Z

Jenkinsfile

-              ${javaVersion() == 17 ? '' : '--toolchains releng/toolchains.xml -Pstrict-release-jdk'}
+            mvn -f org.eclipse.xtext.full.releng --batch-mode clean deploy \
+              ${javaVersion() == 17 ? '' : '--toolchains releng/toolchains.xml -Pstrict-release-jdk'} \
+              -PuseJenkinsSnapshots \


The full-build script also defined -Dit-archetype-tests-skip=true, but that is actually not necessary because that variable is initialized with true.

The same applies for the full-deploy script inlined below.

HannesWell · 2023-04-20T18:38:23Z

jenkins/deploy/Jenkinsfile

+  triggers { // https://jenkins.io/doc/book/pipeline/syntax/#triggers
+    cron('50 21 * * *') // nightly at 21:50
+  }
+


I tested this in my local Jenkins instance and it worked as desired. The job run automatically at the specified time with DEPLOYMENT_TYPE=nightly, and if one trigger an execution manually one has to choose one of the available parameter values.

Rather than doing nightly builds at a fixed schedule, should we do continuous deployment whenever something's merged into main, @cdietrich ?

Not necessarily in the scope of this ticket, though @HannesWell

LorenzoBettini · 2023-04-24T09:29:03Z

@HannesWell sorry for the delay.

I think this PR can be used to clean up a few things you have already detected.

BUT:

I have a strong opinion against merging the Jenkinsfiles. I definitely prefer to maintain several small Jenkinsfiles, each one with a single responsibility (from the Single Responsibility Principle) than having a big one with ifs, whens, or groovy-tricks. This is a context where testing is hard and takes time, and my solution keeps things simple.
the prepare-for-milestone.sh again has a single responsibility and will do bad things if you pass it nightly

The two Jenkinsfiles and the script prepare-for-milestone.sh (which is a shell script, but just calls maven commands) have been carefully baked and tested. I'm not willing to update them. For final releases, I have another script (not already committed), separate from the one for milestones and another Jenkinsfile just for final releases. The original Jenkinsfile before repo merge. was a huge 500 lines of unreadable and untestable Jenkinsfile with ifs, switches, embedded shell scripts... I'm not willing to go back to that road. :)

I'm also against inlining the script files as I had already said.
Maybe they can be cleaned up.
I agree they cannot be run from Windows, but I think it's better to document/suggest the Maven commands in an MD file, than to even create a launch file. (For example, I don't think this -f org.eclipse.xtext.full.releng --fae clean verify is a good command to run locally).

Of course, we have to head from @cdietrich and @szarnekow , but just to stress that again (with smile, of course), I'll be strongly against merging the Jenkinsfiles.

cdietrich · 2023-04-24T09:45:24Z

i prefer separate Jenkinsfiles.
having scripts files (at least for me) makes it more eady to run locally without c&p big lines from jenkinsfiles

HannesWell · 2023-04-26T17:20:09Z

BUT:

* I have a strong opinion against merging the Jenkinsfiles. I definitely prefer to maintain several small Jenkinsfiles, each one with a single responsibility (from the Single Responsibility Principle) than having a big one with ifs, whens, or groovy-tricks. This is a context where testing is hard and takes time, and my solution keeps things simple.

Personally I would still say that even the merged pipeline has a single responsibility: It deploys the xtext artifacts. I would consider the type of deployment a parameter, as it already is for the milestone-deploy workflow.
Making a distinction between nightly and M, RC and GA deployment seems kind of artifical to me, but if you want to do that, why not create a own pipeline for each M1, M2, RC1, RC2 and GA (exaggeratedly asked :) )?
The merged pipeline has 7 lines more, which I would not consider much bigger than before and at the same time one entire file can be removed (with 54).
Furthermore there are only two very simple groovy 'tricks' in it (that every java developer should understand), everything else is standard Jenkins declarative pipeline syntax. You even just added the same 'trick' to the same pipeline in #2629.
I honestly don't understand why it is bad in the deployment pipeline but ok in the 'main' Jenkinsfile which does far more complex things?

I it is right that the merged pipeline has to be tested, but I don't expect it to be too complicated since the commands within the pipeline are not changed but only the triggers are changed.
Therefore it should be sufficient to create the new job that uses the merged pipeline (or reconfigure one of xtext-monorepo-full-deploy-nightly xtext-monorepo-full-deploy-milestone,
verify that the corresponding parameters are available when selecting 'Build with parameters', try a nightly-build and a milestone build and verify that the right profiles are actived and the env-variables have the right value.
Last but not least you can test that the cron trigger works (and temporarily set it to the time in 5min, get a coffee and check the result).

I can offer to test and demonstrate you that the changed parts work in the M2E Jenkins instance. The entire pipeline does not have to be tested since it did not change.

* the `prepare-for-milestone.sh` again has a single responsibility and will do bad things if you pass it `nightly`

Yes, that's why this stage is guarded with a corresponding when condition.

The two Jenkinsfiles and the script prepare-for-milestone.sh (which is a shell script, but just calls maven commands) have been carefully baked and tested. I'm not willing to update them. For final releases, I have another script (not already committed), separate from the one for milestones and another Jenkinsfile just for final releases. The original Jenkinsfile before repo merge. was a huge 500 lines of unreadable and untestable Jenkinsfile with ifs, switches, embedded shell scripts... I'm not willing to go back to that road. :)

I understand that the pipeline before the repo merge was too complex and that you don't want to go back there. But personally I would say that the suggested change is far from that.
I agree that the in two points the pipeline became locally slightly less simple, but even that should be understandable for a Java developer.
Having two different pipelines can make them simpler locally but tends to make the overall system more complex and harder to understand (for others or maybe even you in a more distant future) because there is much duplication and it takes more time to understand the difference and non-differences between a nightly deployment and all the other deployments.
Of course this always has to be balanced, but in my humble opinion the suggested changes are overall for the better.

Having two different pipelines increases the maintains costs on the long run because every change you do to one pipeline you probably also have to apply to the other one. And if you really want to test all changes before (which is good) you have to test two pipelines instead of one.
And as a bonus you also have one Jenkins job less with this change, which also makes the build infra-structure less complex.

I'm also against inlining the script files as I had already said. Maybe they can be cleaned up. I agree they cannot be run from Windows, but I think it's better to document/suggest the Maven commands in an MD file, than to even create a launch file. (For example, I don't think this -f org.eclipse.xtext.full.releng --fae clean verify is a good command to run locally).

The downside of just documenting the commands is that it can easily become outdated. The good thing about a launch-config is that it is executable. If regularly used one will notice discrepancies quite quickly and one will remember to change it more likely.

I think this PR can be used to clean up a few things you have already detected.

I hope I made my points clear, if you nevertheless prefer to keep most of it as it is, I'm fine with stop discussing if you want to save your time.
Can you then please just list the changes you are fine with to change?

LorenzoBettini · 2023-05-03T06:49:23Z

@HannesWell, sorry for the delay.

Releasing nightly, a milestone, or an official release are different tasks; for this reason, I really prefer to have separate Jenkinsfiles, which are much easier to test. Having conditionals makes everything harder to maintain and test.

As I said, also the launch configuration does not look good to me, and for the moment I'd do without it.

I'd keep

the updated URL for deploying locally
the removal of redundant archetype it skip
the removal of script files that we are sure we don't use anymore

HannesWell · 2023-05-04T22:24:32Z

As I said, also the launch configuration does not look good to me, and for the moment I'd do without it.

Can you please elaborate what exactly does not look good in the launch config? At the moment it reflects the full-build script.

I'd keep

* the updated URL for deploying locally

* the removal of redundant archetype it skip

* the removal of script files that we are sure we don't use anymore

OK, I'll update this PR accordingly (but probably not before next week).

Which scripts can then be removed exactly?
Since at least the full-deploy.sh will be left, would you be fine with moving it into the scripts folder, in order to make the root folder a bit less crowed?

szarnekow

Submitting a bunch of older comments

szarnekow · 2023-04-21T09:34:04Z

jenkins/deploy/Jenkinsfile

      steps {
-        sh './scripts/prepare-for-milestone.sh ${RELEASE_TYPE}'
+        sh './scripts/prepare-for-milestone.sh ${DEPLOYMENT_TYPE}'


To be honest, I find the terminology milestone here a bit confusing since it's for every release being it milestone, rc, or ga. Not actionable though.

@szarnekow see also #2657

To be honest, I find the terminology milestone here a bit confusing since it's for every release being it milestone, rc, or ga. Not actionable though.

In fact, I want to keep releasing of milestones and actual releases separate.

szarnekow · 2023-04-21T09:34:41Z

jenkins/deploy/Jenkinsfile

+  triggers { // https://jenkins.io/doc/book/pipeline/syntax/#triggers
+    cron('50 21 * * *') // nightly at 21:50
+  }
+


Rather than doing nightly builds at a fixed schedule, should we do continuous deployment whenever something's merged into main, @cdietrich ?

Not necessarily in the scope of this ticket, though @HannesWell

szarnekow · 2023-04-21T09:36:19Z

jenkins/other-experiments/Jenkinsfile-p2-deploy-nightly

@@ -14,7 +14,12 @@ pipeline {
  stages {
    stage('Maven Tycho Build and Sign') {
      steps {
-        sh './tycho-sign.sh'
+        sh '''


@LorenzoBettini what is the idea behind the other-experiements folder? Is it supposed to stay or become obsolete prospectively?

They are to be considered obsolete! I forgot to remove them

szarnekow · 2023-05-10T10:30:24Z

@HannesWell We (as in: committers on this project) came to the conclusion that we prefer the separate Jenkinsfiles for the builds rather than the single merged Jenkinsfile. We know that others might have chosen differently, but we are more confident with the current split. The same holds true for the existing shell scripts. Rather than inlining them into the Jenkinsfile, we prefer them to be available as shell scripts that can be called from the command line. Here, we accept the limitations that you mentioned: These might not work on Windows / for all users, and some environment variables might be missing in the average case. But generally, this is the setup we'd like to move forward with.

For sure, we are grateful for your input, and proposed changes might be considered in some other future that we don't know yet, but as of now, we'd like to keep the individual Jenkinsfiles and their accompanying scripts.

HannesWell · 2023-05-10T21:47:35Z

@HannesWell We (as in: committers on this project) came to the conclusion that we prefer the separate Jenkinsfiles for the builds rather than the single merged Jenkinsfile. We know that others might have chosen differently, but we are more confident with the current split. The same holds true for the existing shell scripts. Rather than inlining them into the Jenkinsfile, we prefer them to be available as shell scripts that can be called from the command line.

Yes, that's fine. It would not have been my choice (surprise), but I accept your decision.
One question before I adapt this PR: Would you be fine with moving the scripts into the scripts folder to have the root less crowded?

Rather than doing nightly builds at a fixed schedule, should we do continuous deployment whenever something's merged into main, @cdietrich ?

Not necessarily in the scope of this ticket, though @HannesWell

For me this is a good proposal. We do that in m2e and it works quite well and is actually not too complicated:
https://github.com/eclipse-m2e/m2e-core/blob/7b340f774365df15715a92f87ca81bad2bc8a0e0/Jenkinsfile#L68-L97

But at least in m2e we have the deployment script in the Jenkinsfile in contrast to Xtext where the deployment is specified pom.xml of the repos (if I have not misunderstood it). So depending on how that should be done it might require some rework.

Performing the deployment on each push would probably make the question about merging the nightly- and release-deploy pipeline obsolete, since it probably has to be done as part of the 'main' pipeline then.

cdietrich · 2023-05-11T06:27:08Z

Please note the deployment of milestone and release to maven central needs manual closing of staged repo

HannesWell · 2023-05-11T22:40:55Z

I now reduced the PR to the changes you requested.

Please note the deployment of milestone and release to maven central needs manual closing of staged repo

If you use the nexus-maven-plugins to deploy to OSSRH you can set the autoReleaseAfterClose property to true to let the plugin close the repo for you.
But I assume that does not apply for nightly builds?

cdietrich · 2023-05-12T03:11:05Z

yes. nightly builds are deployed automatically anyway

cdietrich · 2023-05-12T05:03:42Z

https://ci.eclipse.org/xtext/job/xtext/job/PR-2620/8/console

HannesWell · 2023-05-16T22:32:08Z

https://ci.eclipse.org/xtext/job/xtext/job/PR-2620/8/console

According to the logs for the failing project the maven-deploy-plugin version 2.7 is used. The reason is that for that bom project the deploy-plugin has its version not explicitly configured and for maven 3.8.6 2.7 is the default binding's version.
But the new parameter is only supported since maven-deploy-plugin version 2.8

In the latest Maven 3.9.2 release the default version of the maven-deploy-plugin is set to 3.1.0, which would be used in this case and work with the new format. Therefore I suggest to update to the latest Maven version.

When searching the Xtext repo for usages of the deploy-plugin I stumbled upon (the org.eclipse.xtext.maven.parent has a similar construct)
https://github.com/eclipse/xtext/blob/18216f46e9113841336beb3df238bd34c31d3a44/org.eclipse.xtend.maven.parent/pom.xml#L58-L73

I wonder what the purpose of that part is? Is it to test that the deploy-plugin is really pinned to version 2.8.2 as defined a few lines below? Is the reason why the latest release cannot be used still valid? The added link is unfortunately dead.
Is it because of deployatend is used, which according to its doc was added in 2.8?
Furthermore it looks like the enforcer-plugin configuration is wrong, it has no rules element:
https://maven.apache.org/enforcer/enforcer-rules/bannedPlugins.html

If that's correct, I suggest to just remove the enforcer-plugin call here and would inherit the m-deploy/install-plugin version from the xtext-parent and do the same for org.eclipse.xtext.maven.parent).

The root pom.xml defines the version of maven-deploy-plugin as 3.0.0 and for the maven-install-plugin as 3.1.0.

LorenzoBettini · 2023-05-17T06:50:40Z

Unfortunately, the BOM has no relation with the parent POM, so, for now, we should replicate a few plugin versions/configurations in the BOM, or things like the ones you described happen. This is required until a relationship is established between the BOM and the parent, but I still have to work on that.

Updating to Maven 3.9.2 is to be postponed due to the problems we'd have in GitHub Actions for macOS.

In general, anyway, it's best to pin versions of standard plugins (IIRC, Maven 4 will enforce that or the build will fail), and not rely on the ones provided by the current version of Maven.

Maybe the enforcer part could be removed now. I don't know why it's there, but it looks like it's due to old issues.

Currently, "deploy at end" is not used.

I'll try to address these issues in another PR.

LorenzoBettini · 2023-05-17T06:52:24Z

I now reduced the PR to the changes you requested.

Please note the deployment of milestone and release to maven central needs manual closing of staged repo

If you use the nexus-maven-plugins to deploy to OSSRH you can set the autoReleaseAfterClose property to true to let the plugin close the repo for you. But I assume that does not apply for nightly builds?

To be able to test releases and milestones, we removed the use of nexus staging plugin.

When you deploy snapshots there's no additional staging going on: snapshots will be deployed immediately.

LorenzoBettini · 2023-05-17T06:58:21Z

Rather than doing nightly builds at a fixed schedule, should we do continuous deployment whenever something's merged into main, @cdietrich ?
Not necessarily in the scope of this ticket, though @HannesWell

For me this is a good proposal. We do that in m2e and it works quite well and is actually not too complicated: https://github.com/eclipse-m2e/m2e-core/blob/7b340f774365df15715a92f87ca81bad2bc8a0e0/Jenkinsfile#L68-L97

But at least in m2e we have the deployment script in the Jenkinsfile in contrast to Xtext where the deployment is specified pom.xml of the repos (if I have not misunderstood it). So depending on how that should be done it might require some rework.

Performing the deployment on each push would probably make the question about merging the nightly- and release-deploy pipeline obsolete, since it probably has to be done as part of the 'main' pipeline then.

I really want to keep the deployment logic in POM only and avoid additional scripts for the deployment.

If we want to deploy (p2 and Maven snapshots) when something is pushed to main we can add a stage with the when condition as in https://github.com/eclipse-m2e/m2e-core/blob/7b340f774365df15715a92f87ca81bad2bc8a0e0/Jenkinsfile#L68-L97 but we'll have to run a build again (skipping tests and signing). In fact, we cannot sign when running tests as we saw in the past. It's not a big deal anyway to run the build again; it will be much faster since we'll skip tests.

LorenzoBettini · 2023-05-17T07:17:48Z

Or even better: in the additional stage that should run only on main we simply trigger the current nightly deploy job

LorenzoBettini · 2023-05-17T08:35:02Z

@HannesWell please cherry pick this commit 0698ae1 on this PR; it should fix the problem.

LorenzoBettini · 2023-05-17T15:31:01Z

@HannesWell that commit is now on master. I'm retriggering the job for your PR, which now should work.

HannesWell · 2023-05-17T17:14:11Z

@HannesWell that commit is now on master. I'm retriggering the job for your PR, which now should work.

Thanks @LorenzoBettini and sorry for the late action, I didn't have access to the right computer over the day.

Just rebased the branch on top of main, since your re-trigger seems to not have been merged with the main in Jenkins.

HannesWell · 2023-05-17T17:16:57Z

In general, anyway, it's best to pin versions of standard plugins (IIRC, Maven 4 will enforce that or the build will fail), and not rely on the ones provided by the current version of Maven.

Yes that's right, pinning is even better. Btw. there are new versions available.
You don't have dependabot enabled for Xtext, have you?

HannesWell · 2023-05-17T17:43:54Z

What do you think about moving the gpg keyring setup into a script and reference it from the nightly-deploy and milestone-deploy Jenkinsfile? This would avoid a duplicated definition.
https://github.com/eclipse/xtext/blob/d84f253401bf0470fbd389fb895b29894d74b912/jenkins/nightly-deploy/Jenkinsfile#L39-L40
https://github.com/eclipse/xtext/blob/d84f253401bf0470fbd389fb895b29894d74b912/jenkins/milestone-deploy/Jenkinsfile#L51-L52

HannesWell · 2024-08-24T12:31:11Z

What do you think about moving the gpg keyring setup into a script and reference it from the nightly-deploy and milestone-deploy Jenkinsfile? This would avoid a duplicated definition.

@LorenzoBettini is that or this PR in general of interest or should it be abandoned?

github-actions · 2024-08-24T13:38:32Z

Test Results

6 460 files ±0 6 460 suites ±0 3h 2m 45s ⏱️ - 9m 30s
43 240 tests ±0 42 657 ✅ ±0 583 💤 ±0 0 ❌ ±0
170 102 runs +4 167 736 ✅ ±0 2 354 💤 ±0 12 ❌ +4

Results for commit 8381ac8. ± Comparison against base commit 6c31185.

LorenzoBettini · 2024-08-24T18:49:54Z

What do you think about moving the gpg keyring setup into a script and reference it from the nightly-deploy and milestone-deploy Jenkinsfile? This would avoid a duplicated definition.

@LorenzoBettini is that or this PR in general of interest or should it be abandoned?

@HannesWell maybe moving the gpg stuff into a separate script could be useful.

cdietrich added this to the Release_2.31 milestone Apr 20, 2023

HannesWell commented Apr 20, 2023

View reviewed changes

HannesWell force-pushed the unifyPipelines branch from 60d6c23 to c52e28f Compare April 26, 2023 17:18

szarnekow reviewed May 10, 2023

View reviewed changes

HannesWell force-pushed the unifyPipelines branch from c52e28f to 5453bc2 Compare May 11, 2023 22:36

HannesWell force-pushed the unifyPipelines branch from 5453bc2 to cdef55a Compare May 11, 2023 22:44

This was referenced May 17, 2023

Continuous deployment of the main branch #2669

Closed

harmonized maven-deploy-plugin #2670

Merged

HannesWell force-pushed the unifyPipelines branch from cdef55a to 87014c5 Compare May 17, 2023 17:11

cdietrich modified the milestones: Release_2.31, Release_2.32 May 29, 2023

cdietrich modified the milestones: Release_2.32, Release_2.33 Aug 27, 2023

cdietrich modified the milestones: Release_2.33, Release_2.34 Nov 17, 2023

cdietrich modified the milestones: Release_2.34, Release_2.35 Feb 27, 2024

cdietrich modified the milestones: Release_2.35, Release_2.36 May 23, 2024

Remove unnecessary build arguments and fix deprecated ones

8381ac8

HannesWell force-pushed the unifyPipelines branch from 87014c5 to 8381ac8 Compare August 24, 2024 12:30

cdietrich modified the milestones: Release_2.36, Release_2.37 Aug 25, 2024

cdietrich modified the milestones: Release_2.37, Release_2.38 Nov 16, 2024

Unify Jenkins pipelines, inline scripts and add full-build launch-config #2620

Are you sure you want to change the base?

Unify Jenkins pipelines, inline scripts and add full-build launch-config #2620

Conversation

HannesWell commented Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LorenzoBettini commented Apr 24, 2023

cdietrich commented Apr 24, 2023

HannesWell commented Apr 26, 2023 • edited Loading

LorenzoBettini commented May 3, 2023

HannesWell commented May 4, 2023 • edited Loading

szarnekow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szarnekow commented May 10, 2023

HannesWell commented May 10, 2023

cdietrich commented May 11, 2023

HannesWell commented May 11, 2023 • edited Loading

cdietrich commented May 12, 2023

cdietrich commented May 12, 2023

HannesWell commented May 16, 2023

LorenzoBettini commented May 17, 2023

LorenzoBettini commented May 17, 2023

LorenzoBettini commented May 17, 2023

LorenzoBettini commented May 17, 2023

LorenzoBettini commented May 17, 2023

LorenzoBettini commented May 17, 2023

HannesWell commented May 17, 2023 • edited Loading

HannesWell commented May 17, 2023

HannesWell commented May 17, 2023

HannesWell commented Aug 24, 2024

github-actions bot commented Aug 24, 2024

Test Results

LorenzoBettini commented Aug 24, 2024

HannesWell commented Apr 20, 2023 •

edited

Loading

HannesWell commented Apr 26, 2023 •

edited

Loading

HannesWell commented May 4, 2023 •

edited

Loading

HannesWell commented May 11, 2023 •

edited

Loading

HannesWell commented May 17, 2023 •

edited

Loading