fix #9983 MigrateNetworkConfiguration error

[Sky-High]

fix #9983: [Issue]: Starting freshly installed Jellyfin server for the second time fails with MigrateNetworkConfiguration error

Changes
migration.xml now is written also when migrations are skipped during a fresh install of Jellyfin server

Issues
fixes #9983

[cvium]

I don't remember why I wrote the conditions for the prestartup migration stuff, but I don't think this PR is the proper fix. Isn't the issue that "network.xml" doesn't exist? There should be a File.Exists check added to the network migration.

[Kyon147]

For me, comparing a backup version of network.xml to the current one that causes the error - the file looks incomplete. When I copied over my back up - Jellyfin loaded fine. So it does look connected to network.xml that @cvium mentions.

[Sky-High]

So it looks like that there is more than one situation to look at?

The issue that I was addressing is the steps of installing a fresh Jellyfin server, where no AppData\Local\jellyfin directory exists. So no config files at all.

This differs of course from when performing an update where this directory already exists. For a fresh install the config files are created with default values from scratch, so no 'migration' in a strict sense. While in the latter case config files do exist, and need be migrated.

The problem I ran into is that the first run of Jellyfin server, including the installation wizard execution, seems to work fine. The second run however aborted at this line:

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 128 in 1ea8150

logger.LogError(ex, "Could not apply migration '{Name}'", migrationRoutine.Name);

This showed up in the jellyfin server log of the second run as:
[10:31:06] [INF] [1] Jellyfin.Server.Migrations.MigrationRunner: Applying migration 'CreateNetworkConfiguration'
[10:31:06] [INF] [1] Jellyfin.Server.Migrations.MigrationRunner: Migration 'CreateNetworkConfiguration' applied successfully
[10:31:06] [INF] [1] Jellyfin.Server.Migrations.MigrationRunner: Applying migration 'MigrateMusicBrainzTimeout'
[10:31:06] [INF] [1] Jellyfin.Server.Migrations.MigrationRunner: Migration 'MigrateMusicBrainzTimeout' applied successfully
[10:31:06] [INF] [1] Jellyfin.Server.Migrations.MigrationRunner: Applying migration 'MigrateNetworkConfiguration'
[10:31:06] [ERR] [1] Jellyfin.Server.Migrations.MigrationRunner: Could not apply migration 'MigrateNetworkConfiguration'

See the Jellyfin log in #9983 as well.

Analysis

Although the problem happens during the second run, the cause of the problem is before that. The problem indicates issues with the (prestartup) migration, which of course should have been performed during the initial run in the first place.

So I started debugging the first run. I discoverd that the prestartup expects 3 migrations:

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 22 in 1ea8150

private static readonly Type[] _preStartupMigrationTypes =

They all have the PerformOnNewInstall==false attribute which causes them to be caught by the if statement:

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 114 in 1ea8150

if (appliedMigrationIds.Contains(migrationRoutine.Id))

As this holds for all prestartup migration routines, the loop

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 111 in 1ea8150

for (var i = 0; i < migrations.Length; i++)

never reaches the statement

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 135 in 1ea8150

saveConfiguration(migrationOptions);

that writes the performed migrations to the migrations.xml file. So these 3 migration routines are not logged in that file.

Note: even though those migrations where shown in the jellyfin log as being marked:
[10:04:03] [INF] [8] Jellyfin.Server.Migrations.MigrationRunner: Marking following migrations as applied because this is a fresh install: ["CreateNetworkConfiguration", "MigrateMusicBrainzTimeout", "MigrateNetworkConfiguration"]

This causes no further problems during the first run of Jellyfin server. When starting it for the second time however, these 3 prestartup migration routines are not marked as performed as this mark is set in HandleStartupWizardCondition

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 101 in 1ea8150

// If startup wizard is not finished, this is a fresh install.

but that only happens during the first run when isStartWizardCompleted==false.

So the migration routines are not trapped by the if statement

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 114 in 1ea8150

if (appliedMigrationIds.Contains(migrationRoutine.Id))

and are executed while there is nothing to migrate. Which causes Jellyfin server to abort the second run, I presume.

fix
From this debug I concluded that the root cause of the problem is that the (prestartup) migration routines are not saved to the migrations.xml file. The fix is to force writing this file at the start of PerformMigrations

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 107 in 1ea8150

private static void PerformMigrations(IMigrationRoutine[] migrations, MigrationOptions migrationOptions, Action<MigrationOptions> saveConfiguration, ILogger logger)

by means of: saveConfiguration(migrationOptions);

In some cases this might be redundant, but on the other hand, it does not seem to hurt either.

A second adaption is needed for the if statement in HandleStartupWizardCondition:

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 96 in 1ea8150

if (isStartWizardCompleted || migrationOptions.Applied.Count != 0)

The condition migrationOptions.Applied.Count != 0 seems inappropriate, as during execution of Run, HandleStartupWizardCondition is called

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 62 in 1ea8150

HandleStartupWizardCondition(migrations, migrationOptions, host.ConfigurationManager.Configuration.IsStartupWizardCompleted, logger);

with migrationOptions containing the 3 prestartup migration routines, so migrationOptions.Applied.Count==3. So there is work to do for HandleStartupWizardCondition and it should not just return with above if statement.

The reason for condition migrationOptions.Applied.Count != 0 is not really clear in the first place. It might be a relict from earlier versions of this method. Anyways, the condition is removed from the if statement and that seems to do the job.

Notes

• @cvium pointed to the network.xml file. In my test case, this file appears to be created properly during the first Jellyfin server run, when finalizing the installation wizard, and seems not too bad for a default config file, as far as I can see. The only problem I detected was with migrations.xml, and its File.Exists check is implicit in
  

  jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

  Line 81 in 1ea8150

  var migrationOptions = File.Exists(migrationConfigPath)

And tracked by isStartWizardCompleted==false as a result of another File.Exists for system.xml which initially does not exist either:

jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

Line 86 in 1ea8150

var serverConfig = File.Exists(appPaths.SystemConfigurationFilePath)

• @Kyon147: the fresh network.xml indeed differs from the one I already had from a previous installation. But for as far as I can see, it is fine as default for networks.xml for a fresh installation.

• although this fix seems to work for my case of a fresh install, there certainly might be other issues. It will gladly dive into those, but some hints will be helpful. I really hope that @cvium can provide some further clues.

[Shadowghost]

I'd expect setting PerformOnNewInstall=false to prevent this issue

[Sky-High]

I don't remember why I wrote the conditions for the prestartup migration stuff, but I don't think this PR is the proper fix. Isn't the issue that "network.xml" doesn't exist? There should be a File.Exists check added to the network migration.

By the way, as you were probably hinting at, there might be another issue, with MigrateNetworkConfiguration.

jellyfin/Jellyfin.Server/Migrations/PreStartupRoutines/MigrateNetworkConfiguration.cs

Line 12 in 1ea8150

public class MigrateNetworkConfiguration : IMigrationRoutine

Running an already installed Jellyfin server (so with existing configuration files; not a first run of a fresh install) after manually removing the entry of this migration routine from migrations.xml, the error described in the Jellyfin log at #9983 also occurs.

To me this is a different issue as I was specifically looking at a fresh install. But I can register it as a separate issue, and look into it.

[Sky-High]

I'd expect setting PerformOnNewInstall=false to prevent this issue

The 3 prestartup migration routines do have PerformOnNewInstall=false. In fact, that was just what revealed the problem: having all prestartup migrations attributed with PerformOnNewInstall=false causes them not to be logged in migrations.xml.

[Sky-High]

To further explain:

• migration routines with PerformOnNewInstall==false are compiled in onlyOldInstalls
  

  jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

  Line 102 in 1ea8150

  var onlyOldInstalls = migrations.Where(m => !m.PerformOnNewInstall).ToArray();

• and subsequently in migrationOptions.Applied:
  

  jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

  Line 104 in 1ea8150

  migrationOptions.Applied.AddRange(onlyOldInstalls.Select(m => (m.Id, m.Name)));

• which is hashed into appliedMigrationIds
  

  jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

  Line 109 in 1ea8150

  var appliedMigrationIds = migrationOptions.Applied.Select(m => m.Id).ToHashSet();

• and then skipped in the for loop
  

  jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

  Line 114 in 1ea8150

  if (appliedMigrationIds.Contains(migrationRoutine.Id))

• so if all migration routines havePerformOnNewInstall==false then everything is skipped and the statement for save-to-file migrations.xml is never reached:
  

  jellyfin/Jellyfin.Server/Migrations/MigrationRunner.cs

  Line 135 in 1ea8150

  saveConfiguration(migrationOptions);

[Sky-High]

This PR now also includes a fix for MigrateNetworkConfiguration.cs. Two issues are addressed:

• if the network.xml config file contains an error, Jellyfin still complains about that in the log but does not abort. This can be of particular importance if the file had already be migrated, and so contains a <VirtualInterfaceNames><string>veth</string></VirtualInterfaceNames> item. This is not according to the 'old' syntax, and the migration issues an error. But now it just complains, while without the fix Jellyfin server would abort.

• MigrateNetworkConfiguration reads the 'old' network.xml with an Xml Deserialize:

  jellyfin/Jellyfin.Server/Migrations/PreStartupRoutines/MigrateNetworkConfiguration.cs

  Line 43 in 1ea8150

  var oldNetworkConfiguration = (OldNetworkConfiguration?)oldNetworkConfigSerializer.Deserialize(xmlReader);

  and after migration writes it again with Xml Serialize
  

  jellyfin/Jellyfin.Server/Migrations/PreStartupRoutines/MigrateNetworkConfiguration.cs

  Line 86 in 1ea8150

  networkConfigSerializer.Serialize(xmlWriter, networkConfiguration);

  The latter call failed as the file networks.xml was still open because of the first call.
  This has now been fixed with using blocks.

[Xabis]

I ran this PR trying to get past this issue.

While this does prevent the crash on 2nd run, the network.xml is never created even with no/empty data directory.

I do not know if it is related, but a few of the ip bindings were failing, related to network discovery.

[Sky-High]

@Xabis : thank you for your comment.

Did you complete fresh installation with the initial setup wizard?

The fresh installation begins with starting the Jellyfin server, and then continues by completing the setup wizard in the web client. The network.xml configuration file indeed is not created during the step of starting the Jellyfin server. It is only created after completing the setup wizard and entering the appropriate values where asked for. At least in my case, the resulting network.xml looks ok to me.

This procedure did make sense to me. The problem I was addressing occurred in the first step, starting the Jellyfin server and is related to migrating functionality, and not to setup functionality, and I can imagine that the migration functions defer installation issues (like creation of the network.xml file) to the startup wizard. I was a little wary of changing the procedure in this respect, so my fix aimed at a flaw related to the migration.xml file. And in my case, the fix works as expected.

Anyway, I am not sure if your issue is directly related to what I ran into, but if you can elaborate on what exactly your findings are I can of course try to find an answer for that as well. Preferably by first finalizing the present PR and opening a new issue and PR, not to mix up basically different problems.

[Xabis]

@Sky-High, I can confirm the file gets written post initial setup; thanks for clarifying the creation path.

I am attempting to run the server in debug mode, directly inside visual studio, and have been running into issues. One of them being the first time setup not working properly, due to socket failures in the SSDP network discovery functionality.

Apologies for the churn.