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

build(snap): update edgexfoundry snap base to core20 #3848

Merged
merged 9 commits into from
Jan 28, 2022
Merged

build(snap): update edgexfoundry snap base to core20 #3848

merged 9 commits into from
Jan 28, 2022

Conversation

siggiskulason
Copy link

@siggiskulason siggiskulason commented Dec 17, 2021

This commit includes a number of updates to migrate the base to core20.

  • postgres: updating postgres to 12.9 from 10.19. This update includes changes to the
    post-refresh hook to handle the three cases of a) new install, b) refresh from
    a postgres-10 version and c) refresh from a postgres-12 version

  • snapcraft-preload: updates to use latest version and fixed cmake setup

  • redis: make-install-var replaced with make-parameters

  • kong-daemon: setpriv package replaced with util-linux

Signed-off-by: Siggi Skulason siggi.skulason@canonical.com

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

  • I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
  • I am not introducing a new dependency (add notes below if you are)
  • I have added unit tests for the new feature or bug fix (if not, why?) See Add test for postgres refresh from v10 to v12 canonical/edgex-checkbox-provider#21
  • I have fully tested (add details below) this the new feature or bug fix (if not, why?)
  • I have opened a PR for the related docs change (if not, why?) N/A

Testing Instructions

Test by building the edgexfoundry snap and running a) checkbox tests and b) TAF tests to confirm that there are no regressions.

Test the refresh by doing
1: new install

sudo snap remove --purge edgexfoundry
sudo snap install ./edgexfoundry_2.2.0-dev.13_amd64.snap --dangerous

2: refresh from Ireland

sudo snap remove --purge edgexfoundry
sudo snap install edgexfoundry --channel=2.0/stable
sudo snap install ./edgexfoundry_2.2.0-dev.13_amd64.snap --dangerous

Expected result: This will fail, as the epoch has been updated to 6. We will provide a later build with a 6* epoch, which will contain the pre-refresh hook code required to upgrade.

3: refresh from current build

sudo snap remove --purge edgexfoundry
sudo snap install ./edgexfoundry_2.2.0-dev.13_amd64.snap --dangerous
sudo snap install ./edgexfoundry_2.2.0-dev.13_amd64.snap --dangerous

New Dependency Instructions (If applicable)

@siggiskulason
Copy link
Author

siggiskulason commented Dec 17, 2021

This version passes:

  • all TAF functional tests
  • all TAF integration tests
  • checkbox tests

@siggiskulason siggiskulason marked this pull request as draft December 17, 2021 17:48
@siggiskulason siggiskulason marked this pull request as ready for review December 20, 2021 16:46
snap/snapcraft.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@anonymouse64 anonymouse64 left a comment

Choose a reason for hiding this comment

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

seems reasonable to me for the most part, but I do wonder if it's okay for us to just delete Postgres 10 database on refresh and create a fresh version 12 database...

I have not tested the changes locally

snap/local/runtime-helpers/bin/install-setup-postgres.sh Outdated Show resolved Hide resolved
snap/local/runtime-helpers/bin/install-setup-postgres.sh Outdated Show resolved Hide resolved
@farshidtz farshidtz marked this pull request as draft January 7, 2022 12:48
snap/snapcraft.yaml Outdated Show resolved Hide resolved
snap/hooks/post-refresh Outdated Show resolved Hide resolved
@farshidtz
Copy link
Member

We'll need to increment the epoch from 5 to 6 in this PR to prevent upgrades from existing deployments that have Postgres 10 installed but no way to backup the data in preperation for the Postgres 12 restore. This is to avoid data loss resulted from replacing Postgres 10 with 12; see https://www.postgresql.org/docs/12/upgrading.html

In future, another snap would be created with epoch 6* to act as a bridge, based on the code before this PR, but including only the backup feature. That snap would be released to the latest track to provide a path for 2.0|2.1|2.2 to 2.2.x upgrades. Installations from the LTS 2.1 track will not be affected by this release.

For more details on how the epoch works: https://snapcraft.io/docs/snap-epochs

Siggi Skulason added 7 commits January 25, 2022 13:52
This commit includes a number of updates to migrate the base to core20.

- postgres: updating postgres to 12.9 from 10.19. This update includes changes to the
  post-refresh hook to handle the three cases of a) new install, b) refresh from
  a postgres-10 version and c) refresh from a postgres-12 version

- snapcraft-preload: updates to use latest version and fixed cmake setup

- redis: make-install-var replaced with make-parameters

- kong-daemon: setpriv package replaced with util-linux

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
This commit adds code in the post- and pre- refresh hooks  to dump
the postgres database before a refresh and then read the data back in
after upgrading postres from version 10 to version 12

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
- remove some of the passthrough statements in snapcraft.yaml
- move the snapcraft-runner and snapcraft-preload logic to the postgres utility script

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
- update log messages
- run kong-post-gres-setup.sh with selector argument to pick the function to run

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
- remove sql file in post_refresh

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
- resolve issue with --devmode install not working

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
- Set epoch to 6 for Kamakura.

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
- Update comments
- Remove use of snapcraft-runner in command chain for connect-plug-edgex-secretstore-token

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
@siggiskulason siggiskulason marked this pull request as ready for review January 25, 2022 13:56
Copy link
Member

@farshidtz farshidtz left a comment

Choose a reason for hiding this comment

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

Thanks a lot @siggiskulason for the updates. It looks very good overall.

I've added a few suggestions inline.

snap/local/runtime-helpers/bin/kong-postgres-setup.sh Outdated Show resolved Hide resolved
snap/local/runtime-helpers/bin/kong-postgres-setup.sh Outdated Show resolved Hide resolved
snap/snapcraft.yaml Show resolved Hide resolved
snap/snapcraft.yaml Outdated Show resolved Hide resolved
snap/local/runtime-helpers/bin/kong-postgres-setup.sh Outdated Show resolved Hide resolved
snap/local/runtime-helpers/bin/kong-postgres-setup.sh Outdated Show resolved Hide resolved
- updated comments and constant names, fixed typos and other minor modifications from PR review

Signed-off-by: Siggi Skulason <siggi.skulason@canonical.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@siggiskulason
Copy link
Author

siggiskulason commented Jan 27, 2022

Thanks, @farshidtz for the review, I've pushed an update with the requested changes.

Also, for testing purposes, I've pushed this snap to the latest/edge/core-refresh branch, so it can be tested with

sudo snap install checkbox-edgexfoundry --devmode --edge
sudo DEFAULT_TEST_CHANNEL="latest/edge/core-refresh" checkbox-edgexfoundry.latest

and the checkbox test for this PR can be run using this channel

@farshidtz farshidtz merged commit 2940d6a into edgexfoundry:main Jan 28, 2022
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