search

StackStorm / st2-rbac-backend

RBAC backend for StackStorm (previously part of EWC aka StackStorm Enteprise)
https://docs.stackstorm.com/latest/rbac.html
Apache License 2.0
5 stars 12 forks source link

Fix postun for RPM packages #18

Closed Kami closed 5 years ago

Kami commented 5 years ago

This pull request fixes an issue with RPM package upgrade removing files installed by a new package version on upgrade.

The reason for that is how RPM post and pre script ordering works. For upgrades is as follows:

This means that postun from old package would remove files installed by a new package.

TODO

Kami commented 5 years ago

NOTE 1: There is still an issue when upgrading from old versions (aka when existing system already has old version of "broken" st2-rbac-backend package installed - https://www.golinuxhub.com/2018/05/how-to-execute-script-at-pre-post-preun-postun-spec-file-rpm.html).

I don't know how to approach that. Because of how post and pre scriptlet ordering works on upgrade postun of old packages runs last so there is nothing we can do.

One option we can do is to document the issue and document the workaround. Also since this only affects enterprise version, where should that documentation go (probably not forum).

Based on how scriplet ordering works for deb packages, I believe it only affects RPM and not Debian installations - https://wiki.debian.org/MaintainerScripts.

This only affects people upgrading from v3.0.0 (where broken st2-rbac-backend package was present) to v3.0.1

We can fix it with this code change for people upgrading from pre v3.0.0 to v3.0.1 (just need to merge this and push new v3.0.1 package to enterprise stable).

Kami commented 5 years ago

I confirmed it only affects rpm, but not debian packages.

I tested it on two fresh Ubuntu 16.04 installations. One running v2.9.1 and one running v3.0.0. Upgrade to v3.0.1 worked correctly in both scenarios.

Kami commented 5 years ago

Looks like we may be able to add a workaround to new package by utilizing %postrans of a new package - https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#ordering.

I think this workaround should work - 2075b95318ef2f3fbebd92733cb93e4f6f55c1d4.

It's not ideal because it means "uninstall" work work correctly (package will also be re-installed on uninstall), but I think that's not a big issue. We don't really support full StackStorm installation anyway.

Kami commented 5 years ago

After we confirm this issue and workaround we should also check "postun" steps of other RPM packages to make sure we don't have the same issue elsewhere.

Kami commented 5 years ago

So I tested this change and it doesn't work yet. It needs some more changes.

It seems like the only approach which fixes it is also running /opt/stackstorm/st2/bin/pip install --find-links /opt/stackstorm/share/wheels/ --no-index --upgrade --force-reinstall st2-enterprise-rbac-backend if st2-apply-rbac-definitions binary is not available.

It looks looks like that in some scenarios Python package is present, but binary is not. I believe acedc0bcc325b1352a24ba55b94862582164dcc2 should fix it (testing it as I type this).

Kami commented 5 years ago

That's the latest change which I confirmed it works and covers all the scenario - 07d4dfb7c8025077b7a02a176c0d7dd57b54412e. Only downside is that for some reason reinstall step takes quite a while when it runs as part of posttrans step (if you run it manually it completes immediately).

I tested fresh v3.0.1 install and also upgrading from v2.9.1 and v3.0.0 to v3.0.1 (CentOS 7).

I will do some more testing and if everything looks OK, I will promote latest package to enterprise stable.

arm4b commented 5 years ago

:+1: