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
> 

-- 
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From cd883693fb7d07de662b57370440dae50fddfe5b Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Thu, 17 Dec 2015 08:50:13 +0100
Subject: [PATCH] Prepared integration tests for replica promotion

---
 .../test_integration/test_replica_promotion.py     | 218 +++++++++++++++++++++
 1 file changed, 218 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..e790890db06e52fa155348e39ce4804ecdd1d3bf
--- /dev/null
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -0,0 +1,218 @@
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration import tasks
+from ipatests.test_integration.test_caless import assert_error
+from ipalib.constants import DOMAIN_LEVEL_0
+from ipalib.constants import DOMAIN_LEVEL_1
+from env_config import get_global_config
+from ipalib.constants import DOMAIN_SUFFIX_NAME
+
+
+class Dummy(IntegrationTest):
+    config = get_global_config()
+
+    @classmethod
+    def install(cls, mh):
+        tasks.install_master(cls.master, domain_level=cls.domain_level)
+
+    def teardown_method(self, method):
+        for host in self.replicas:
+            tasks.uninstall_replica(self.master, host)
+            tasks.uninstall_client(host)
+            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(Dummy):
+
+    topology = 'star'
+    num_replicas = 1
+    domain_level = DOMAIN_LEVEL_0
+
+    def test_promotion_disabled(self):
+        """
+        Testcase:
+        http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan#Test_case:
+        _Make_sure_the_feature_is_unavailable_under_domain_level_0
+        """
+        client = self.replicas[0]
+        tasks.install_client(self.master, client)
+        args = ['ipa-replica-install', '-U',
+                '-p', self.master.config.dirman_password,
+                '-w', self.master.config.admin_password,
+                '--ip-address', client.ip]
+        result = client.run_command(args, raiseonerr=False)
+        assert_error(result,
+                     'You must provide a file generated by ipa-replica-prepare'
+                     ' to create a replica when the domain is at level 0', 1)
+
+    def test_backup_restore(self):
+        """
+        TestCase:
+        http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan#Test_case:
+        _ipa-restore_after_domainlevel_raise_restores_original_domain_level
+        """
+        command = ["ipa", "topologysegment-find", DOMAIN_SUFFIX_NAME]
+        tasks.install_replica(self.master, self.replicas[0])
+        backup_file = tasks.ipa_backup(self.master)
+        self.master.run_command(["ipa", "domainlevel-set", "1"])
+        result = self.master.run_command(command)
+        found1 = result.stdout_text.rfind("1 segment matched")
+        assert(found1 > 0), result.stdout_text
+        tasks.ipa_restore(self.master, backup_file)
+        result2 = self.master.run_command(command, raiseonerr=False)
+        found2 = result2.stdout_text.rfind("0 segments matched")
+        assert(found2 > 0), result2.stdout_text
+
+
+class TestKRAInstall(IntegrationTest):
+    """
+    TestCase: http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan
+    #Test_case:_ipa-kra-install_with_replica_file_works_only_on_domain_level_0
+    """
+    config = get_global_config()
+    topology = 'star'
+    num_clients = 0
+    domain_level = DOMAIN_LEVEL_0
+    num_replicas = 2
+
+    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)
+        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
+        master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+        result2 = tasks.install_kra(replica2,
+                                    domain_level=DOMAIN_LEVEL_0,
+                                    raiseonerr=False)
+        assert_error(result2, "No replica file is required", 1)
+        tasks.install_kra(replica2)
+
+
+class TestCAInstall(IntegrationTest):
+    config = get_global_config()
+    topology = 'star'
+    num_clients = 0
+    domain_level = DOMAIN_LEVEL_0
+    num_replicas = 2
+
+    @classmethod
+    def install(cls, mh):
+        tasks.install_master(cls.master, domain_level=cls.domain_level)
+
+    def test_ca_install_without_replica_file(self):
+        """
+        TestCase:
+        http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan#Test_case:
+        _ipa-ca-install_with_replica_file_works_only_on_domain_level_0
+        """
+        master = self.master
+        replica1 = self.replicas[0]
+        replica2 = self.replicas[1]
+        for i in self.replicas:
+            tasks.install_replica(master, i, setup_ca=False,
+                                  setup_dns=True)
+        result1 = tasks.install_ca(replica1, domain_level=1, raiseonerr=False)
+        assert_error(result1, "A replica file is required", 1)
+        tasks.install_ca(replica1, domain_level=0)
+        # Now prepare the replica file, copy it to the client and raise
+        # domain level on master to test the reverse situation
+        master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+        result2 = tasks.install_ca(replica2, domain_level=0, raiseonerr=False)
+        assert_error(result2, 'Too many parameters provided.'
+                              ' No replica file is required', 1)
+        tasks.install_ca(replica2, domain_level=1)
+
+
+class TestReplicaPromotionLevel1(Dummy):
+    """
+    TestCase: http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan#
+    Test_case:_Make_sure_the_old_workflow_is_disabled_at_domain_level_1
+    """
+
+    topology = 'star'
+    num_clients = 0
+    num_replicas = 1
+    domain_level = DOMAIN_LEVEL_1
+
+    def test_replica_prepare_disabled(self):
+        config = self.master.config
+        replica = self.replicas[0]
+        args = ['ipa-replica-prepare',
+                '-p', config.dirman_password,
+                '--ip-address', replica.ip,
+                replica.hostname]
+        result = self.master.run_command(args, raiseonerr=False)
+        assert_error(result, "Replica creation using 'ipa-replica-prepare'"
+                             " to generate replica file\n"
+                             "is supported only in 0-level IPA domain", 1)
+
+
+class TestReplicaManageCommands(IntegrationTest):
+    topology = "star"
+    num_replicas = 2
+    num_clients = 0
+    domain_level = DOMAIN_LEVEL_0
+
+    def test_replica_manage_commands(self):
+        """
+        TestCase: http://www.freeipa.org/page/V4/Replica_Promotion/Test_plan
+        #Test_case:_ipa-replica-manage_is_deprecated_in_domain_level_1
+        """
+        master = self.master
+        replica1 = self.replicas[0]
+        replica2 = self.replicas[1]
+
+        result1 = master.run_command(["ipa-replica-manage",
+                                      "connect",
+                                      replica1.hostname,
+                                      replica2.hostname],
+                                     raiseonerr=False)
+        assert result1.returncode == 0, result1.stderr_text
+        result2 = master.run_command(["ipa-replica-manage",
+                                      "disconnect",
+                                      replica1.hostname,
+                                      replica2.hostname],
+                                     raiseonerr=False)
+        assert result2.returncode == 0, result2.stderr_text
+        master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+        result3 = master.run_command(["ipa-replica-manage",
+                                      "connect",
+                                      replica1.hostname,
+                                      replica2.hostname],
+                                     raiseonerr=False)
+        assert_error(result3, 'Creation of IPA replication agreement is'
+                              ' deprecated with managed IPA replication'
+                              ' topology. Please use `ipa topologysegment-*`'
+                              ' commands to manage the topology', 1)
+        tasks.create_segment(master, replica1, replica2)
+        result4 = master.run_command(["ipa-replica-manage",
+                                      "disconnect",
+                                      replica1.hostname,
+                                      replica2.hostname],
+                                     raiseonerr=False)
+        assert_error(result4, 'Removal of IPA replication agreement is'
+                              ' deprecated with managed IPA replication'
+                              ' topology. Please use `ipa topologysegment-*`'
+                              ' commands to manage the topology', 1)
-- 
2.4.3

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