Hi all,

Finally the tests pass.

The patch 0024 applies on top of patch 0022 (please, consider reviewing
it also). Besides, the whole functionality depends on Martin's patch N 0421

All patches pass pylint.


On 12/19/2015 11:56 PM, Martin Basti wrote:
> 
> 
> On 17.12.2015 10:04, Oleg Fayans wrote:
>> Hi Martin,
>>
>> I am sorry, in my previous email I attached the old version of patch
>> 0016. The correct on is attached.
>>
>> On 12/16/2015 05:47 PM, Martin Basti wrote:
>>>
>>> On 16.12.2015 15:39, Martin Basti wrote:
>>>>
>>>> On 15.12.2015 10:29, Oleg Fayans wrote:
>>>>> Hi Martin,
>>>>>
>>>>> The updated patches are attached. Patch 0017 includes all changes from
>>>>> patch 0018, so, if you approve this one, there would be no need to
>>>>> continue with the review of 0018. This one contains all changes
>>>>> related
>>>>> to you remarks from 0018 review. Please see my explanation on the
>>>>> stdout+stderr part in the thread from patch 0018.
>>>>> With these two patches applied one of the tests fails due this bug:
>>>>> https://fedorahosted.org/freeipa/ticket/5550
>>>>>
>>>>> On 12/09/2015 12:17 PM, Martin Basti wrote:
>>>>>> On 09.12.2015 12:10, Martin Basti wrote:
>>>>>>> On 09.12.2015 11:14, Oleg Fayans wrote:
>>>>>>>> Hi Martin
>>>>>>>>
>>>>>>>> On 12/09/2015 10:30 AM, Martin Basti wrote:
>>>>>>>>> On 08.12.2015 23:48, Oleg Fayans wrote:
>>>>>>>>>> Substituted a hardcoded suffix name with a constant
>>>>>>>>>> DOMAIN_SUFFIX_NAME
>>>>>>>>>>
>>>>>>>>>> On 12/08/2015 02:33 PM, Oleg Fayans wrote:
>>>>>>>>>>> Hi all,
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> The patches are rebased against the current master.
>>>>>>>>>>>
>>>>>>>>>>> On 12/02/2015 05:10 PM, Martin Basti wrote:
>>>>>>>>>>>> On 02.12.2015 16:18, Oleg Fayans wrote:
>>>>>>>>>>>>> Hi Martin,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 12/01/2015 04:08 PM, Martin Basti wrote:
>>>>>>>>>>>>>> On 27.11.2015 16:26, Oleg Fayans wrote:
>>>>>>>>>>>>>>> And patch N 16 passes lint too:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 11/27/2015 04:03 PM, Oleg Fayans wrote:
>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 11/27/2015 03:26 PM, Martin Basti wrote:
>>>>>>>>>>>>>>>>> On 27.11.2015 15:04, Oleg Fayans wrote:
>>>>>>>>>>>>>>>>>> Hi Martin,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> All your suggestions were taken into account. Both
>>>>>>>>>>>>>>>>>> patches are
>>>>>>>>>>>>>>>>>> updated. Thank you for your help!
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 11/26/2015 10:50 AM, Martin Basti wrote:
>>>>>>>>>>>>>>>>>>> On 26.11.2015 10:04, Oleg Fayans wrote:
>>>>>>>>>>>>>>>>>>>> Hi Martin,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> I agree to all your points but one. please, see my
>>>>>>>>>>>>>>>>>>>> comment
>>>>>>>>>>>>>>>>>>>> below
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On 11/25/2015 07:42 PM, Martin Basti wrote:
>>>>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 0) Note
>>>>>>>>>>>>>>>>>>>>> Please be aware of
>>>>>>>>>>>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5469
>>>>>>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>>>>>>> KRA testing
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 1)
>>>>>>>>>>>>>>>>>>>>> Please do not use MIN and MAX_DOMAIN_LEVEL constants,
>>>>>>>>>>>>>>>>>>>>> this may
>>>>>>>>>>>>>>>>>>>>> change
>>>>>>>>>>>>>>>>>>>>> over time, use DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 for
>>>>>>>>>>>>>>>>>>>>> domain
>>>>>>>>>>>>>>>>>>>>> level 0
>>>>>>>>>>>>>>>>>>>>> and 1
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 2)
>>>>>>>>>>>>>>>>>>>>> Why uninstall KRA then server, is not enough just
>>>>>>>>>>>>>>>>>>>>> uninstall
>>>>>>>>>>>>>>>>>>>>> server
>>>>>>>>>>>>>>>>>>>>> which
>>>>>>>>>>>>>>>>>>>>> covers KRA uninstall?
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> +    def teardown_method(self, method):
>>>>>>>>>>>>>>>>>>>>> +        for host in self.replicas:
>>>>>>>>>>>>>>>>>>>>> + host.run_command(self.kra_uninstall,
>>>>>>>>>>>>>>>>>>>>> raiseonerr=False)
>>>>>>>>>>>>>>>>>>>>> + tasks.uninstall_master(host)
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 3)
>>>>>>>>>>>>>>>>>>>>> Can be this function more generic? It should allow
>>>>>>>>>>>>>>>>>>>>> specify
>>>>>>>>>>>>>>>>>>>>> host
>>>>>>>>>>>>>>>>>>>>> where
>>>>>>>>>>>>>>>>>>>>> KRA should be installed not just master
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> +    def test_kra_install_master(self):
>>>>>>>>>>>>>>>>>>>>> + self.master.run_command(self.kra_install)
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 4)
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> TestLevel0(Dummy):
>>>>>>>>>>>>>>>>>>>>> Can be the test name more specific, something like
>>>>>>>>>>>>>>>>>>>>> TestReplicaPromotionLevel0
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 5)
>>>>>>>>>>>>>>>>>>>>> please remove this, the patch is on review and it
>>>>>>>>>>>>>>>>>>>>> will be
>>>>>>>>>>>>>>>>>>>>> pushed
>>>>>>>>>>>>>>>>>>>>> sooner
>>>>>>>>>>>>>>>>>>>>> than tests
>>>>>>>>>>>>>>>>>>>>> +    @pytest.mark.xfail  # Ticket N 5455
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> and as I mentioned in ticket #5455, I cannot reproduce
>>>>>>>>>>>>>>>>>>>>> it with
>>>>>>>>>>>>>>>>>>>>> ipa-kra-install, so please provide steps to
>>>>>>>>>>>>>>>>>>>>> reproduce if
>>>>>>>>>>>>>>>>>>>>> you
>>>>>>>>>>>>>>>>>>>>> insist
>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>> this still does not work as expected with KRA.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> 6) This is completely wrong, it removes everything
>>>>>>>>>>>>>>>>>>>>> that we
>>>>>>>>>>>>>>>>>>>>> tried to
>>>>>>>>>>>>>>>>>>>>> achieve with previous patches with domain level in CI
>>>>>>>>>>>>>>>>>>>> Actually, being able to configure domain level per
>>>>>>>>>>>>>>>>>>>> class
>>>>>>>>>>>>>>>>>>>> is WAY
>>>>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>>>>> convenient, than to always have to think which domain
>>>>>>>>>>>>>>>>>>>> level is
>>>>>>>>>>>>>>>>>>>> appropriate for which particular test during jenkins
>>>>>>>>>>>>>>>>>>>> job
>>>>>>>>>>>>>>>>>>>> configuration. In fact, I should have thought about it
>>>>>>>>>>>>>>>>>>>> from the
>>>>>>>>>>>>>>>>>>>> very
>>>>>>>>>>>>>>>>>>>> beginning. For example, in test_replica_promotion.py we
>>>>>>>>>>>>>>>>>>>> have on
>>>>>>>>>>>>>>>>>>>> class,
>>>>>>>>>>>>>>>>>>>> which intiates with domain level = 1, while others -
>>>>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>>>>> domain
>>>>>>>>>>>>>>>>>>>> level
>>>>>>>>>>>>>>>>>>>> 0. With config-based approach, we would have to
>>>>>>>>>>>>>>>>>>>> implement a
>>>>>>>>>>>>>>>>>>>> separate
>>>>>>>>>>>>>>>>>>>> step that raises domain level. Overall, I am against
>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>> approach,
>>>>>>>>>>>>>>>>>>>> when you have to remember to set certain domain
>>>>>>>>>>>>>>>>>>>> level in
>>>>>>>>>>>>>>>>>>>> config
>>>>>>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>>>>> any particular test. The tests themselves should be
>>>>>>>>>>>>>>>>>>>> aware of
>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>> domain level they need.
>>>>>>>>>>>>>>>>>>> I do not say that we should not have something that
>>>>>>>>>>>>>>>>>>> overrides
>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>> in from config in a particular test case, I say your
>>>>>>>>>>>>>>>>>>> patch is
>>>>>>>>>>>>>>>>>>> doing it
>>>>>>>>>>>>>>>>>>> wrong.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I agree it is useful to have param domain_level in
>>>>>>>>>>>>>>>>>>> install_master,
>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>> intall_topo methods,  but is cannot be
>>>>>>>>>>>>>>>>>>> MAX_DOMAIN_LEVEL by
>>>>>>>>>>>>>>>>>>> default,
>>>>>>>>>>>>>>>>>>> because with your current patch the domain_level in
>>>>>>>>>>>>>>>>>>> config is
>>>>>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>>>> used
>>>>>>>>>>>>>>>>>>> at all, it will be always MAX_DOMAIN_LEVEL
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> For example I want to achieve this goal:
>>>>>>>>>>>>>>>>>>> test_vault.py, this test suite can run on domain level1
>>>>>>>>>>>>>>>>>>> and on
>>>>>>>>>>>>>>>>>>> domain
>>>>>>>>>>>>>>>>>>> level0, so with one test we can test 2 domain levels
>>>>>>>>>>>>>>>>>>> just
>>>>>>>>>>>>>>>>>>> with
>>>>>>>>>>>>>>>>>>> putting
>>>>>>>>>>>>>>>>>>> domain level into config file.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I agree that with extraordinary test like replica
>>>>>>>>>>>>>>>>>>> promotion test
>>>>>>>>>>>>>>>>>>> is, we
>>>>>>>>>>>>>>>>>>> need something that allows override the config file.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> As I said bellow, domain_level default value should be
>>>>>>>>>>>>>>>>>>> None in
>>>>>>>>>>>>>>>>>>> install_master and install_topo plugin. If domain level
>>>>>>>>>>>>>>>>>>> was
>>>>>>>>>>>>>>>>>>> specified
>>>>>>>>>>>>>>>>>>> use the specified one, if not (value is None) use the
>>>>>>>>>>>>>>>>>>> domain
>>>>>>>>>>>>>>>>>>> level
>>>>>>>>>>>>>>>>>>> from
>>>>>>>>>>>>>>>>>>> config file.
>>>>>>>>>>>>>>>>>> Agreed :)
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Martin
>>>>>>>>>>>>>>>>>>>>> [PATCH] Enabled setting domain_level per class
>>>>>>>>>>>>>>>>>>>>> derived from
>>>>>>>>>>>>>>>>>>>>> TestIntegration
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> When I configure domain level 0 in yaml config, how
>>>>>>>>>>>>>>>>>>>>> is this
>>>>>>>>>>>>>>>>>>>>> supposed to
>>>>>>>>>>>>>>>>>>>>> get into install methods when you removed that code?
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> -        "--domain-level=%i" %
>>>>>>>>>>>>>>>>>>>>> host.config.domain_level
>>>>>>>>>>>>>>>>>>>>> +        "--domain-level=%i" % domain_level
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> You always use MAX_DOMAIN_LEVEL in this case or
>>>>>>>>>>>>>>>>>>>>> whatever is
>>>>>>>>>>>>>>>>>>>>> specified in
>>>>>>>>>>>>>>>>>>>>> domain_level option.
>>>>>>>>>>>>>>>>>>>>> I suggest to use domain_level=None, and when it is
>>>>>>>>>>>>>>>>>>>>> None use
>>>>>>>>>>>>>>>>>>>>> 'host.config.domain_level', if it is not None, use
>>>>>>>>>>>>>>>>>>>>> 'domain_level'
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> With this we can specify domain level in config
>>>>>>>>>>>>>>>>>>>>> file for
>>>>>>>>>>>>>>>>>>>>> test
>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>> can
>>>>>>>>>>>>>>>>>>>>> be used for both domain levels and you can manually
>>>>>>>>>>>>>>>>>>>>> specify
>>>>>>>>>>>>>>>>>>>>> domain
>>>>>>>>>>>>>>>>>>>>> level
>>>>>>>>>>>>>>>>>>>>> for test that requires specific domain level.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Also this should go away
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>           @classmethod
>>>>>>>>>>>>>>>>>>>>>           def install(cls, mh):
>>>>>>>>>>>>>>>>>>>>> +        if hasattr(cls, "domain_level") and
>>>>>>>>>>>>>>>>>>>>> cls.master:
>>>>>>>>>>>>>>>>>>>>> + cls.master.config.domain_level = cls.domain_level
>>>>>>>>>>>>>>>>>>>>>               if cls.topology is None:
>>>>>>>>>>>>>>>>>>>>>                   return
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> I do not see reason why test should override
>>>>>>>>>>>>>>>>>>>>> configuration in
>>>>>>>>>>>>>>>>>>>>> config in
>>>>>>>>>>>>>>>>>>>>> this case.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Martin
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On 25.11.2015 16:44, Oleg Fayans wrote:
>>>>>>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Here is the updated version of the patch (more
>>>>>>>>>>>>>>>>>>>>>> tests +
>>>>>>>>>>>>>>>>>>>>>> fixed the
>>>>>>>>>>>>>>>>>>>>>> issues of the first one) + patch 0017, that
>>>>>>>>>>>>>>>>>>>>>> implements the
>>>>>>>>>>>>>>>>>>>>>> necessary
>>>>>>>>>>>>>>>>>>>>>> changes in the background code, i. e. patch 16
>>>>>>>>>>>>>>>>>>>>>> does not
>>>>>>>>>>>>>>>>>>>>>> work
>>>>>>>>>>>>>>>>>>>>>> without
>>>>>>>>>>>>>>>>>>>>>> patch 17
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> On 11/18/2015 05:20 PM, Martin Basti wrote:
>>>>>>>>>>>>>>>>>>>>>>> On 09.11.2015 15:09, Oleg Fayans wrote:
>>>>>>>>>>>>>>>>>>>>>>>> Hi guys,
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Here are first two automated testcases from this
>>>>>>>>>>>>>>>>>>>>>>>> (so far
>>>>>>>>>>>>>>>>>>>>>>>> incomplete)
>>>>>>>>>>>>>>>>>>>>>>>> testplan:
>>>>>>>>>>>>>>>>>>>>>>>> http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Testplan review is highly appreciated
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> PATCH 16: NACK
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> 1)
>>>>>>>>>>>>>>>>>>>>>>> What is the reason to add an unused parameter to
>>>>>>>>>>>>>>>>>>>>>>> 'domain_level' to
>>>>>>>>>>>>>>>>>>>>>>> install_topo()?
>>>>>>>>>>>>>>>>>>>>>>> Also it is good practise to add new option as the
>>>>>>>>>>>>>>>>>>>>>>> last
>>>>>>>>>>>>>>>>>>>>>>> parameter.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> 2)
>>>>>>>>>>>>>>>>>>>>>>> cab you in both tests specify a domain level with
>>>>>>>>>>>>>>>>>>>>>>> constant
>>>>>>>>>>>>>>>>>>>>>>> instead of
>>>>>>>>>>>>>>>>>>>>>>> number literal?
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> 3)
>>>>>>>>>>>>>>>>>>>>>>> both test call install_topo with custom domain
>>>>>>>>>>>>>>>>>>>>>>> level,
>>>>>>>>>>>>>>>>>>>>>>> but it
>>>>>>>>>>>>>>>>>>>>>>> cannot
>>>>>>>>>>>>>>>>>>>>>>> work
>>>>>>>>>>>>>>>>>>>>>>> because 1)  (did you run the test?)
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> 4)
>>>>>>>>>>>>>>>>>>>>>>> How the test "TestLevel1" is supposed to work?
>>>>>>>>>>>>>>>>>>>>>>> Respectively why there is call of install_topo()
>>>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>>>> installs
>>>>>>>>>>>>>>>>>>>>>>> replica.
>>>>>>>>>>>>>>>>>>>>>>> As this test just tests that ipa-replica-prepare is
>>>>>>>>>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>>>>>>>> working
>>>>>>>>>>>>>>>>>>>>>>> anymore,
>>>>>>>>>>>>>>>>>>>>>>> is it worth to spend 20 minutes with installing
>>>>>>>>>>>>>>>>>>>>>>> replica and
>>>>>>>>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>>>>>>>>> just no
>>>>>>>>>>>>>>>>>>>>>>> tot use it? IMO to install master in install step is
>>>>>>>>>>>>>>>>>>>>>>> enough.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Martin^2
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> ./make-lint
>>>>>>>>>>>>>>>>> ************* Module ipatests.test_integration.base
>>>>>>>>>>>>>>>>> ipatests/test_integration/base.py:66: [E1101(no-member),
>>>>>>>>>>>>>>>>> IntegrationTest.install] Class 'IntegrationTest' has no
>>>>>>>>>>>>>>>>> 'domain_level'
>>>>>>>>>>>>>>>>> member)
>>>>>>>>>>>>>>>>> ************* Module
>>>>>>>>>>>>>>>>> ipatests.test_integration.test_replica_promotion
>>>>>>>>>>>>>>>>> ipatests/test_integration/test_replica_promotion.py:16:
>>>>>>>>>>>>>>>>> [E1101(no-member), Dummy.install] Class 'Dummy' has no
>>>>>>>>>>>>>>>>> 'domain_level'
>>>>>>>>>>>>>>>>> member)
>>>>>>>>>>>>>>>>> ipatests/test_integration/test_replica_promotion.py:117:
>>>>>>>>>>>>>>>>> [E1101(no-member),
>>>>>>>>>>>>>>>>> TestCAInstall.test_ca_install_without_replica_file]
>>>>>>>>>>>>>>>>> Module 'ipatests.test_integration.tasks' has no
>>>>>>>>>>>>>>>>> 'setup_replica'
>>>>>>>>>>>>>>>>> member)
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Is it so hard to run pylint before patch is posted for
>>>>>>>>>>>>>>>>> review?
>>>>>>>>>>>>>>>> Sorry, my bad. Fixed.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 1)
>>>>>>>>>>>>>> Why is this change in the patch?
>>>>>>>>>>>>>> -    # Clean up the test directory
>>>>>>>>>>>>>> -    host.run_command(['rm', '-rvf', host.config.test_dir])
>>>>>>>>>>>>> Otherwise 2 out of 8 tests fail with IOError at line 78 of
>>>>>>>>>>>>> ipatests/test_integration/tasks.py
>>>>>>>>>>>>>
>>>>>>>>>>>>> I do not understand yet how does this happen, but if you
>>>>>>>>>>>>> remove
>>>>>>>>>>>>> ipatests folder once, it then fails to be created again.
>>>>>>>>>>>>>
>>>>>>>>>>>> So this should be in separated patch and investigated properly.
>>>>>>>>>>> Agree. Removed
>>>>>>>>>>>>>> -
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> 2)
>>>>>>>>>>>>>> is enough to have this check only in install_master,
>>>>>>>>>>>>>> install_topo can
>>>>>>>>>>>>>> pass None to install_master
>>>>>>>>>>>>>> +    if domain_level is None:
>>>>>>>>>>>>>> +        domain_level = master.config.domain_level
>>>>>>>>>>>>> Done
>>>>>>>>>>>>>
>>>>>>>>>>>>>> 3)
>>>>>>>>>>>>>> IMO replica-manage del should cleanup hosts entry, so
>>>>>>>>>>>>>> following
>>>>>>>>>>>>>> code
>>>>>>>>>>>>>> should not be needed.
>>>>>>>>>>>>>> +    if cleanhost:
>>>>>>>>>>>>>> +        kinit_admin(master)
>>>>>>>>>>>>>> +        master.run_command(["ipa", "host-del",
>>>>>>>>>>>>>> "--updatedns",
>>>>>>>>>>>>>> replica.hostname],
>>>>>>>>>>>>>> +                           raiseonerr=False)
>>>>>>>>>>>>> Well, in fact it does not. At least the corresponding dns
>>>>>>>>>>>>> record
>>>>>>>>>>>>> stays
>>>>>>>>>>>>> and causes the subsequent ipa-client-install to fail. Probably
>>>>>>>>>>>>> it's a
>>>>>>>>>>>>> bug. On the other hand, if I promote an existing client to
>>>>>>>>>>>>> replica and
>>>>>>>>>>>>> then delete this replica, then, I probably want the host
>>>>>>>>>>>>> record
>>>>>>>>>>>>> (that
>>>>>>>>>>>>> was created during client-install) to stay in the system. So,
>>>>>>>>>>>>> does not
>>>>>>>>>>>>> look like a bug to me.
>>>>>>>>>>>> No you don't, because replica uninstallation also removes the
>>>>>>>>>>>> client.
>>>>>>>>>>>>
>>>>>>>>>>>> I tried it with ipa42, ipa-replica-manage del removes host
>>>>>>>>>>>> entry,
>>>>>>>>>>>> and
>>>>>>>>>>>> DNS A records, only SSHFP records stay there (I'm not sure
>>>>>>>>>>>> if it
>>>>>>>>>>>> is bug
>>>>>>>>>>>> or feature)
>>>>>>>>>>>>
>>>>>>>>>>>> Also I received this message
>>>>>>>>>>>> """
>>>>>>>>>>>> Failed to cleanup replica1.ipa.test DNS entries: no matching
>>>>>>>>>>>> entry
>>>>>>>>>>>> found
>>>>>>>>>>>> You may need to manually remove them from the tree
>>>>>>>>>>>> """
>>>>>>>>>>>> But, A record has been removed, so this is probably false
>>>>>>>>>>>> positive and
>>>>>>>>>>>> it needs to have a ticket.
>>>>>>>>>>> Agree, that was an issue with my setup.
>>>>>>>>>>> Removed
>>>>>>>>>>>
>>>>>>>>>>>>>> 4)
>>>>>>>>>>>>>> This variable is not used
>>>>>>>>>>>>>> +    kra_uninstall = ["ipa-kra-install", "--uninstall", "-U"]
>>>>>>>>>>>>> Removed
>>>>>>>>>>>>>
>>>>>>>>>>>>>> 5)
>>>>>>>>>>>>>> Why do you need this
>>>>>>>>>>>>>> +    kra_install = ["ipa-kra-install", "-U", "-p",
>>>>>>>>>>>>>> config.dirman_password]
>>>>>>>>>>>>>> when you implemented tasks.install_kra that returns the
>>>>>>>>>>>>>> exactly
>>>>>>>>>>>>>> the same
>>>>>>>>>>>>>> result?
>>>>>>>>>>>>> Right. Removed
>>>>>>>>>>>>>
>>>>>>>>>>>>>> 6)
>>>>>>>>>>>>>> PEP8
>>>>>>>>>>>>>> ./ipatests/test_integration/tasks.py:928:1: E302 expected 2
>>>>>>>>>>>>>> blank
>>>>>>>>>>>>>> lines,
>>>>>>>>>>>>>> found 1
>>>>>>>>>>>>>> ./ipatests/test_integration/tasks.py:934:1: E302 expected 2
>>>>>>>>>>>>>> blank
>>>>>>>>>>>>>> lines,
>>>>>>>>>>>>>> found 1
>>>>>>>>>>>>>> ./ipatests/test_integration/tasks.py:939:1: E302 expected 2
>>>>>>>>>>>>>> blank
>>>>>>>>>>>>>> lines,
>>>>>>>>>>>>>> found 1
>>>>>>>>>>>>>> ./ipatests/test_integration/tasks.py:943:1: E302 expected 2
>>>>>>>>>>>>>> blank
>>>>>>>>>>>>>> lines,
>>>>>>>>>>>>>> found 1
>>>>>>>>>>>>>> ./ipatests/test_integration/tasks.py:950:80: E501 line too
>>>>>>>>>>>>>> long
>>>>>>>>>>>>>> (80 > 79
>>>>>>>>>>>>>> characters)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:29:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (85 > 79 characters)
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:64:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (85 > 79 characters)
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:67:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (88 > 79 characters)
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:93:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (80 > 79 characters)
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:94:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (83 > 79 characters)
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:118:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (81 > 79 characters)
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:128:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (80 > 79 characters)
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:129:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (82 > 79 characters)
>>>>>>>>>>>>>> ./ipatests/test_integration/test_replica_promotion.py:181:80:
>>>>>>>>>>>>>> E501
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> line
>>>>>>>>>>>>>> too long (80 > 79 characters)
>>>>>>>>>>>>> Most of these complaints are unrelated to the current patches.
>>>>>>>>>>>>> It's better to create a separate patch addressing PEP8 errors.
>>>>>>>>>>>>>
>>>>>>>>>>>> I beg for your pardon, those PEP8 errors have been
>>>>>>>>>>>> introduced by
>>>>>>>>>>>> your
>>>>>>>>>>>> patches.
>>>>>>>>>>> Fixed
>>>>>>>>>>>
>>>>>>>>>>>>>> 7)
>>>>>>>>>>>>>> Why this must be stored in instance? IMO to have it stored as
>>>>>>>>>>>>>> local
>>>>>>>>>>>>>> variable is perfect
>>>>>>>>>>>>>> TestKRAInstall, TestCAInstall
>>>>>>>>>>>>>>              self.replica1_filename =
>>>>>>>>>>>>>> tasks.get_replica_filename(replica1)
>>>>>>>>>>>>>>              self.replica2_filename =
>>>>>>>>>>>>>> tasks.get_replica_filename(replica2)
>>>>>>>>>>>>> Agree. Fixed
>>>>>>>>>>>>>
>>>>>>>>> This patch is missing something.
>>>>>>>> I am sorry, I forgot to revert my previous change. The correct
>>>>>>>> patch is
>>>>>>>> attached
>>>>>>>>
>>>>>>> ************* Module
>>>>>>> ipatests.test_integration.test_replica_promotion
>>>>>>> ipatests/test_integration/test_replica_promotion.py:15:
>>>>>>> [E1123(unexpected-keyword-arg), Dummy.install] Unexpected keyword
>>>>>>> argument 'domain_level' in function call)
>>>>>>> ipatests/test_integration/test_replica_promotion.py:15:
>>>>>>> [E1101(no-member), Dummy.install] Class 'Dummy' has no
>>>>>>> 'domain_level'
>>>>>>> member)
>>>>>>> ipatests/test_integration/test_replica_promotion.py:19:
>>>>>>> [E1101(no-member), Dummy.teardown_method] Module
>>>>>>> 'ipatests.test_integration.tasks' has no 'uninstall_replica' member)
>>>>>>> ipatests/test_integration/test_replica_promotion.py:67:
>>>>>>> [E1101(no-member), TestReplicaPromotionLevel0.test_backup_restore]
>>>>>>> Module 'ipatests.test_integration.tasks' has no 'ipa_backup' member)
>>>>>>> ipatests/test_integration/test_replica_promotion.py:72:
>>>>>>> [E1101(no-member), TestReplicaPromotionLevel0.test_backup_restore]
>>>>>>> Module 'ipatests.test_integration.tasks' has no 'ipa_restore'
>>>>>>> member)
>>>>>>> ipatests/test_integration/test_replica_promotion.py:120:
>>>>>>> [E1123(unexpected-keyword-arg), TestCAInstall.install] Unexpected
>>>>>>> keyword argument 'domain_level' in function call)
>>>>>>>
>>>>>> Sorry I forgot to apply patch 17, my bad, I'm continuing with review
>>>> LGTM, I haven't had time to test it, but if you are sure that test is
>>>> working, we may push this.
>>>>
>>> Is this expected due the bug you mentioned?
>>> _____
>>> __________________________________________________________________________
>>>
>>> TestReplicaPromotionLevel0.test_kra_install_master
>>> ________________________________________________________________________________
>>>
>>>
>>>
>>> self =
>>> <ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0
>>>
>>> object at 0x7f5071a59e50>
>>>
>>>      def test_kra_install_master(self):
>>>          result1 = tasks.install_kra(self.master, raiseonerr=False)
>>>>        assert result1.returncode == 0, result1.stderr_text
>>> E       AssertionError: Usage: ipa-kra-install [options] [replica_file]
>>> E
>>> E         ipa-kra-install: error: Replica file
>>> /root/ipatests/replica-info.gpg does not exist
>>> E         The ipa-kra-install command failed. See
>>> /var/log/ipaserver-kra-install.log for more information
>>> E
>>> E       assert 2 == 0
>>> E        +  where 2 = <pytest_multihost.transport.SSHCommand object at
>>> 0x7f5071adbd50>.returncode
>>>
> IMO the test needs fix, KRA on replica file needs KRA related
> certificates in replica file
> 
> [ipa.ipatests.test_integration.host.Host.replica2.ParamikoTransport] RUN
> ['ipa-kra-install', '-U', '-p', 'Secret123',
> '/root/ipatests/replica-info.gpg']
> [ipa.ipatests.test_integration.host.Host.replica2.cmd27] RUN
> ['ipa-kra-install', '-U', '-p', 'Secret123',
> '/root/ipatests/replica-info.gpg']
> [ipa.ipatests.test_integration.host.Host.replica2.cmd27] Missing KRA
> certificates, please create a new replica file.
> [ipa.ipatests.test_integration.host.Host.replica2.cmd27] The
> ipa-kra-install command failed. See /var/log/ipaserver-kra-install.log
> for more information
> [ipa.ipatests.test_integration.host.Host.replica2.cmd27] Exit code: 1
> FAILED
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> traceback
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> 
> 
> self = <ipatests.test_integration.test_replica_promotion.TestKRAInstall
> object at 0x7f660bc1a590>
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From c84eb7367f90e89b353ec0d0526f5d81054ca191 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Mon, 29 Feb 2016 11:06:48 +0100
Subject: [PATCH] Integration tests for replica promotion feature

---
 .../test_integration/test_replica_promotion.py     | 233 +++++++++++++++++++++
 1 file changed, 233 insertions(+)
 create mode 100644 ipatests/test_integration/test_replica_promotion.py

diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
new file mode 100644
index 0000000000000000000000000000000000000000..4c3b7ac1fbe7dc1d898bfba726aa90f1b293ab45
--- /dev/null
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -0,0 +1,233 @@
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration import tasks
+from ipatests.test_integration.test_caless import assert_error
+from ipalib.constants import DOMAIN_LEVEL_0
+from ipalib.constants import DOMAIN_LEVEL_1
+from env_config import get_global_config
+from ipalib.constants import DOMAIN_SUFFIX_NAME
+
+
+class ReplicaPromotionBase(IntegrationTest):
+    config = get_global_config()
+
+    @classmethod
+    def install(cls, mh):
+        tasks.install_master(cls.master, domain_level=cls.domain_level)
+
+    def teardown_method(self, method):
+        for host in self.replicas:
+            tasks.uninstall_replica(self.master, host)
+            tasks.uninstall_client(host)
+            result = self.master.run_command(
+                ["ipa", "host-del", "--updatedns", host.hostname],
+                raiseonerr=False)
+             # Workaround for 5627
+            if "host not found" in result.stderr_text:
+                self.master.run_command(["ipa",
+                                         "host-del",
+                                         host.hostname], raiseonerr=False)
+
+    def test_kra_install_master(self):
+        result1 = tasks.install_kra(self.master,
+                                    first_instance=True,
+                                    raiseonerr=False)
+        assert result1.returncode == 0, result1.stderr_text
+        tasks.kinit_admin(self.master)
+        result2 = self.master.run_command(["ipa", "vault-find"],
+                                          raiseonerr=False)
+        found = result2.stdout_text.find("0 vaults matched")
+        assert(found > 0), result2.stdout_text
+
+
+class TestReplicaPromotionLevel0(ReplicaPromotionBase):
+
+    topology = 'star'
+    num_replicas = 1
+    domain_level = DOMAIN_LEVEL_0
+
+    def test_promotion_disabled(self):
+        """
+        Testcase:
+        http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan#Test_case:
+        _Make_sure_the_feature_is_unavailable_under_domain_level_0
+        """
+        client = self.replicas[0]
+        tasks.install_client(self.master, client)
+        args = ['ipa-replica-install', '-U',
+                '-p', self.master.config.dirman_password,
+                '-w', self.master.config.admin_password,
+                '--ip-address', client.ip]
+        result = client.run_command(args, raiseonerr=False)
+        assert_error(result,
+                     'You must provide a file generated by ipa-replica-prepare'
+                     ' to create a replica when the domain is at level 0', 1)
+
+    def test_backup_restore(self):
+        """
+        TestCase:
+        http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan#Test_case:
+        _ipa-restore_after_domainlevel_raise_restores_original_domain_level
+        """
+        command = ["ipa", "topologysegment-find", DOMAIN_SUFFIX_NAME]
+        tasks.install_replica(self.master, self.replicas[0])
+        backup_file = tasks.ipa_backup(self.master)
+        self.master.run_command(["ipa", "domainlevel-set", "1"])
+        result = self.master.run_command(command)
+        found1 = result.stdout_text.rfind("1 segment matched")
+        assert(found1 > 0), result.stdout_text
+        tasks.ipa_restore(self.master, backup_file)
+        result2 = self.master.run_command(command, raiseonerr=False)
+        found2 = result2.stdout_text.rfind("0 segments matched")
+        assert(found2 > 0), result2.stdout_text
+
+
+class TestKRAInstall(IntegrationTest):
+    """
+    TestCase: http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan
+    #Test_case:_ipa-kra-install_with_replica_file_works_only_on_domain_level_0
+    """
+    config = get_global_config()
+    topology = 'star'
+    num_clients = 0
+    domain_level = DOMAIN_LEVEL_0
+    num_replicas = 2
+
+    @classmethod
+    def install(cls, mh):
+        tasks.install_master(cls.master, domain_level=cls.domain_level)
+
+    def test_kra_install_without_replica_file(self):
+        master = self.master
+        replica1 = self.replicas[0]
+        replica2 = self.replicas[1]
+        tasks.install_kra(master, first_instance=True)
+        tasks.install_replica(master, replica1)
+        result1 = tasks.install_kra(replica1,
+                                    domain_level=DOMAIN_LEVEL_1,
+                                    raiseonerr=False)
+        assert_error(result1, "A replica file is required", 1)
+        tasks.install_kra(replica1,
+                          domain_level=DOMAIN_LEVEL_0,
+                          raiseonerr=True)
+        # Now prepare the replica file, copy it to the client and raise
+        # domain level on master to test the reverse situation
+        tasks.replica_prepare(master, replica2)
+        master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+        tasks.install_replica(master, replica2)
+        result2 = tasks.install_kra(replica2,
+                                    domain_level=DOMAIN_LEVEL_0,
+                                    raiseonerr=False)
+        assert_error(result2, "No replica file is required", 1)
+        tasks.install_kra(replica2)
+
+
+class TestCAInstall(IntegrationTest):
+    config = get_global_config()
+    topology = 'star'
+    num_clients = 0
+    domain_level = DOMAIN_LEVEL_0
+    num_replicas = 2
+
+    @classmethod
+    def install(cls, mh):
+        tasks.install_master(cls.master, domain_level=cls.domain_level)
+
+    def test_ca_install_without_replica_file(self):
+        """
+        TestCase:
+        http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan#Test_case:
+        _ipa-ca-install_with_replica_file_works_only_on_domain_level_0
+        """
+        master = self.master
+        replica1 = self.replicas[0]
+        replica2 = self.replicas[1]
+        for i in self.replicas:
+            tasks.install_replica(master, i, setup_ca=False,
+                                  setup_dns=True)
+        result1 = tasks.install_ca(replica1, domain_level=1, raiseonerr=False)
+        assert_error(result1, "If you wish to replicate CA to this host, please"
+                              " re-run 'ipa-ca-install'\nwith a replica file"
+                              " generated on an existing CA master as argument.",
+                              1)
+
+        tasks.install_ca(replica1, domain_level=0)
+        # Now prepare the replica file, copy it to the client and raise
+        # domain level on master to test the reverse situation
+        master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+        result2 = tasks.install_ca(replica2, domain_level=0, raiseonerr=False)
+        assert_error(result2, 'Too many parameters provided.'
+                              ' No replica file is required', 1)
+        tasks.install_ca(replica2, domain_level=1)
+
+
+class TestReplicaPromotionLevel1(ReplicaPromotionBase):
+    """
+    TestCase: http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan#
+    Test_case:_Make_sure_the_old_workflow_is_disabled_at_domain_level_1
+    """
+
+    topology = 'star'
+    num_clients = 0
+    num_replicas = 1
+    domain_level = DOMAIN_LEVEL_1
+
+    def test_replica_prepare_disabled(self):
+        config = self.master.config
+        replica = self.replicas[0]
+        args = ['ipa-replica-prepare',
+                '-p', config.dirman_password,
+                '--ip-address', replica.ip,
+                replica.hostname]
+        result = self.master.run_command(args, raiseonerr=False)
+        assert_error(result, "Replica creation using 'ipa-replica-prepare'"
+                             " to generate replica file\n"
+                             "is supported only in 0-level IPA domain", 1)
+
+
+class TestReplicaManageCommands(IntegrationTest):
+    topology = "star"
+    num_replicas = 2
+    num_clients = 0
+    domain_level = DOMAIN_LEVEL_0
+
+    def test_replica_manage_commands(self):
+        """
+        TestCase: http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan
+        #Test_case:_ipa-replica-manage_is_deprecated_in_domain_level_1
+        """
+        master = self.master
+        replica1 = self.replicas[0]
+        replica2 = self.replicas[1]
+
+        result1 = master.run_command(["ipa-replica-manage",
+                                      "connect",
+                                      replica1.hostname,
+                                      replica2.hostname],
+                                     raiseonerr=False)
+        assert result1.returncode == 0, result1.stderr_text
+        result2 = master.run_command(["ipa-replica-manage",
+                                      "disconnect",
+                                      replica1.hostname,
+                                      replica2.hostname],
+                                     raiseonerr=False)
+        assert result2.returncode == 0, result2.stderr_text
+        master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+        result3 = master.run_command(["ipa-replica-manage",
+                                      "connect",
+                                      replica1.hostname,
+                                      replica2.hostname],
+                                     raiseonerr=False)
+        assert_error(result3, 'Creation of IPA replication agreement is'
+                              ' deprecated with managed IPA replication'
+                              ' topology. Please use `ipa topologysegment-*`'
+                              ' commands to manage the topology', 1)
+        tasks.create_segment(master, replica1, replica2)
+        result4 = master.run_command(["ipa-replica-manage",
+                                      "disconnect",
+                                      replica1.hostname,
+                                      replica2.hostname],
+                                     raiseonerr=False)
+        assert_error(result4, 'Removal of IPA replication agreement is'
+                              ' deprecated with managed IPA replication'
+                              ' topology. Please use `ipa topologysegment-*`'
+                              ' commands to manage the topology', 1)
-- 
1.8.3.1

From 9cc97961e97fbdf1e69e10ac359d910824887cd1 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Mon, 29 Feb 2016 12:51:08 +0100
Subject: [PATCH] Enabled setting domain level explicitly in test class

Needed for replica promotion tests
---
 ipatests/test_integration/base.py  |  8 +++++++-
 ipatests/test_integration/tasks.py | 26 ++++++++++++++++----------
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/ipatests/test_integration/base.py b/ipatests/test_integration/base.py
index 4f57e959032a5fda0ad002fca95501da1160605b..ee271679912fa6218dcd91de61570f9d8fc09d06 100644
--- a/ipatests/test_integration/base.py
+++ b/ipatests/test_integration/base.py
@@ -37,6 +37,7 @@ class IntegrationTest(object):
     num_ad_domains = 0
     required_extra_roles = []
     topology = None
+    domain_level = None
 
     @classmethod
     def setup_class(cls):
@@ -62,11 +63,16 @@ class IntegrationTest(object):
 
     @classmethod
     def install(cls, mh):
+        if cls.domain_level is not None:
+            domain_level = cls.domain_level
+        else:
+            domain_level = cls.master.config.domain_level
         if cls.topology is None:
             return
         else:
             tasks.install_topo(cls.topology,
-                               cls.master, cls.replicas, cls.clients)
+                               cls.master, cls.replicas,
+                               cls.clients, domain_level)
     @classmethod
     def teardown_class(cls):
         pass
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index ad2eab218272cc19d208b788d518910821ce5a8a..72df1b146ae35188fc399f90e48300857ba1f0e0 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -92,9 +92,9 @@ def allow_sync_ptr(host):
 
 
 def apply_common_fixes(host):
+    prepare_host(host)
     fix_etc_hosts(host)
     fix_hostname(host)
-    prepare_host(host)
 
 
 def backup_file(host, filename):
@@ -224,7 +224,9 @@ def enable_replication_debugging(host):
                      stdin_text=logging_ldif)
 
 
-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False, domain_level=None):
+    if domain_level is None:
+        domain_level = host.config.domain_level
     host.collect_log(paths.IPASERVER_INSTALL_LOG)
     host.collect_log(paths.IPACLIENT_INSTALL_LOG)
     inst = host.domain.realm.replace('.', '-')
@@ -240,7 +242,7 @@ def install_master(host, setup_dns=True, setup_kra=False):
         '-r', host.domain.realm,
         '-p', host.config.dirman_password,
         '-a', host.config.admin_password,
-        "--domain-level=%i" % host.config.domain_level
+        "--domain-level=%i" % domain_level
     ]
 
     if setup_dns:
@@ -291,7 +293,6 @@ def master_authoritative_for_client_domain(master, client):
 
 
 def replica_prepare(master, replica):
-    apply_common_fixes(replica)
     fix_apache_semaphores(replica)
     prepare_reverse_zone(master, replica.ip)
     args = ['ipa-replica-prepare',
@@ -307,7 +308,10 @@ def replica_prepare(master, replica):
 
 
 def install_replica(master, replica, setup_ca=True, setup_dns=False,
-                    setup_kra=False):
+                    setup_kra=False, domain_level=None):
+    if domain_level is None:
+        domain_level = domainlevel(master)
+    apply_common_fixes(replica)
     replica.collect_log(paths.IPAREPLICA_INSTALL_LOG)
     replica.collect_log(paths.IPAREPLICA_CONNCHECK_LOG)
     allow_sync_ptr(master)
@@ -325,7 +329,7 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False,
         ])
     if master_authoritative_for_client_domain(master, replica):
         args.extend(['--ip-address', replica.ip])
-    if domainlevel(master) == DOMAIN_LEVEL_0:
+    if domain_level == DOMAIN_LEVEL_0:
         # prepare the replica file on master and put it to replica, AKA "old way"
         replica_prepare(master, replica)
         replica_filename = get_replica_filename(replica)
@@ -334,7 +338,8 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False,
         # install client on a replica machine and then promote it to replica
         install_client(master, replica)
         fix_apache_semaphores(replica)
-
+        args.extend(['-n', replica.domain.name,
+                     '-r', replica.domain.realm])
     replica.run_command(args)
     enable_replication_debugging(replica)
     setup_sssd_debugging(replica)
@@ -371,7 +376,8 @@ def install_client(master, client, extra_args=()):
                         '--realm', client.domain.realm,
                         '-p', client.config.admin_name,
                         '-w', client.config.admin_password,
-                        '--server', master.hostname]
+                        '--server', master.hostname,
+                        '--force']
                        + list(extra_args))
 
     setup_sssd_debugging(client)
@@ -971,13 +977,13 @@ def double_circle_topo(master, replicas, site_size=6):
             yield site[1], site_servers[-1]
 
 
-def install_topo(topo, master, replicas, clients,
+def install_topo(topo, master, replicas, clients, domain_level=None,
                  skip_master=False, setup_replica_cas=True):
     """Install IPA servers and clients in the given topology"""
     replicas = list(replicas)
     installed = {master}
     if not skip_master:
-        install_master(master)
+        install_master(master, domain_level=domain_level)
 
     add_a_records_for_hosts_in_master_domain(master)
 
-- 
1.8.3.1

From ccf143c509adf7c85d49c96a43fc2ae5f6af4aa3 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Mon, 29 Feb 2016 13:03:57 +0100
Subject: [PATCH] Removed a constantly failing call to prepare_host

prepare_host is executed from within each of install_master, install_replica
and install_client in tasks.py anyway, so no need to call it here also.
Besindes this call kept failing when IntegrationTest wes initialized more than
once during the test execution.
---
 ipatests/pytest_plugins/integration.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ipatests/pytest_plugins/integration.py b/ipatests/pytest_plugins/integration.py
index a191d848669111211cc1a13f412e3f777eae565e..b4002e33c12c768080d4f59a554b5db9a09a8960 100644
--- a/ipatests/pytest_plugins/integration.py
+++ b/ipatests/pytest_plugins/integration.py
@@ -30,7 +30,6 @@ from pytest_multihost import make_multihost_fixture
 
 from ipapython import ipautil
 from ipapython.ipa_log_manager import log_mgr
-from ipatests.test_integration import tasks
 from ipatests.test_integration.config import Config
 from ipatests.test_integration.env_config import get_global_config
 
@@ -197,8 +196,6 @@ def mh(request, class_integration_logs):
     print(mh.config)
     for host in mh.config.get_all_hosts():
         host.add_log_collector(collect_log)
-        cls.log.info('Preparing host %s', host.hostname)
-        tasks.prepare_host(host)
 
     setup_class(cls, mh)
     mh._pytestmh_request.addfinalizer(lambda: teardown_class(cls))
-- 
1.8.3.1

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to