Hi, Martin.

An updated version of the patch is attached. Please see my comments below

On 03/04/2016 08:39 AM, Martin Basti wrote:
> 
> 
> On 04.03.2016 08:37, Martin Basti wrote:
>>
>>
>> On 03.03.2016 18:38, Martin Basti wrote:
>>>
>>>
>>> On 02.03.2016 13:47, Oleg Fayans wrote:
>>>> 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
>>>>>>>>
>>>
>>> 1)
>>> this method is unused please remove it
>>>
>>>     def test_kra_install_master(self):

Well, in fact it is used twice: in both domain levels, so I'd better
keep it:

-bash-4.3$ ipa-run-tests test_integration/test_replica_promotion.py
--collect-only
====================================================================================
test session starts
=====================================================================================
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: sourceorder, multihost
collected 8 items
<Module 'test_integration/test_replica_promotion.py'>
  <Class 'TestReplicaPromotionLevel0'>
    <Instance '()'>
      <Function 'test_kra_install_master'>
      <Function 'test_promotion_disabled'>
      <Function 'test_backup_restore'>
  <Class 'TestKRAInstall'>
    <Instance '()'>
      <Function 'test_kra_install_without_replica_file'>
  <Class 'TestCAInstall'>
    <Instance '()'>
      <Function 'test_ca_install_without_replica_file'>
  <Class 'TestReplicaPromotionLevel1'>
    <Instance '()'>
      <Function 'test_kra_install_master'>
      <Function 'test_replica_prepare_disabled'>
  <Class 'TestReplicaManageCommands'>
    <Instance '()'>
      <Function 'test_replica_manage_commands'>


>>>
>>> 2)
>>> Why are these there? I do not see any usage
>>>
>>> from env_config import get_global_config
>>> config = get_global_config()

Removed

>>>
>>> 3) nitpick
>>> +    num_clients = 0
>>> this is set by default

Removed

>>>
>>> otherwise LGTM
>>>
>>> Results of testing tomorrow.
>>>
>>> Martin^2
>>>
>>
>> I applied all patches including workarounds, but test failed.
>>
>> ipatests.test_integration.test_replica_promotion.TestReplicaPromotionLevel0
>>
>>
>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] RUN
>> ['ipa-replica-install', '-U', '-p', 'Secret123', '-w', 'Secret123',
>> '--setup-ca', '--ip-address', '192.168.144.102',
>> '/root/ipatests/replica-info.gpg']
>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] The host
>> replica1.ipa.test already exists on the master server.
>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] You should
>> remove it before proceeding:
>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]     % ipa
>> host-del replica1.ipa.test
>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51]
>> ipa.ipapython.install.cli.install_tool(Replica): ERROR    The
>> ipa-replica-install command failed. See
>> /var/log/ipareplica-install.log for more information
>> [ipa.ipatests.test_integration.host.Host.replica1.cmd51] Exit code: 3
>> FAILED

this is exactly the error that happens when a workaround for 5627 is not
applied. I have re-run the tests with all the patches and everything
passed. Could you please double-check, whether patch 0027 was applied
correctly?

bash-4.3$ ipa-run-tests test_integration/test_replica_promotion.py --pdb
====================================================================================
test session starts
=====================================================================================
platform linux2 -- Python 2.7.10 -- py-1.4.30 -- pytest-2.7.3
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: sourceorder, multihost
collected 8 items

test_integration/test_replica_promotion.py ........

================================================================================
8 passed in 7561.93 seconds
=================================================================================


>>
> And it needs ticket, otherwise it will not be in 4-3 branch.

https://fedorahosted.org/freeipa/ticket/5723
-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From b77c6a3c583d085f678e3ab749b2387a0bbb7c7d Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Fri, 4 Mar 2016 14:15:50 +0100
Subject: [PATCH] Integration tests for replica promotion feature

---
 .../test_integration/test_replica_promotion.py     | 223 +++++++++++++++++++++
 1 file changed, 223 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..300cd05a7685602ad1a4bc71b0fba135c9a5e3d5
--- /dev/null
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -0,0 +1,223 @@
+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 ipalib.constants import DOMAIN_SUFFIX_NAME
+
+
+class ReplicaPromotionBase(IntegrationTest):
+
+    @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
+    """
+    topology = 'star'
+    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):
+    topology = 'star'
+    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_replicas = 1
+    domain_level = DOMAIN_LEVEL_1
+
+    def test_replica_prepare_disabled(self):
+        replica = self.replicas[0]
+        args = ['ipa-replica-prepare',
+                '-p', self.master.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
+    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)
-- 
2.5.0

-- 
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