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

Automation of upgrades for standalone servers #6583

Merged
merged 70 commits into from Oct 6, 2021
Merged

Conversation

julsemaan
Copy link
Collaborator

@julsemaan julsemaan commented Sep 16, 2021

NOT READY

Missing:

  • Tracking monit configuration in pf.conf
  • Upgrade script to migrate existing monit configuration in pf.conf
  • Regeneration of monit configuration
  • Have monit use the SMTP configuration in 'alerting' (with support for authentication and TLS)
  • packetfence-local-auth in packetfence-tunnel should be configurable via pf.conf
  • Cluster detection and bail-out
  • Allow to update the OS or not (currently doesn't do it)
  • Include file for iptables.conf
  • Handle broken git_commit_id file (see The git_commit_id file seems to be broken #6596), we can probably use 73d3647 or a more recent commit ID as a fallback since no changes were pushed in the config files since the 11.0 release
  • Disable monit before the upgrade (will get reenabled by updatesystemd at the end)
  • Create a package to use script on 11.0.0 version
  • Update upgrade documentation to reflect changes (install guide should be done)
  • Downloadable hook scripts (pre package upgrade, post package upgrade, etc)
    See Automation of upgrades for standalone servers #6583 (comment) for details

Description

Allows to upgrade a standalone server via a single script that performs the whole upgrade

Impacts

We won't need @lzammit to do the upgrades anymore

Delete branch after merge

YES

Checklist

(REQUIRED) - [yes, no or n/a]

  • Document the feature
  • Add unit tests
  • Add acceptance tests (TestLink)

NEWS file entries

TODO

@julsemaan
Copy link
Collaborator Author

Leaving a note here for the hooks logic, will need to discuss this with @nqb and the rest of the team:

I added a logic to allow defining hooks in addons/upgrade/hooks/ at different stages of the upgrade. Defining hooks with the expected file names in this directory will allow to hook-into the upgrade process

The content of this directory should be managed by a seperate package built on maintenance/X.Y and do-upgrade.sh will have to upgrade it before running the "real" upgrade process

@julsemaan
Copy link
Collaborator Author

Other than the 3 missing items in the TODO which I need @nqb's feedback and help with, this should be all done

I'll meet with @nqb for the remaining steps

@nqb
Copy link
Contributor

nqb commented Sep 28, 2021

Packages packetfence-upgrade have been uploaded to replace packetfence-export packages.

@julsemaan
Copy link
Collaborator Author

@nqb, I've performed the adjustments based on our discussion, it should be ready for packaging

@nqb
Copy link
Contributor

nqb commented Sep 30, 2021

@satkunas, could you look at the conflicting files (related to frontend) ?

Copy link
Contributor

@nqb nqb left a comment

Choose a reason for hiding this comment

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

First review, without paying so much attention to run-upgrade.sh. I will review it more precisely during my second.

  • IMHO, we should avoid duplicating documentation related to fields between documentation.conf, .vue files and AsciiDoc documentation

conf/iptables.conf.example Outdated Show resolved Hide resolved
conf/documentation.conf Outdated Show resolved Hide resolved
debian/packetfence.postinst Outdated Show resolved Hide resolved
debian/packetfence.postinst Outdated Show resolved Hide resolved
conf/documentation.conf Outdated Show resolved Hide resolved
rpm/packetfence.spec Outdated Show resolved Hide resolved
elif is_deb_based; then
OS="Debian-11"
fi
curl https://www.packetfence.org/downloads/PacketFence/latest-stable-$OS.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

If suggest to do a GitHub API call to get latest stable based on stable tag.
We decided to not use it anymore but it could be interested to use it for that purpose.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The thing is the stable tag will differ for RHEL8 and RHEL9 when we get there so we'd need a stable-rhel8 stable-debian11, etc

By putting it in a file here, we're able to stop updating the stable release when we switch OS

Copy link
Contributor

@nqb nqb Oct 5, 2021

Choose a reason for hiding this comment

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

I understand your idea and your approach. I will see if I can automate update of these two files when we will release.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would indeed be the goal, the release pipeline should be performing this, not one of us manually

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be ready now. I will run a pipeline and see if it works. Meanwhile, could you add support to pull latest-devel-$OS-txt when we are testing on devel or on development branches ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Pipeline triggered with BUILD_ARTIFACTS_WEBSITE=yes:
https://gitlab.com/inverse-inc/packetfence/-/pipelines/382749074

It should create latest-devel-$OS.txt files.

@nqb
Copy link
Contributor

nqb commented Oct 1, 2021

@julsemaan, as discussed together, please add following features:

  • take a backup and export before running upgrade
  • stop services before running upgrade
    Thanks!

@nqb
Copy link
Contributor

nqb commented Oct 1, 2021

@julsemaan, another thing that came to my mind and which is related to #6468, is it possible to perform upgrade in a non-interactive way ?

I see that you use some environment variables to avoid asking questions but I didn't test completely.

@julsemaan
Copy link
Collaborator Author

@julsemaan, another thing that came to my mind and which is related to #6468, is it possible to perform upgrade in a non-interactive way ?

I see that you use some environment variables to avoid asking questions but I didn't test completely.

It is possible to do this indeed

- for EL: packetfence-release pkg
- for Debian : packetfence-archive-keyring pkg

packetfence main package is now dependent to each of this package.
having gpg or gnupg2 is necessary to install packetfence too
@nqb nqb force-pushed the feature/automate-upgrades branch from b47d282 to bb4e5e9 Compare October 5, 2021 09:03
@julsemaan
Copy link
Collaborator Author

@fdurand, can you take a look at this to make sure it makes sense and we'll be ready to merge, we'll do the doc post-merge

@nqb
Copy link
Contributor

nqb commented Oct 5, 2021

fixpermissions and gpg stuff work as expected. Tested on EL8 and Debian 11.

Waiting end of https://gitlab.com/inverse-inc/packetfence/-/pipelines/382848646 to confirm rsync command is working properly.

Copy link
Member

@fdurand fdurand left a comment

Choose a reason for hiding this comment

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

It miss generatemonitconfig in lib/pf/cmd/pf.pm

addons/monit/monit_checks_configurations/00_packetfence.tt Outdated Show resolved Hide resolved
@julsemaan
Copy link
Collaborator Author

It miss generatemonitconfig in lib/pf/cmd/pf.pm

Its in its own file actually: https://github.com/inverse-inc/packetfence/blob/bcaf91b9c9947ff149ee82b7ca204a6508665f72/lib/pf/cmd/pf/generatemonitconfig.pm

@fdurand
Copy link
Member

fdurand commented Oct 5, 2021

It miss generatemonitconfig in lib/pf/cmd/pf.pm

Its in its own file actually: https://github.com/inverse-inc/packetfence/blob/bcaf91b9c9947ff149ee82b7ca204a6508665f72/lib/pf/cmd/pf/generatemonitconfig.pm

it´s just to have the output when you do a bin/pfcmd

@nqb
Copy link
Contributor

nqb commented Oct 6, 2021

Ready to merge on my side. I will let @fdurand and @julsemaan provide their inputs before merging.

Copy link
Member

@fdurand fdurand left a comment

Choose a reason for hiding this comment

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

It miss the symbolic link for packetfence.monit-drop-in.service

@julsemaan
Copy link
Collaborator Author

It miss the symbolic link for packetfence.monit-drop-in.service

Not sure which symlink you're talking about

@fdurand
Copy link
Member

fdurand commented Oct 6, 2021

It miss the symbolic link for packetfence.monit-drop-in.service

Not sure which symlink you're talking about

you need to have a symbolic link between the file and the debian directory

It miss the symbolic link for packetfence.monit-drop-in.service

Not sure which symlink you're talking about

My bad it´s useless since you are not using dh_installinit

@fdurand
Copy link
Member

fdurand commented Oct 6, 2021

it´s ok on my side to merge

@nqb nqb merged commit 2669a06 into devel Oct 6, 2021
nqb added a commit that referenced this pull request Oct 6, 2021
nqb added a commit that referenced this pull request Oct 7, 2021
Related to #6583
Necessary for Perl unit tests.
nqb added a commit that referenced this pull request Oct 18, 2021
nqb referenced this pull request Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants