Fix detection of KRA installation so upgrades can succeed #1517
Conversation
|
I recently noticed this failure, too. When did it start to happen? |
|
Hi @rcritten |
|
Hm, must have been there from the start of the check given that the configuration of the service is decided on sstore content for the +1 for the test edit: the |
|
Implemented Flo's suggestions. |
There was a problem hiding this comment.
Hi @rcritten
thank you for the new PR. The commit properly fixes the issue but I have an inline comment for the test.
|
|
||
| def test_invoke_upgrader(self): | ||
| # For a new version in to require the upgrade to run | ||
| sysupgrade.set_upgrade_state('ipa', 'data_version', |
There was a problem hiding this comment.
Unless I'm mistaken, this command would be run on the controller from where the tests are launched (which can be different from the master) and will trigger an error.
Moreover, I don't think you need to tweak the data_version for ipa-server-upgrade to run, the upgrade can be executed even without changing the version.
There was a problem hiding this comment.
Right, of course. My purpose was to be absolutely sure that the upgrade was successful. I can remove this and revisit in the future if need be.
| @@ -1710,7 +1710,7 @@ def upgrade_configuration(): | |||
| ) | |||
| upgrade_pki(ca, fstore) | |||
|
|
|||
| if kra.is_configured(): | |||
| if kra.is_installed(): | |||
There was a problem hiding this comment.
18 lines below there's a very similar check, you should probably change that one, too.
|
|
||
| from ipatests.test_integration.base import IntegrationTest | ||
| from ipatests.pytest_plugins.integration import tasks | ||
| from ipaserver.install import sysupgrade |
There was a problem hiding this comment.
Sigh, it's what I get for rushing. Fixed.
|
Hi @rcritten I'm just waiting for the PR-CI success to give the ACK. |
|
PR CI build fails with |
|
Probably PR needs a rebase |
Use is_installed() instead of is_configured() because is_installed() does a config file check to see if the service is in use. https://pagure.io/freeipa/issue/7389 Signed-off-by: Rob Crittenden <rcritten@redhat.com>
|
Now that the tests are green, ACK |
|
master:
|
To determine whether KRA needs to be updated the standard
service.is_configured() was used. Since the KRA and CA share
the same service name (pki-tomcatd) this wasn't being detected
properly.
Add an additional check to see if the KRA CS.cfg is present.
https://pagure.io/freeipa/issue/7389
Signed-off-by: Rob Crittenden rcritten@redhat.com