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

Regression: Since commit 597bed6d22b93ed0754f3f02b632ebbcf8206596 some dependencies are not updated before reactive handlers are called #166

Closed
thedac opened this issue Apr 28, 2020 · 0 comments · Fixed by #167

Comments

@thedac
Copy link
Contributor

thedac commented Apr 28, 2020

A regression has occurred in layer-basic, such that during a charm upgrade wheelhouse dependencies are not updated before they are called on by reactive handlers. Leading to hook errors similar to what is described in LP Bug#1875496

Steps to recreate the failure (note designate is used as an example but all reactive charms are affected):

charm pull cs:designate-38
cd designate
juju deploy ./tests/bundles/bionic_train.yaml
(wait for cloud to settle)
juju upgrade-charm --switch cs:designate-41

Per fnordahl's hint I built a new version of designate (with the charms.openstack changes) with layer-basic one commit before 597bed6 and it upgrades without error.

That seems like pretty definitive proof this is a layer-basic bug.

For future tests:
From cs:designate-38 to cs:designate-41 shows the failure.

cs:~thedac/designate-0 has layer-basic @ commit 0c8d7e2 and everything else up to date.

From cs:designate-38 to cs:~thedac/designate-0 successfully upgrades.

Logs:
Failed upgrade-charm hook upgrading to cs:designate-41 [1]
Failed upgrade-charm hook in debug-hooks session upgrading to cs:designate-41 [2]
Successful upgrade-charm hook upgrading to cs:~thedac/designate-0 [3]

Potential solution:
--quote---
cory_fu: My suspicion is that it's because the version of the library doesn't change (0.0.1-dev1) and when we were previously using filenames rather than package names, it forced it to reinstall but when we use the package name it looks at the file and thinks it's the same version and doesn't update even though we're using --ignore-installed. I think instead of --ignore-installed, we want to use --force-reinstall.
thedac: Yes, I would agree we want --force-reinstall
--end quote--

[0] https://bugs.launchpad.net/charm-aodh/+bug/1875496
[1] https://paste.ubuntu.com/p/8nHyn68tTV/
[2] http://paste.ubuntu.com/p/kX2qWTdzRR/
[3] https://pastebin.canonical.com/p/8fYYD8p4YY/

johnsca added a commit that referenced this issue Apr 28, 2020
Prior to #160, we were passing filenames to pip for the wheelhouse
install rather than package names. It seems that using filenames
implicitly disables pip's caching of package archive files. After #160,
since we are now using package names rather than file names (which does
allow pip to handle dependency resolution better), the packages were
getting copied out of the wheelhouse directory and into pip's cache dir
on the initial install and not updated unless the filename changed
(i.e., the version number changed).  Some of the OpenStack charms use a
shared library directly from GitHub which doesn't bother tracking the
library version, causing it to always have the same version.

Adding `--no-cache-dir` forces pip to always use the copy from the
wheelhouse, even if it seems like it hasn't been changed, based on the
file name.

This also switches from `--ignore-installed` to `--force-reinstall`
because the latter is cleaner since the former can leave artifacts from
previous package versions in the venv if they are removed from the newer
pacakge version.

Fixes #166
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 a pull request may close this issue.

1 participant