foreman-documentation Server upgrade procedures

Still a draft because I'm building this on top of https://github.com/theforeman/foreman-documentation/pull/3855 which hasn't been merged yet.

What changes are you introducing?

Reviewing procedures for upgrading connected and disconnected server. This includes the following changes:

A new section named 'Backing up custom Foreman Server configuration before upgrading' which groups together steps to back up config that would otherwise be overwritten by the upgrade. (This section is now shared between connected and disconnected procedures, which helps reduce duplication.)
Grouping together related steps in the upgrade procedure. (This helps reduce the number of major steps in each procedure.)
Other, mostly minor changes to the wording

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Suboptimal upgrade experience. I shared a more detailed explanation and justification in https://issues.redhat.com/browse/SAT-29153

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

I haven't made many changes in the disconnected procedure because that is being updated in https://github.com/theforeman/foreman-documentation/pull/3725

Checklists

[x] I am okay with my commits getting squashed when you merge this PR.
[x] I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

[ ] Foreman 3.14/Katello 4.16
[ ] Foreman 3.13/Katello 4.15 (EL9 only)
[ ] Foreman 3.12/Katello 4.14 (Satellite 6.16; orcharhino 7.2 on EL9 only)
[ ] Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only; orcharhino 7.0 on EL8+EL9; orcharhino 7.1 with Leapp)
[ ] Foreman 3.10/Katello 4.12
[ ] Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
[ ] Foreman 3.8/Katello 4.10
[ ] Foreman 3.7/Katello 4.9 (Satellite 6.14)
We do not accept PRs for Foreman older than 3.7.

May 19 '25 16:05 aneta-petrova

The PR preview for ca5241c4281e8128b743a95a736c4b5573c9a6f7 is available at theforeman-foreman-documentation-preview-pr-3867.surge.sh

The following output files are affected by this PR:

show diff

show diff as HTML

May 19 '25 16:05 github-actions[bot]

In chapter 2. Upgrading Foreman there's this intro:

Upgrading Foreman includes upgrading your Foreman server and upgrading Smart Proxy servers.

For foreman-deb we don't have any instructions on upgrading Smart Proxies. Perhaps we should? But taking a step back: is this really a useful overview of the chapters? In other versions there are also more steps, like upgrading the external DB. That's not mentioed here.

It's not the most useful of overviews but still better than the original "Use the following procedures to upgrade your existing Foreman to Foreman 3.14.", which is what we had there before https://github.com/theforeman/foreman-documentation/commit/16ed4ed44aaef56ae667aad2142c7e186e3af2cb was merged ("procedure" is self-referential language that we shouldn't be using).

It's true that some versions have more steps, which is why the overview uses "includes" and not "consists of". This will be useful if in the future more actions are added (like database upgrades) -- there is no risk of the upgrade process going out of sync with the overview. Besides, we already have a detailed high-level overview in "Upgrade path overview", I wouldn't want to recreate it here.

The fact that we don't have Smart Proxy upgrade procedure for Debian is unfortunate and I didn't notice the disconnect before. I'll be looking at the proxy upgrade procedure in a follow-up PR and I'm making a note that it's something I should ask about.

Then Appendix A: Troubleshooting permission issues

I wonder how often this happens. It mentions "pre-upgrade" checks but that's something we only do in foreman-maintain. It was added in 702aa1d and I'm so confused by it so I'm going to write out my thoughts.

It complains that it couldn't add a role because of missing permissions. Creating roles is something we do during seeding so if the fix is seeding we're certainly doing something wrong. This should never happen and if it does, it's a bug. In the past we have seen that some plugins used wrong seeds that were still 2 digits (nn-name.rb) instead of 3 digits (nnn-name.rb), as is needed sincehttps://github.com/theforeman/foreman/commit/d03c67d3e0b11275b50a31005874c037a8bef46e. Then the priority on sorting seeds was messed up. Looking at https://github.com/theforeman/foreman_ansible/tree/master/db/seeds.d that is indeed the case and a bug. It may not actually solve the bug, but it's certainly wrong behavior.

Another note: foreman-maintain health check --label duplicate_permissions can't catch that specific error message so running it isn't a good step to identify them and thus also not a good verification.

I plan to look at the troubleshooting part of the process in a follow-up PR. I'm making a note about this in the downstream ticket I have.

May 22 '25 05:05 aneta-petrova

To summarize the current status (for my sake but also for anyone else who's following the progress):

Writers have reviewed, so I'm setting style review done.
All tech review so far has been implemented, the only open thread https://github.com/theforeman/foreman-documentation/pull/3867#discussion_r2099689161 is waiting for further input. This is a blocker that needs to be resolved before merging.
A potential usability concern in https://github.com/theforeman/foreman-documentation/pull/3867#discussion_r2100616717 is not a blocker.

I'll ask for a tech re-review and a final ack once the blocker around discovered hosts is resolved.

May 22 '25 08:05 aneta-petrova

@evgeni Can you please re-review? Is there anything else that we should address?

May 26 '25 09:05 aneta-petrova

@lzap would simply rebooting the discovered hosts (without deleting them) also work? I'd think they'll refresh the facts

May 26 '25 15:05 ekohl

would simply rebooting the discovered hosts (without deleting them) also work? I'd think they'll refresh the facts

Yes that is a reasonable thing to do, actually, should not be necessary. I vaguely remember there is a loop and they upload facts every N minutes.

May 26 '25 18:05 lzap

Yes that is a reasonable thing to do, actually, should not be necessary. I vaguely remember there is a loop and they upload facts every N minutes.

There is indeed a loop, but only on change: https://github.com/theforeman/foreman-discovery-image/blob/4e8e664ec8cd49016d139c8260bb8e73b89953ac/root/usr/bin/discovery-register#L65-L85. Those look like the legacy facts only so perhaps that has broken now and it always uploads. Otherwise it won't do anything.

@aneta-petrova my proposal would then be to change this to an optional step after upgrading.

One nuance in that is that the discovery image is on the TFTP server which means you should reboot the discovered hosts connected to the server you're upgrading. So either {ProjectServer} or the relevant {SmartProxyServer}, but we can ignore that for now.

May 27 '25 10:05 ekohl

@aneta-petrova my proposal would then be to change this to an optional step after upgrading.

@ekohl I added it as another bullet point to the post-upgrade section. Do you think this will work? I didn't make it optional (Optional:) on purpose because from what I can see, users should always do it if the have discovered hosts. Limiting it with a condition (If your server shows discovered hosts:) seems like a better choice.

May 27 '25 13:05 aneta-petrova

I applied the latest suggestion to the post-upgrade tasks section. I'm not asking for another round of style re-reviews for that addition because I'll be looking into reviewing the whole section soon so there will be a chance to revisit it.

Style and tech acks are in place so this is now ready to merge.

May 28 '25 12:05 aneta-petrova