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

(nexus-repository-migrator) Fixes Nexus 3.71 Breaking Change Upgrade #2519

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

JPRuskin
Copy link
Contributor

@JPRuskin JPRuskin commented Aug 14, 2024

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:

  • 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

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?

  • Tested on Windows Server 2022 (PowerShell 5), upgrading from 3.70.x, with configured SSL
  • Tested on Windows Server 2012 (PowerShell 3), upgrading from 3.70.x, without SSL

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Migrated package (a package has been migrated from another repository)

Checklist:

  • My code follows the code style of this repository.
  • My change requires a change to documentation (this usually means the notes in the description of a package).
  • I have updated the documentation accordingly (this usually means the notes in the description of a package).
  • I have updated the package description and it is less than 4000 characters.
  • All files are up to date with the latest Contributing Guidelines
  • The added/modified package passed install/uninstall in the Chocolatey Test Environment(https://github.com/chocolatey-community/chocolatey-test-environment/). Note that we don't support the use of any other environment.
  • The changes only affect a single package (not including meta package).

The changes affect two packages, but this has been agreed with other members of the community to be alright.

@JPRuskin JPRuskin requested a review from a team as a code owner August 14, 2024 18:13
@JPRuskin JPRuskin requested a review from Windos August 14, 2024 18:13
@JPRuskin JPRuskin requested a review from AdmiringWorm August 14, 2024 18:20
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

automatic/nexus-repository/nexus-repository.nuspec Outdated Show resolved Hide resolved
automatic/nexus-repository/tools/chocolateyinstall.ps1 Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@JPRuskin
Copy link
Contributor Author

JPRuskin commented Aug 15, 2024

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.
Edit Edit: No, changed my mind. If 3.71 has been dropped, JRE is >11, so the migrator will no longer have a compatible version of JRE. People with a failed upgrade are in a Bad State™️, and need manual intervention to get out.

Final edit: popped a bonus script into the migrator package.

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@JPRuskin JPRuskin marked this pull request as draft August 15, 2024 15:03
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@AppVeyorBot
Copy link

❌ Package verification failed, please review the Appveyor Logs and the provided Artifacts before requesting a human reviewer to take a look.

@JPRuskin JPRuskin changed the title (nexus-repository) Fixes Nexus 3.71 Breaking Change Upgrade (nexus-repository-migrator) Fixes Nexus 3.71 Breaking Change Upgrade Aug 28, 2024
@JPRuskin
Copy link
Contributor Author

JPRuskin commented Sep 3, 2024

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.

@JPRuskin JPRuskin marked this pull request as ready for review September 4, 2024 10:12
@JPRuskin JPRuskin requested a review from a team as a code owner September 4, 2024 10:12
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

1 similar comment
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@JPRuskin JPRuskin dismissed steviecoaster’s stale review September 12, 2024 10:47

Stevie's out for the next while, and has expressed that I'm good to dismiss it in favour of other reviews.

Copy link
Member

@Windos Windos left a 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?

@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@JPRuskin
Copy link
Contributor Author

JPRuskin commented Sep 30, 2024

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?

@Windos @pauby @steviecoaster

Copy link
Member

@pauby pauby left a 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.
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@pauby pauby self-requested a review October 1, 2024 08:20
Copy link
Member

@pauby pauby left a 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.

@Windos
Copy link
Member

Windos commented Oct 1, 2024

I can re-do my testing of the code in the morning, should have results up ~16 hours

@pauby
Copy link
Member

pauby commented Oct 1, 2024

@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.

Copy link
Member

@Windos Windos left a 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!
image

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.)
image

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.

automatic/nexus-repository/tools/helpers.ps1 Show resolved Hide resolved
}
}

function Wait-NexusAvailability {
function Get-NexusUri {
Copy link
Member

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)).

Comment on lines +107 to +108
Write-Warning "Could not figure out SSL configuration for $($env:ComputerName)"
$env:ComputerName
Copy link
Member

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.

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

Successfully merging this pull request may close these issues.

(nexus-repository) 3.71.* breaks upgrade
6 participants