Skip to content

Rewrite upgrade instructions#1788

Merged
tbrisker merged 5 commits into
theforeman:gh-pagesfrom
ekohl:rewrite-upgrading-guide
Mar 18, 2021
Merged

Rewrite upgrade instructions#1788
tbrisker merged 5 commits into
theforeman:gh-pagesfrom
ekohl:rewrite-upgrading-guide

Conversation

@ekohl

@ekohl ekohl commented Mar 4, 2021

Copy link
Copy Markdown
Member

This limits the manual to RPM/deb packages since that's what most users use. Those who use git or tarballs are expected to handle it themselves.

In the next commit it copies the OS selector from the quickstart. This makes the upgrade instructions shorter since you don't need to skip over irrelevant OSes. For users this is less confusing.

It also removes the foreman-maintain reference. This can be done by moving the cleanup step to a point where the services should be stopped anyway. Avoiding a start-stop cycle makes the overall process shorter. The noop command removes the redundant --dont-save-answers. --noop already implies that.

With all of this in place it's easy to add PostgreSQL upgrade instructions only on EL8. In the previous structure it would have been too confusing.

@ekohl

ekohl commented Mar 4, 2021

Copy link
Copy Markdown
Member Author

@ehelms in theforeman/foreman-installer#646 you had much more complex instructions but I found these to work for me. Could review?

@ekohl ekohl force-pushed the rewrite-upgrading-guide branch 2 times, most recently from c6118ac to a5523a8 Compare March 4, 2021 12:21
Comment thread _includes/manuals/2.4/3.6_upgrade.md Outdated
Comment thread _includes/manuals/2.4/3.6_upgrade.md Outdated
Comment thread _includes/manuals/2.4/3.6_upgrade.md
@ekohl

ekohl commented Mar 9, 2021

Copy link
Copy Markdown
Member Author

I think I addressed the comments. They're in separate commits for easier review.

@ekohl

ekohl commented Mar 10, 2021

Copy link
Copy Markdown
Member Author

I'll need to update this to remove the PG upgrade instructions since we're not going to push that into 2.4.

@ekohl ekohl marked this pull request as draft March 10, 2021 18:02
@ekohl

ekohl commented Mar 10, 2021

Copy link
Copy Markdown
Member Author

And probably also incorporate #1194 which has been open for way too long.

Comment thread _includes/manuals/2.4/3.6_upgrade.md Outdated
*Please note* that there can be significant differences between stable and
development branches. You MUST take care to select a branch and make sure you
get the correct one.
Foreman runs best on PostgreSQL 12 but the default version on EL8 is 10. The installer will try to use version 12 but can't upgrade existing databases. An upgrade can be performed by [switching module streams](https://docs.fedoraproject.org/en-US/modularity/using-modules-switching-streams/).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you decided this no longer holds true (the part about the installer trying to use PG 12). And this upgrade to PG 12 for Foreman 2.4 is an optional step (that we should encourage users to under take).

@ehelms ehelms left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit around the PG upgrade wording, everything else looks good

ekohl added 5 commits March 11, 2021 16:37
This limits the manual to RPM/deb packages since that's what most users
use. Those who use git or tarballs are expected to handle it themselves.
Previously the menu was:

    3. Installing Foreman
        ...
        3.6 Upgrade
          Upgrading to Foreman 2.4

After this change it's

    3. Installing Foreman
        ...
        3.6 Upgrade to 2.4
This copies the OS selector from the quickstart. This makes the upgrade
instructions shorter since you don't need to skip over irrelevant OSes.
For users this is less confusing.

It also removes the foreman-maintain reference. This can be done by
moving the cleanup step to a point where the services should be stopped
anyway. Avoiding a start-stop cycle makes the overall process shorter.

The noop command removes the redundant --dont-save-answers. --noop
already implies that.
Includes a workaround for BZ 1935301 in upgrades. postgresql-upgrade is
unable to deal with data_directory in postgresql.conf. This is added by
puppetlabs/postgresql so pretty much guaranteed to be present.
@ekohl ekohl force-pushed the rewrite-upgrading-guide branch from f2fac59 to 6998b96 Compare March 11, 2021 15:45
@ekohl ekohl marked this pull request as ready for review March 11, 2021 15:45
@ekohl

ekohl commented Mar 11, 2021

Copy link
Copy Markdown
Member Author

I've updated this slightly. All changes are in the last commit (PG upgrade instructions on EL8). It now only recommends to upgrade. Please review and merge when approved.

@ekohl ekohl requested review from ehelms and tbrisker March 11, 2021 15:46

@ehelms ehelms left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tbrisker final review and merge is left to you

<div class="upgrade_os upgrade_os_debian10 upgrade_os_ubuntu1804">
Upgrading from the last release to {{page.version}} has been tested. Updating
the packages will upgrade the application and automatically migrate the
database.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do deb packages still run migrations in postinstall?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 188 to 189
foreman-rake db:migrate
foreman-rake db:seed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we run migrations from the installer and seeds on startup? this should be optional probably

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to go full in on installer only. This procedure is safe and tested for both cases.


su - postgres -c 'vacuumdb --full --dbname=foreman'

##### Optional Step 3 (C) - Run foreman-installer

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

considering we are aiming to have the installer as the main recommended way, shouldn't this come first and not marked as optional (with a comment it isn't relevant if you aren't using the installer)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to got that far. Still plenty of improvement possible but this is now clearer than it used to be (IMHO).

Comment on lines +219 to +233
##### Optional Step 3 (D) - Update config files
<div class="upgrade_os upgrade_os_none">
<i>*No operating system selected.*</i>
</div>
<div class="upgrade_os upgrade_os_el7">
In order to catch all configuration changes and manage them properly you should install and run
rpmconf from the EPEL repository along with vim-enhanced (for vimdiff).

{% highlight bash %}
rpmconf -a --frontend=vimdiff
{% endhighlight %}
</div>
<div class="upgrade_os upgrade_os_el8 upgrade_os_debian10 upgrade_os_ubuntu1804">
This step is irrelevant for the chosen operating system.
</div>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps only show this whole section only for upgrade_os_el7?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to write that complex code. If you use HTML tags, you can't use markdown. So ##### needs to be some HTML tag.

@tbrisker tbrisker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the effort @ekohl ! I agree there is more that can be done but this is already a great improvement.

@tbrisker tbrisker merged commit b2b43db into theforeman:gh-pages Mar 18, 2021
@ekohl ekohl deleted the rewrite-upgrading-guide branch March 18, 2021 17:33
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.

3 participants