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.
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From cb6e8f22c333c05164461496bda8f6b52d5ad076 Mon Sep 17 00:00:00 2001
From: Oleg Fayans <ofay...@redhat.com>
Date: Fri, 27 Nov 2015 16:17:51 +0100
Subject: [PATCH] Prepared integration tests for replica promotion
---
.../test_integration/test_replica_promotion.py | 200 +++++++++++++++++++++
1 file changed, 200 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..504af0513cf795649ead3d4228780059b453c5a2
--- /dev/null
+++ b/ipatests/test_integration/test_replica_promotion.py
@@ -0,0 +1,200 @@
+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
+
+
+class Dummy(IntegrationTest):
+ config = get_global_config()
+ kra_install = ["ipa-kra-install", "-U", "-p", config.dirman_password]
+ kra_uninstall = ["ipa-kra-install", "--uninstall", "-U"]
+
+
+ @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, cleanhost=True)
+
+ def test_kra_install_master(self):
+ result1 = tasks.install_kra(self.master, raiseonerr=False)
+ assert result1.returncode == 0, result1.stderr_text
+ tasks.kinit_admin(self.master)
+ result2 = self.master.run_command(["ipa", "vault-find"],
+ raiseonerr=False)
+ assert(result2.stdout_text.find("0 vaults matched") > 0), result2.stdout_text
+
+
+class TestReplicaPromotionLevel0(Dummy):
+
+ topology = 'line'
+ 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", "realm"]
+ 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)
+ assert(result.stdout_text.rfind("1 segment matched") > 0), result.stdout_text
+ tasks.ipa_restore(self.master, backup_file)
+ result2 = self.master.run_command(command, raiseonerr=False)
+ assert(result2.stdout_text.rfind("0 segments matched") > 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 = 'line'
+ num_clients = 0
+ domain_level = DOMAIN_LEVEL_0
+ num_replicas = 2
+ kra_install = ["ipa-kra-install", "-U", "-p", config.dirman_password]
+
+ def test_kra_install_without_replica_file(self):
+ self.master.run_command(self.kra_install)
+ replica1 = self.replicas[0]
+ replica2 = self.replicas[1]
+ self.replica1_filename = tasks.get_replica_filename(replica1)
+ self.replica2_filename = tasks.get_replica_filename(replica2)
+ result1 = replica1.run_command(self.kra_install, raiseonerr=False)
+ assert_error(result1, "A replica file is required", 1)
+ replica1.run_command(self.kra_install + [self.replica1_filename])
+ # Now prepare the replica file, copy it to the client and raise
+ # domain level on master to test the reverse situation
+ self.master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+ result2 = replica2.run_command(self.kra_install + [self.replica2_filename],
+ raiseonerr=False)
+ assert_error(result2, "supported only in 0-level IPA domain", 1)
+ replica2.run_command(self.kra_install)
+
+
+class TestCAInstall(IntegrationTest):
+ config = get_global_config()
+ topology = 'line'
+ num_clients = 0
+ domain_level = DOMAIN_LEVEL_0
+ num_replicas = 2
+ ca_install = ["ipa-ca-install", "-U", "-p", config.dirman_password]
+
+ @classmethod
+ def install(cls, mh):
+ tasks.install_master(cls.master)
+
+ 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
+ """
+ for i in self.replicas:
+ tasks.install_replica(self.master, i, setup_ca=False, setup_dns=True)
+ replica1 = self.replicas[0]
+ replica2 = self.replicas[1]
+ self.replica1_filename = tasks.get_replica_filename(replica1)
+ self.replica2_filename = tasks.get_replica_filename(replica2)
+ result1 = replica1.run_command(self.ca_install, raiseonerr=False)
+ assert_error(result1, "A replica file is required", 1)
+ replica1.run_command(self.ca_install + [self.replica1_filename])
+ # Now prepare the replica file, copy it to the client and raise
+ # domain level on master to test the reverse situation
+ self.master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+ result2 = replica2.run_command(self.ca_install + [self.replica2_filename],
+ raiseonerr=False)
+ assert_error(result2, "supported only in 0-level IPA domain", 1)
+ replica2.run_command(self.ca_install)
+
+
+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 = 'line'
+ 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, '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
+ """
+
+ result1 = self.master.run_command(["ipa-replica-manage",
+ "connect",
+ self.replicas[0].hostname,
+ self.replicas[1].hostname],
+ raiseonerr=False)
+ assert result1.returncode == 0, result1.stderr_text
+ result2 = self.master.run_command(["ipa-replica-manage",
+ "disconnect",
+ self.replicas[0].hostname,
+ self.replicas[1].hostname],
+ raiseonerr=False)
+ assert result2.returncode == 0, result2.stderr_text
+ self.master.run_command(["ipa", "domainlevel-set", str(DOMAIN_LEVEL_1)])
+ result3 = self.master.run_command(["ipa-replica-manage",
+ "connect",
+ self.replicas[0].hostname,
+ self.replicas[1].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(self.master, self.replicas[0], self.replicas[1])
+ result4 = self.master.run_command(["ipa-replica-manage",
+ "disconnect",
+ self.replicas[0].hostname,
+ self.replicas[1].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
