[Freeipa-devel] [PATCH 016, 024, 025] First part of the replica promotion tests + testplan
Martin Basti
mbasti at redhat.com
Fri Mar 4 07:37:12 UTC 2016
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):
>
> 2)
> Why are these there? I do not see any usage
>
> from env_config import get_global_config
> config = get_global_config()
>
> 3) nitpick
> + num_clients = 0
> this is set by default
>
> 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
More information about the Freeipa-devel
mailing list