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.

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

[JENKINS-68149] Handle old WAR location from RPM #301

Merged
merged 1 commit into from
Apr 1, 2022

Conversation

basil
Copy link
Member

@basil basil commented Mar 30, 2022

Thanks to @meiswjn for reporting this issue in JENKINS-68149. The problem here is that it is all too easy to mess up the merge of the old sysconfig script when upgrading, which then results in the migration script thinking that you've explicitly changed the WAR path when in fact you just messed up the merge. I actually messed this up myself once while testing, which is why I added an explicit check for this case in the migration script. Ah, but I only handled the Debian case, not the RPM case. So if you hit this edge case (messing up the merge for the RPM version) you could very well hit a problem. This PR removes the sharp edge by adding additional logic for the RPM case.

@basil basil added the bug label Mar 30, 2022
@basil basil requested a review from a team as a code owner March 30, 2022 06:34
Copy link
Member

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

Looks reasonable. Untested.

@meiswjn
Copy link

meiswjn commented Mar 30, 2022

That was fast! I can test this as we still got a similar system to update.

@timja
Copy link
Member

timja commented Mar 30, 2022

That was fast! I can test this as we still got a similar system to update.

I assume you need it in a release to test it? or were you going to copy the migration script from your other system and patch it to test?

@meiswjn
Copy link

meiswjn commented Mar 30, 2022

That was fast! I can test this as we still got a similar system to update.

I assume you need it in a release to test it? or were you going to copy the migration script from your other system and patch it to test?

We will edit the RPM of 2.332.1 according to this PR and then try it on the new system. We'll try this in an hour.

@meiswjn
Copy link

meiswjn commented Mar 30, 2022

Sadly we were unsuccessful in patching the RPM, so we would need a release to test it.

@basil
Copy link
Member Author

basil commented Mar 30, 2022

The .rpm file from the PR build (1.1 is Red Hat, 1.2 is openSUSE) contains the standard 2.341 WAR with the changes from this PR, but it is signed with the test key rather than the official key, so you may not want to install it on a production server.

@basil
Copy link
Member Author

basil commented Mar 31, 2022

@meiswjn Were you interested in installing the package from the PR build or would you like us to merge and release this in the next weekly?

@meiswjn
Copy link

meiswjn commented Apr 1, 2022

I decided to just quickly deploy a new RHEL machine and test this. It works:
2.319.1 systemctl status:
└─40475 /etc/alternatives/java -Djava.awt.headless=true -DJENKINS_HOME=/var/lib/jenkins -jar /usr/lib/jenkins/jenkins.war --logfile=/var/log/jenkins/jenkins.log --webroot=/var/cache/jenkins/war

Immediately afterwards, upgrade to 2.341.1:
└─40635 /etc/alternatives/java -Djava.awt.headless=true -DJENKINS_HOME=/var/lib/jenkins -jar /usr/share/java/jenkins.war --logfile=/var/log/jenkins/jenkins.log

(Service startup succesful in both cases)

@MarkEWaite MarkEWaite merged commit 7b05b58 into jenkinsci:master Apr 1, 2022
@MarkEWaite
Copy link
Contributor

@timja and @basil should we consider this as an lts-candidate for 2.332.2 that is scheduled to release next week or should it wait for 2.332.3?

@basil basil deleted the rpm branch April 1, 2022 14:49
@basil
Copy link
Member Author

basil commented Apr 1, 2022

I say go for 2.332.2. The fix is low risk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants