Update check without shutdown

[CosmicHorrorDev]

Closes: #129

Description

Contributions

This is obviously still missing some bits that I'm planning on finishing up tomorrow, but I've got some other stuff to get done now so I'm just submitting my current progress.

Accomplishes

• Basic structure for odin update subcommand including respecting --dry-run, --check, and --force flags
• Moved server base logic into a separate module
  • Done because updating requires access to starting, stopping, and installing the server

TODO

• Add in parsing installed and latest buildid
  • Add unit tests for this
• Double check that the command for the latest buildid doesn't use the cache still
• Add in env var flag to check for updates on shutdown (or maybe startup, or both?)
• Change update cron script to use the new update subcommand
• Update any relevant documentation

Checklist

• I added one or multiple labels which best describes this PR.
• I have tested the changes locally.
• This PR has a reviewer on it.
• I have validated my changes in a docker container and on Ubuntu. (Only needed for Odin or Docker Changes)

[CosmicHorrorDev]

We just had an update so there's likely a decent amount of time before another one hits. One thing I've been pondering would be the use of adding a --backup {{ BACKUP_NAME }}. The use case is that doing a backup before an update would be a common use case and would make something like this update command much simpler

if odin update --check; then
    # Time to update
    pidof valheim_server.x86_64
    SERVER_RUNNING=$?

    if [[ -n $SERVER_RUNNING ]]; then
        odin stop
    fi

    odin backup blah blah blah
    odin update

    if [[ -n $SERVER_RUNNING ]]; then
        odin start
    fi
fi

to simply

odin update --backup blah blah blah

I think this would be nice since it drastically reduces the complexity from how we will likely do backups (can still do checking for if they want to backup on update, but it should still be much much simpler.

[mbround18]

We just had an update so there's likely a decent amount of time before another one hits. One thing I've been pondering would be the use of adding a --backup {{ BACKUP_NAME }}. The use case is that doing a backup before an update would be a common use case and would make something like this update command much simpler

if odin update --check; then
    # Time to update
    pidof valheim_server.x86_64
    SERVER_RUNNING=$?

    if [[ -n $SERVER_RUNNING ]]; then
        odin stop
    fi

    odin backup blah blah blah
    odin update

    if [[ -n $SERVER_RUNNING ]]; then
        odin start
    fi
fi

to simply

odin update --backup blah blah blah

I think this would be nice since it drastically reduces the complexity from how we will likely do backups (can still do checking for if they want to backup on update, but it should still be much much simpler.

I would like to keep the backup command separate for a separation of concern, I do want to add a restore command which would unzip the contents for the user. Thoughts?

[CosmicHorrorDev]

That sounds good on the --backup flag! I'll switch the scripts over to use the new update subcommand, update the documentation, and manually test the changes and I think we should be good to go here after that!

And I do agree that the backup subcommand could use some more love. I think ideally something like

odin backup [create | restore] FILEPATH

would work out really well. Doing a nested subcommand helps group the functionality better, and since odin knows about backups are being created from / restored to we should only need the filepath to the backup itself. I also think it would be a good idea to have the restore subcommand make another backup before restoring, so that people don't accidentally wipe any information when they're going to restore one of their backups. Lmk what you think!

[CosmicHorrorDev]

I think most of the logic should be good now. I've tested odin update --check and odin update with a stale server I was keeping around and it seemed to work right, but I'm gonna go through more thorough testing to make sure that

• --force and --dry-run both work correctly in all cases
• The new auto_update.sh logic works correctly including
  • Starting the server again only if it was running before
  • Only attempting an update when an update was available
  • Backup still working correctly
• That the cron job still works correctly too

And I still need to add in the new documentation that shouldn't take very long

[CosmicHorrorDev]

Still need to address pending actions from other comments with testing and updating documentation too

[CosmicHorrorDev]

Alright all manual testing done, and I'm pretty happy with the final state of things, so it's finally time to graduate this to a normal PR 🎉

Overview

Restructuring

• Functionality dealing with manipulating the server was moved into crate::server
  • Everything is grouped into different modules that are rexported just to keep things organized
• Mod specific stuff was moved into crate::mods
• Some common constants got moved to crate::constants

Update Functionality

Externally you can manage different update functionality with odin [ --dry-run ] update [ --check | --force ] each flag of which should be handled correctly. This usage can be seen in the modified auto_update.sh file, but should also be usable for anyone who wants to do custom scripting using it.

Within odin all update functionality is defined within crate::server::update.

• update_server essentially follows the same actions as odin update --force
• update_is_available is a convenience function to easily check if an update is available. This function takes a decent bit of time since it queries the steam api, so it's best to store the returned value and only re-fetch it as needed
• Lastly UpdateInfo provides a way to get the underlying build ids for the server installation and latest available one according to steam. This information is useful for debugging purposes, or in any other case where the user may want to know the build ids

Documentation

The description under AUTO_UPDATE was changed to reflect that shutting down is no longer required.

Note: I was going to update the odin docs to reflect the new subcommand, but they're already out of date so I'm just going to defer that responsibility to a later PR

Testing

I manually tested this a decent bit, but having more people test things out is never a bad thing :D

Additionally I added in unit tests that should cover all the update checking functionality!

[dkurzaj]

The dry run and detection seem to work well. But I did not manage to simulate an update by changing the value of the buildid in steamapps/appmanifest_896660.acf. Odin did detect that a new version existed when I modified it, but steamcmd seems to check somewhere else because that only change was not enough for it to accept to update.

$ odin update -f
[ODIN][INFO]  - Run with DEBUG_MODE as 1 if you think there is an issue with Odin
[ODIN][INFO]  - Checking for updates
[ODIN][ERROR] - Failed to find steamcmd in path
[ODIN][ERROR] - Checking for script under steam user.
[ODIN][INFO]  - Executing: /home/steam/steamcmd/steamcmd.sh .....
[ODIN][INFO]  - Using steamcmd script at /home/steam/steamcmd/steamcmd.sh
[ODIN][INFO]  - A server update is available!
[ODIN][INFO]  - Scanning for Valheim process
[ODIN][INFO]  - Found Process with pid 566! Sending Interrupt!
[ODIN][INFO]  - Process signal interrupt sent successfully!
[ODIN][INFO]  - Waiting for server to completely shutdown...
[ODIN][INFO]  - Server has been shutdown successfully!
[ODIN][INFO]  - Installing 896660 to /home/steam/valheim
[ODIN][ERROR] - Failed to find steamcmd in path
[ODIN][ERROR] - Checking for script under steam user.
[ODIN][INFO]  - Executing: /home/steam/steamcmd/steamcmd.sh .....
[ODIN][INFO]  - Using steamcmd script at /home/steam/steamcmd/steamcmd.sh
Redirecting stderr to '/home/steam/Steam/logs/stderr.txt'
[  0%] Checking for available updates...
[----] Verifying installation...
Steam Console Client (c) Valve Corporation
-- type 'quit' to exit --
Loading Steam API...OK.

Connecting anonymously to Steam Public...Logged in OK
Waiting for user info...OK
Success! App '896660' already up to date.
[ODIN][INFO]  - Checking for BepInEx Environment...
[ODIN][INFO]  - Server has been started and Daemonized. It should be online shortly!
[ODIN][INFO]  - Keep an eye out for 'Game server connected' in the log!
[ODIN][INFO]  - (this indicates its online without any errors.)

As you can see it only restarts my server without trying to download a new version because steamcmd says it's "already up to date". So it's probably just my test method which wasn't great. Otherwise, dry run, auto-detection with a change to the buildid and force option worked alright!

So, as far as I was able to test it, this PR seems great to me, thank you! :)

[CosmicHorrorDev]

Glad that you were willing to try it out! I've got an outdated server instance I'm keeping around and it does perform an update on it when I run odin update. The real trial will be when another official update occurs :D

[dkurzaj]

@mbround18 do you think the PR can be merged? I see that 1 change is marked as requested and blocking the merge, but I can not see any unresolved conversation, that's strange (but I'm more familiar with Gitlab reviewing process, so maybe I'm missing something).

[CosmicHorrorDev]

He's looking over it tonight!

[mbround18]

@mbround18 do you think the PR can be merged? I see that 1 change is marked as requested and blocking the merge, but I can not see any unresolved conversation, that's strange (but I'm more familiar with Gitlab reviewing process, so maybe I'm missing something).

I am going to be running this on multiple systems after work today to validate it. I have done a precursory set of tests to check and the code looks good but I want to push it up to some live environments to test.

[mbround18]

@dkurzaj and all,

Image up for testing: mbround18/valheim:development-update

Use it for a preview, this is going up on several servers right now. As a heads up, I am going to let it bake over night to validate the auto update cron and a few things.