Hi Martin,

I've made the requested changes.

The full set of necessary patches is attached.


On 03/02/2016 10:05 AM, Martin Basti wrote:
> 
> 
> On 02.03.2016 00:12, Oleg Fayans wrote:
>> Hi Martin,
>>
>> On 03/01/2016 07:04 PM, Martin Basti wrote:
>>>
>>> On 01.03.2016 14:56, Martin Basti wrote:
>>>>
>>>>
>>>> On 01.03.2016 12:37, Martin Basti wrote:
>>>>>
>>>>> On 01.03.2016 12:32, Martin Basti wrote:
>>>>>>
>>>>>> On 29.02.2016 13:16, Oleg Fayans wrote:
>>>>>>> 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.
>>>>>> hello,
>>>>>>
>>>>>> I cannot apply patches on master branch
>>>>>> Martin^2
>>>>> My bad I applied wrong patch
>>>>>
>>>>>>>
>>>>>>> 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>
>>>>>>>>
>>>> I just read the code.
>>>>
>>>> PATCH 16:
>>>> 0)
>>>> PEP8
>>>> ./ipatests/test_integration/test_replica_promotion.py:24:14: E111
>>>> indentation is not a multiple of four
>>>> ./ipatests/test_integration/test_replica_promotion.py:24:14: E113
>>>> unexpected indentation
>>>> ./ipatests/test_integration/test_replica_promotion.py:148:80: E501
>>>> line too long (80 > 79 characters)
>>>> ./ipatests/test_integration/test_replica_promotion.py:150:80: E501
>>>> line too long (81 > 79 characters)
>>>>
>>>> 1)
>>>> workaround is not workaround, because the host entry is removed
>>>> anyway, the error is raised from POST callback, please remove it
>>>> +             # Workaround for 5627
>>>> +            if "host not found" in result.stderr_text:
>>>> +                self.master.run_command(["ipa",
>>>> +                                         "host-del",
>>>> +                                         host.hostname],
>>>> raiseonerr=False)
>>> sorry, I was wrong with this, check is in pre_callback, but please
>>> remove it anyway, I will send patch to fix it ASAP
>> Done
> I realized that the fix I'm working on is for 4.4 only, so for 4.3 add
> this as separated patch.
Done, patch 0027

>>>> 2)
>>>> Please name it better, for example "replica" instead of "i"
>>>> +        for i in self.replicas:
>>>> +            tasks.install_replica(master, i, setup_ca=False,
>>>> +                                  setup_dns=True)
>> Done
>>
>>>> 3)
>>>> Please use constant for domain level (multiple times)
>>>> + result1 = tasks.install_ca(replica1, domain_level=1,
>>>> raiseonerr=False)
>>>>
>>>> +        tasks.install_ca(replica1, domain_level=0)
>>>> +        result2 = tasks.install_ca(replica2, domain_level=0,
>>>> raiseonerr=False)
>>>> ... more times
>> Done
>>
>>>> 4)
>>>> This link does not exists, only connect is deprecated not
>>>> ipa-replica-manage at all
>>>> +    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
>>>> +        """
>> Fixed
>>
>>>> 5)
>>>> Missing testcases:
>>>>
>>>> Test case: Unprivileged users are not allowed to enroll and promote
>>>> clients
>>>> Test case: Replica created using old workflow is functional after
>>>> domain upgrade
>>>> Test case: ipa-csreplica-manage connect is deprecated in domain level 1
>>>> Test case: Replica can be installed using one command
>>>> Test case: Prohibit ipa server uninstallation from disconnecting
>>>> topology segment
>>>>
>> They are on the way, not fully ready yet
>>
>>>> PATCH 24:
>>>>
>>>> 1)
>>>> why there is this change, how it is related to this patch?:
>>>>   def apply_common_fixes(host):
>>>> +    prepare_host(host)
>>>>       fix_etc_hosts(host)
>>>>       fix_hostname(host)
>>>> -    prepare_host(host)
>> Good catch! That was one of my attempts to address the issue that was
>> successfully resolved in patch 0025. Will remove it once we agree on the
>> rest of the changes

Removed

>>
>>>> 2)
>>>> Why is there this change, how it is related to this patch?:
>>>>   def replica_prepare(master, replica):
>>>> -    apply_common_fixes(replica)
>>>>       fix_apache_semaphores(replica)
>>>> ...
>>>>   def install_replica(master, replica, setup_ca=True, setup_dns=False,
>>>> ...
>>>> +    apply_common_fixes(replica)
>> Just to make this call independent from domain level (at domain_level 1
>> replica_prepare never gets called)
> It should be in separate commit, because it is not related to adding
> domain_level in class functionality

Done. Patch 0026

>>
>>
>>>> 3)
>>>> why is there this change, how it is related to this patch?:
>>>> -
>>>> +        args.extend(['-n', replica.domain.name,
>>>> +                     '-r', replica.domain.realm])
>> At least -r is a required parameter. -n was added for further
>> robustness. Can be safely removed, though
> It should be in separate commit, as this is not related to domain levels

Done. Patch 0026

>>
>>>> 4)
>>>> why there force, how is this change related to this patch (domain
>>>> levels)?
>>>>                           '-w', client.config.admin_password,
>>>> -                        '--server', master.hostname]
>>>> +                        '--server', master.hostname,
>>>> +                        '--force']
>>>>                          + list(extra_args))
>> client refuses to install unless everything is super clear in the dns
>> setup (including reverse zone). Otherwise the installer fails and
>> informs you that you may use '--force' at your own risk. I can rerun the
>> tests without this option to provide you with the exact output, if you
>> like.
> It should be in separated commit, because it is not related to domain
> levels

I've run the tests without this option again at it passed. Must have
been some temporary issue. Removed this change.

>>
>>>> Otherwise domain level related changes LGTM
>>>>
>>>> PATCH 25
>>>>
>>>> LGTM
>>>>
>>>> Martin^2
>>>>
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 6bd25c80f38c3bad19785c157700c87b5c34e87f Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Tue, 1 Mar 2016 23:57:52 +0100
Subject: [PATCH] Integration tests for replica promotion feature

---
 .../test_integration/test_replica_promotion.py     | 232 +++++++++++++++++++++
 1 file changed, 232 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..bff10f1401106b5b901723448d9ed988dba4836d
--- /dev/null
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -0,0 +1,232 @@
+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)
+
+    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 replica in self.replicas:
+            tasks.install_replica(master, replica, setup_ca=False,
+                                  setup_dns=True)
+        result1 = tasks.install_ca(replica1,
+                                   domain_level=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=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=DOMAIN_LEVEL_0,
+                                   raiseonerr=False)
+        assert_error(result2, 'Too many parameters provided.'
+                              ' No replica file is required', 1)
+        tasks.install_ca(replica2, domain_level=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_connect_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 073e898731685fddef1768b3aeff6f68b801a48c Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@ofayans.usersys.redhat.com>
Date: Fri, 19 Feb 2016 13:46:10 +0100
Subject: [PATCH] Removed messing around with resolv.conf

---
 ipatests/test_integration/tasks.py | 44 ++------------------------------------
 1 file changed, 2 insertions(+), 42 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index d37b616bd6efe437a1a979cc7a9ad8c7ea803773..e859cfc8501eec32fb2233960dd8d7fa64be76d1 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -44,8 +44,6 @@ from ipalib.constants import DOMAIN_LEVEL_0
 
 log = log_mgr.get_logger(__name__)
 
-IPATEST_NM_CONFIG = '20-ipatest-unmanaged-resolv.conf'
-
 
 def check_arguments_are(slice, instanceof):
     """
@@ -91,12 +89,9 @@ def allow_sync_ptr(host):
                      raiseonerr=False)
 
 
-def apply_common_fixes(host, fix_resolv=True):
+def apply_common_fixes(host):
     fix_etc_hosts(host)
     fix_hostname(host)
-    modify_nm_resolv_conf_settings(host)
-    if fix_resolv:
-        fix_resolv_conf(host)
     prepare_host(host)
 
 
@@ -154,40 +149,6 @@ def host_service_active(host, service):
         return False
 
 
-def modify_nm_resolv_conf_settings(host):
-    if not host_service_active(host, 'NetworkManager'):
-        return
-
-    config = "[main]\ndns=none\n"
-    path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
-
-    host.put_file_contents(path, config)
-    host.run_command(['systemctl', 'restart', 'NetworkManager'],
-                     raiseonerr=False)
-
-
-def undo_nm_resolv_conf_settings(host):
-    if not host_service_active(host, 'NetworkManager'):
-        return
-
-    path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
-    host.run_command(['rm', '-f', path], raiseonerr=False)
-    host.run_command(['systemctl', 'restart', 'NetworkManager'],
-                     raiseonerr=False)
-
-
-def fix_resolv_conf(host):
-    backup_file(host, paths.RESOLV_CONF)
-    lines = host.get_file_contents(paths.RESOLV_CONF).splitlines()
-    lines = ['#' + l if l.startswith('nameserver') else l for l in lines]
-    for other_host in host.domain.hosts:
-        if other_host.role in ('master', 'replica'):
-            lines.append('nameserver %s' % other_host.ip)
-    contents = '\n'.join(lines)
-    log.debug('Writing the following to /etc/resolv.conf:\n%s', contents)
-    host.put_file_contents(paths.RESOLV_CONF, contents)
-
-
 def fix_apache_semaphores(master):
     systemd_available = master.transport.file_exists(paths.SYSTEMCTL)
 
@@ -204,7 +165,6 @@ def fix_apache_semaphores(master):
 def unapply_fixes(host):
     restore_files(host)
     restore_hostname(host)
-    undo_nm_resolv_conf_settings(host)
 
     # Clean up the test directory
     host.run_command(['rm', '-rvf', host.config.test_dir])
@@ -269,7 +229,7 @@ def install_master(host, setup_dns=True, setup_kra=False):
     host.collect_log(paths.SLAPD_INSTANCE_ERROR_LOG_TEMPLATE % inst)
     host.collect_log(paths.SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE % inst)
 
-    apply_common_fixes(host, fix_resolv=False)
+    apply_common_fixes(host)
     fix_apache_semaphores(host)
 
     args = [
-- 
1.8.3.1

From 49f3cc9504c29e114a4bcc6b78b426118c131ce4 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Wed, 2 Mar 2016 12:55:33 +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 | 16 ++++++++++------
 2 files changed, 17 insertions(+), 7 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..660e6de2c0f32fa9238b679b7afa2ea8eb6ee6aa 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -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:
@@ -307,7 +309,9 @@ 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)
     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)
@@ -971,13 +975,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

From 03bd80158888964e3ced3a68f022a64fcd7994b7 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Wed, 2 Mar 2016 13:00:31 +0100
Subject: [PATCH] Made apply_common_fixes call at replica installation
 independent on domain_level

Besides added obligatory domain/realm-specific commandline options
to replica installation
---
 ipatests/test_integration/tasks.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 660e6de2c0f32fa9238b679b7afa2ea8eb6ee6aa..5553dd7accbbb436c4bac805f8a1be5198f98950 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -293,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',
@@ -312,6 +311,7 @@ def install_replica(master, replica, setup_ca=True, setup_dns=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)
@@ -338,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)
-- 
1.8.3.1

From 5f0093130d12c8e953873e0f20af6a193fd92b2a Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Wed, 2 Mar 2016 13:39:00 +0100
Subject: [PATCH] Workaround for ticket 5627

---
 ipatests/test_integration/test_replica_promotion.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ipatests/test_integration/test_replica_promotion.py b/ipatests/test_integration/test_replica_promotion.py
index bff10f1401106b5b901723448d9ed988dba4836d..c791e7d84c35eee4854c837aae241b43948f3555 100644
--- a/ipatests/test_integration/test_replica_promotion.py
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -21,6 +21,11 @@ class ReplicaPromotionBase(IntegrationTest):
             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,
-- 
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