-
Notifications
You must be signed in to change notification settings - Fork 383
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
(nexus-repository-migrator) Fixes Nexus 3.71 Breaking Change Upgrade #2519
base: master
Are you sure you want to change the base?
Conversation
b645a6d
to
ebd8648
Compare
✅ Package verification completed without issues. PR is now pending human review |
✅ Package verification completed without issues. PR is now pending human review |
My one fun question is "do we want to handle trying to fix up installs where they have attempted to upgrade to 3.71.0.6 and it's failed"? I suspect they will be in a fun state. Edit: I think this should sort of work, now. Final edit: popped a bonus script into the migrator package. |
❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look. |
f49b23c
to
dad9cf2
Compare
✅ Package verification completed without issues. PR is now pending human review |
dad9cf2
to
cacfb4a
Compare
✅ Package verification completed without issues. PR is now pending human review |
cacfb4a
to
4ac67da
Compare
✅ Package verification completed without issues. PR is now pending human review |
4ac67da
to
62c5f08
Compare
✅ Package verification completed without issues. PR is now pending human review |
62c5f08
to
b8d0143
Compare
✅ Package verification completed without issues. PR is now pending human review |
❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look. |
Just to keep people updated: I'm (we're) happy with the code in this PR, so it's ready for review. However, we're keeping it in draft because Sonatype have a few changes they're making to the migrator to not break indexes and such. Until that new version is released, we [probably] shouldn't release this package. It's arguable. That said, please do let loose with feedback. Edit: It looks like it's been released. |
39f0b6e
to
830407d
Compare
✅ Package verification completed without issues. PR is now pending human review |
830407d
to
67b9cbf
Compare
✅ Package verification completed without issues. PR is now pending human review |
1 similar comment
✅ Package verification completed without issues. PR is now pending human review |
Stevie's out for the next while, and has expressed that I'm good to dismiss it in favour of other reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither a solid approve or change request from me, just the result of my personal testing.
As mentioned in the inline comments, I originally didn't see that the migrator package had a parameter that needed to be passed in if I wanted to attempt the migration at install time. This led to issues with my trying to run the migrator after the fact.
I mentioned at least one variable defined in the install which is then used in the migration script (I didn't see any others) but even when manually re-creating that variable the migration did not succeed.
I ended up having to force a re-install of the migration package with the /AttemptMigration
parameter and then the migration worked as expected.
I'm not sure if the best way to stop people like me from holding this wrong is in the documentation (i.e. adding to the "you need to install this migration package" error) or code (making attempting the migration the default behaviour), that is probably worth a discussion?
automatic/nexus-repository-migrator/nexus-repository-migrator.nuspec
Outdated
Show resolved
Hide resolved
automatic/nexus-repository-migrator/nexus-repository-migrator.nuspec
Outdated
Show resolved
Hide resolved
91e91ec
to
81be408
Compare
✅ Package verification completed without issues. PR is now pending human review |
3.72 has now been released; it would be good to get that packaged and pushed. Does anyone fancy re-reviewing this, based on the changes made to address the feedback given? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note I'm not reviewing the code or testing the install / upgrade / uninstall.
Nexus' recent update has added some fairly catastrophic breaking changes, and necessitated a large addition to the install script for the sake of migrating the database to the new H2 format (previously only available to pro customers) This new migration process: - Checks to see if the version we're on can be migrated from (e.g. ==3.70) - Checks to see if the install is using OrientDb (and so needs to be upgraded) - Checks to see if the machine has sufficient memory and disk space - Backs up the OrientDb files from the $DataDir\db directory - Downloads the migration tool - Migrates the backed up databases - Copies the migrated file(s) back to the database directory - Updates the Nexus properties to enable the datastore - Deletes the entire target folder (there's a separate discussion that we may want to be doing this anyway) - Continues with the standard upgrade process This works back to PowerShell v3, largely due to `Compress-ArchiveCompat`
Sources all connection variables in Wait-NexusAvailability (other than Hostname) from the on-disk configuration. Also automatically backs-up and restores the port and SSL configuration if they are defined in the configuration.
It was raised that we should have a separate package for the Nexus Repository Migrator logic. This commit removes that code from the Nexus Repository package, and adds it to a separate package that can be installed.
The upgrade to JRE exposed that the package was simply copying new files over the old ones, which can break things when old files combine with new. This change solidifies the package logic to backup SSL configuration if it's present, and specifically removes the program directory before re-copying the latest files. This should ensure that Nexus can start, after the upgrade.
The post-migration launch process can take significantly longer than normal, due to the additional work Nexus is doing. This change adds a configurable timeout to the Wait-NexusAvailability command, and uses it in the Nexus-Repository-Migrator package to increase the timeout from 3 minutes to 15.
81be408
to
1857bc4
Compare
✅ Package verification completed without issues. PR is now pending human review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I've said this a few times, but just being clear again, I have not checked the code or tested this package. Just the wording.
I can re-do my testing of the code in the morning, should have results up ~16 hours |
@TheCakeIsNaOH as you reviewed this previously, are you able to review this again and approve or add comments? Unless anybody else feels strongly, once we have @Windos and @TheCakeIsNaOH review, I suggest we merge this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so, I tested this against a testing restore of a "real" Nexus instance that had an OrientDB. This Nexus instance uses S3 storage to host the configured repos. Despite not having access to this storage, prior to the upgrade Nexus started up fine. After the upgrade, Nexus is now no longer starting up and I can't see a specific reason for this in the logs. I actually think it was to do with a NuGet v2 feed (and so it was a Nexus issue and not an upgrade issue).
Chasing that hunch, I did a take 2 and removed the suspect repo before going through the migration, and it worked!
I did run into some other rough edges which I've commented directly on the code itself. The result of those rough edges is that at the end of the DB migration and Nexus upgrade was it couldn't detect that Nexus was back up and running (when it absolutely was.)
tl;dr the migration and upgrade work, but in my case the rough edge of "not using the FQDN I provided" resulted in the package install/upgrades to "fail" due to not being able to validate that Nexus web interface was running.
} | ||
} | ||
|
||
function Wait-NexusAvailability { | ||
function Get-NexusUri { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had issues with this function. The cert on my Nexus server is a wild card, so it doesn't have a common name or anything on it that is specific to the server nor that would match the given FQDN that I pass in as a package parameter.
Also, despite my given said FQDN, it ended up using the hostname. My expectation as a user is if I provide an FQDN, then that should be used (regardless of what the cert may say... if I've provided it and I got it wrong, then it's my own fault for shooting myself in the foot (imho)).
Write-Warning "Could not figure out SSL configuration for $($env:ComputerName)" | ||
$env:ComputerName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would personally prefer this to default to localhost rather than computer name. Some environments (mine 😅) aren't going to resolve the computer name to something useful for the purposes of ensuring Nexus started.
Description
We now have a new package,
nexus-repository-migrator
, that contains logic to migrate the Nexus Repository database from 3.70.* to 3.71.*.This new migration process:
By default, the package just downloads the migrator file and adds the
ConvertFrom-NexusOrientDb
script to PATH. You can also pass/Migration
to run it during the package installation.After this is completed, you should be able to upgrade using the next available version of Nexus Repository - the 3.71.0.6 version will still break due to the merged files!
This works back to PowerShell v3, largely due to
Compress-ArchiveCompat
.As mentioned, we now ensure that we are removing and replacing the program directory instead of copying over / merging it with the new content, as the upgrade to JRE and Nexus broke stuff with that strategy. The changes now source all connection variables in Wait-NexusAvailability (other than Hostname) from the on-disk configuration, and automatically backs-up and restores the port and SSL configuration if they are defined in the configuration. This should result in a smoother experience for people in future upgrades, and ensure
Wait-NexusAvailability
fails less frequently on SSL enabled installations.Finally, we re-enable the update script for Nexus Repository.
The update script for the package will only find packages that match the 3.70 to 3.71 migration, currently! This is a mindful choice that we can reconsider in the future.
Motivation and Context
Nexus' recent update has added some fairly catastrophic breaking changes, and
necessitated a large addition to the install script for the sake of migrating
the database to the new H2 format (previously only available to pro customers)
Fixes #2516
How Has this Been Tested?
Types of changes
[ ] Bug fix (non-breaking change which fixes an issue)[ ] New feature (non-breaking change which adds functionality)[ ] Migrated package (a package has been migrated from another repository)Checklist:
The changes affect two packages, but this has been agreed with other members of the community to be alright.