Re: [Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
On 01.03.2016 15:22, Milan Kubík wrote: On 02/19/2016 02:11 PM, Oleg Fayans wrote: Hi Milan, On 02/12/2016 04:03 PM, Milan Kubík wrote: Agreed. The latest patch gets rid of all resolv.conf related manipulations. The tests work (where not affected by https://fedorahosted.org/bind-dyndb-ldap/ticket/160) -- Milan Kubik Works for me, tested on sudo test that requires autodiscovery. ACK Pushed to: ipa-4-3: a8f53296f633a8c2a0f6a041dd1d4bec854d206e master: cfbb7769a70f4cac4bb6d6b7fe36116b43c830e7 -- 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
Re: [Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
On 02/19/2016 02:11 PM, Oleg Fayans wrote: Hi Milan, On 02/12/2016 04:03 PM, Milan Kubík wrote: Agreed. The latest patch gets rid of all resolv.conf related manipulations. The tests work (where not affected by https://fedorahosted.org/bind-dyndb-ldap/ticket/160) -- Milan Kubik Works for me, tested on sudo test that requires autodiscovery. ACK -- Milan Kubik -- 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
Re: [Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
Hey guys, could anyone review this please? On 02/19/2016 02:11 PM, Oleg Fayans wrote: > Hi Milan, > > On 02/12/2016 04:03 PM, Milan Kubík wrote: >> On 02/04/2016 08:49 AM, Oleg Fayans wrote: >>> Hi Petr, >>> >>> On 02/03/2016 02:19 PM, Petr Spacek wrote: On 3.2.2016 10:22, Oleg Fayans wrote: > Guys, can anyone take a look at this? The commit message does not explain why you are setting search path. >>> Fixed. >>> I have to say that I do not like touching resolv.conf, as stated many times earlier. Why the test has to reconfigure the host and cannot use values provided by the provisioning system? >>> This patch exactly removes this messing around with nameservers in >>> resolv.conf >>> It introduces the possibility to put ipa domain in the search directive >>> of resolv.conf so that we could test service autodiscovery during client >>> installation. >>> >>> >>> >> I just verified that the tampering with resolv.conf is not needed >> (libvirt and ovirt/rhev). I think this is an artifact from the whole >> issue of "let's use improvised domain names, what can go wrong" approach >> that was uncovered by the enforced DNS checks. I think we can defer the >> networking configuration to provisioning system. > > Agreed. The latest patch gets rid of all resolv.conf related > manipulations. The tests work (where not affected by > https://fedorahosted.org/bind-dyndb-ldap/ticket/160) > > >> >> -- >> Milan Kubik >> > > > -- Oleg Fayans Quality Engineer FreeIPA team RedHat. -- 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
Re: [Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
Hi Milan,
On 02/12/2016 04:03 PM, Milan Kubík wrote:
> On 02/04/2016 08:49 AM, Oleg Fayans wrote:
>> Hi Petr,
>>
>> On 02/03/2016 02:19 PM, Petr Spacek wrote:
>>> On 3.2.2016 10:22, Oleg Fayans wrote:
Guys, can anyone take a look at this?
>>> The commit message does not explain why you are setting search path.
>> Fixed.
>>
>>> I have to say that I do not like touching resolv.conf, as stated many times
>>> earlier. Why the test has to reconfigure the host and cannot use values
>>> provided by the provisioning system?
>> This patch exactly removes this messing around with nameservers in
>> resolv.conf
>> It introduces the possibility to put ipa domain in the search directive
>> of resolv.conf so that we could test service autodiscovery during client
>> installation.
>>
>>
>>
> I just verified that the tampering with resolv.conf is not needed
> (libvirt and ovirt/rhev). I think this is an artifact from the whole
> issue of "let's use improvised domain names, what can go wrong" approach
> that was uncovered by the enforced DNS checks. I think we can defer the
> networking configuration to provisioning system.
Agreed. The latest patch gets rid of all resolv.conf related
manipulations. The tests work (where not affected by
https://fedorahosted.org/bind-dyndb-ldap/ticket/160)
>
> --
> Milan Kubik
>
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 073e898731685fddef1768b3aeff6f68b801a48c Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Fri, 19 Feb 2016 13:46:10 +0100
Subject: [PATCH] Removed messing around with resolv.conf
---
ipatests/test_integration/tasks.py | 44 ++
1 file changed, 2 insertions(+), 42 deletions(-)
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index d37b616bd6efe437a1a979cc7a9ad8c7ea803773..e859cfc8501eec32fb2233960dd8d7fa64be76d1 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -44,8 +44,6 @@ from ipalib.constants import DOMAIN_LEVEL_0
log = log_mgr.get_logger(__name__)
-IPATEST_NM_CONFIG = '20-ipatest-unmanaged-resolv.conf'
-
def check_arguments_are(slice, instanceof):
"""
@@ -91,12 +89,9 @@ def allow_sync_ptr(host):
raiseonerr=False)
-def apply_common_fixes(host, fix_resolv=True):
+def apply_common_fixes(host):
fix_etc_hosts(host)
fix_hostname(host)
-modify_nm_resolv_conf_settings(host)
-if fix_resolv:
-fix_resolv_conf(host)
prepare_host(host)
@@ -154,40 +149,6 @@ def host_service_active(host, service):
return False
-def modify_nm_resolv_conf_settings(host):
-if not host_service_active(host, 'NetworkManager'):
-return
-
-config = "[main]\ndns=none\n"
-path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
-
-host.put_file_contents(path, config)
-host.run_command(['systemctl', 'restart', 'NetworkManager'],
- raiseonerr=False)
-
-
-def undo_nm_resolv_conf_settings(host):
-if not host_service_active(host, 'NetworkManager'):
-return
-
-path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
-host.run_command(['rm', '-f', path], raiseonerr=False)
-host.run_command(['systemctl', 'restart', 'NetworkManager'],
- raiseonerr=False)
-
-
-def fix_resolv_conf(host):
-backup_file(host, paths.RESOLV_CONF)
-lines = host.get_file_contents(paths.RESOLV_CONF).splitlines()
-lines = ['#' + l if l.startswith('nameserver') else l for l in lines]
-for other_host in host.domain.hosts:
-if other_host.role in ('master', 'replica'):
-lines.append('nameserver %s' % other_host.ip)
-contents = '\n'.join(lines)
-log.debug('Writing the following to /etc/resolv.conf:\n%s', contents)
-host.put_file_contents(paths.RESOLV_CONF, contents)
-
-
def fix_apache_semaphores(master):
systemd_available = master.transport.file_exists(paths.SYSTEMCTL)
@@ -204,7 +165,6 @@ def fix_apache_semaphores(master):
def unapply_fixes(host):
restore_files(host)
restore_hostname(host)
-undo_nm_resolv_conf_settings(host)
# Clean up the test directory
host.run_command(['rm', '-rvf', host.config.test_dir])
@@ -269,7 +229,7 @@ def install_master(host, setup_dns=True, setup_kra=False):
host.collect_log(paths.SLAPD_INSTANCE_ERROR_LOG_TEMPLATE % inst)
host.collect_log(paths.SLAPD_INSTANCE_ACCESS_LOG_TEMPLATE % inst)
-apply_common_fixes(host, fix_resolv=False)
+apply_common_fixes(host)
fix_apache_semaphores(host)
args = [
--
1.8.3.1
--
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
Re: [Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
On 02/04/2016 08:49 AM, Oleg Fayans wrote: Hi Petr, On 02/03/2016 02:19 PM, Petr Spacek wrote: On 3.2.2016 10:22, Oleg Fayans wrote: Guys, can anyone take a look at this? The commit message does not explain why you are setting search path. Fixed. I have to say that I do not like touching resolv.conf, as stated many times earlier. Why the test has to reconfigure the host and cannot use values provided by the provisioning system? This patch exactly removes this messing around with nameservers in resolv.conf It introduces the possibility to put ipa domain in the search directive of resolv.conf so that we could test service autodiscovery during client installation. I just verified that the tampering with resolv.conf is not needed (libvirt and ovirt/rhev). I think this is an artifact from the whole issue of "let's use improvised domain names, what can go wrong" approach that was uncovered by the enforced DNS checks. I think we can defer the networking configuration to provisioning system. -- Milan Kubik -- 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
Re: [Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
Hi Petr,
On 02/03/2016 02:19 PM, Petr Spacek wrote:
> On 3.2.2016 10:22, Oleg Fayans wrote:
>> Guys, can anyone take a look at this?
>
> The commit message does not explain why you are setting search path.
Fixed.
>
> I have to say that I do not like touching resolv.conf, as stated many times
> earlier. Why the test has to reconfigure the host and cannot use values
> provided by the provisioning system?
This patch exactly removes this messing around with nameservers in
resolv.conf
It introduces the possibility to put ipa domain in the search directive
of resolv.conf so that we could test service autodiscovery during client
installation.
>
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 9d11bc937adb425f504de345504f01f81714353b Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Thu, 4 Feb 2016 08:45:03 +0100
Subject: [PATCH] Moved NM configuration calls to the IntegrationTest base
class
Reconfiguring and restarting of NetworkManager in the middle of test execution
sometimes causes unexpected results. See [1] for details.
It is better to have it configured in advance during test class initialization
with default configuration applied at test teardown
Also, disabled manual resetting nameservers in resolv.conf before replica and
client installations (as per discussion with pspacek). Added possibility to
update the "search" directive in resolv.conf to test service auto-discovery
during client installation.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1303095
---
ipatests/test_integration/base.py | 5 +
ipatests/test_integration/tasks.py | 11 +++
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/ipatests/test_integration/base.py b/ipatests/test_integration/base.py
index 4f57e959032a5fda0ad002fca95501da1160605b..fef202b8e203806f711de4062b1a429482c411bb 100644
--- a/ipatests/test_integration/base.py
+++ b/ipatests/test_integration/base.py
@@ -64,6 +64,9 @@ class IntegrationTest(object):
def install(cls, mh):
if cls.topology is None:
return
+for host in cls.get_all_hosts():
+# Tell NetworkManager to not mess around with resolv.conf
+tasks.modify_nm_resolv_conf_settings(host)
else:
tasks.install_topo(cls.topology,
cls.master, cls.replicas, cls.clients)
@@ -78,6 +81,8 @@ class IntegrationTest(object):
tasks.uninstall_master(replica)
for client in cls.clients:
tasks.uninstall_client(client)
+for host in cls.get_all_hosts():
+tasks.undo_nm_resolv_conf_settings(host)
IntegrationTest.log = log_mgr.get_logger(IntegrationTest())
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 318c8c880fe0df22c9b5089cbd88e017936e4df5..42d2e109c7b91cf8f5ca47c5028c80aa7a927db1 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -91,7 +91,6 @@ def allow_sync_ptr(host):
def apply_common_fixes(host, fix_resolv=True):
fix_etc_hosts(host)
fix_hostname(host)
-modify_nm_resolv_conf_settings(host)
if fix_resolv:
fix_resolv_conf(host)
prepare_host(host)
@@ -175,11 +174,9 @@ def undo_nm_resolv_conf_settings(host):
def fix_resolv_conf(host):
backup_file(host, paths.RESOLV_CONF)
-lines = host.get_file_contents(paths.RESOLV_CONF).splitlines()
-lines = ['#' + l if l.startswith('nameserver') else l for l in lines]
-for other_host in host.domain.hosts:
-if other_host.role in ('master', 'replica'):
-lines.append('nameserver %s' % other_host.ip)
+lines_read = host.get_file_contents(paths.RESOLV_CONF).splitlines()
+lines = ['#' + l if l.startswith('search') else l for l in lines_read]
+lines[0] = "search %s\n" % host.domain.name
contents = '\n'.join(lines)
log.debug('Writing the following to /etc/resolv.conf:\n%s', contents)
host.put_file_contents(paths.RESOLV_CONF, contents)
@@ -201,8 +198,6 @@ def fix_apache_semaphores(master):
def unapply_fixes(host):
restore_files(host)
restore_hostname(host)
-undo_nm_resolv_conf_settings(host)
-
# Clean up the test directory
host.run_command(['rm', '-rvf', host.config.test_dir])
--
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
Re: [Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
On 3.2.2016 10:22, Oleg Fayans wrote: > Guys, can anyone take a look at this? The commit message does not explain why you are setting search path. I have to say that I do not like touching resolv.conf, as stated many times earlier. Why the test has to reconfigure the host and cannot use values provided by the provisioning system? -- Petr^2 Spacek -- 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
Re: [Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
Guys, can anyone take a look at this? On 01/29/2016 04:09 PM, Oleg Fayans wrote: > > > -- Oleg Fayans Quality Engineer FreeIPA team RedHat. -- 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
[Freeipa-devel] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095
--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From b866ff5336f742e170895b575df7a09419c2d731 Mon Sep 17 00:00:00 2001
From: Oleg Fayans
Date: Fri, 29 Jan 2016 16:04:17 +0100
Subject: [PATCH] Moved NM configuration calls to the IntegrationTest base
class
Reconfiguring and restarting of NetworkManager in the middle of test execution
sometimes causes unexpected results. See [1] for details.
It is better to have it configured in advance during test class initialization
with default configuration applied at test teardown
Also, disabled manual resetting nameservers in resolv.conf before replica and
client installations (as per discussion with pspacek).
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1303095
---
ipatests/test_integration/base.py | 5 +
ipatests/test_integration/tasks.py | 11 +++
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/ipatests/test_integration/base.py b/ipatests/test_integration/base.py
index 4f57e959032a5fda0ad002fca95501da1160605b..fef202b8e203806f711de4062b1a429482c411bb 100644
--- a/ipatests/test_integration/base.py
+++ b/ipatests/test_integration/base.py
@@ -64,6 +64,9 @@ class IntegrationTest(object):
def install(cls, mh):
if cls.topology is None:
return
+for host in cls.get_all_hosts():
+# Tell NetworkManager to not mess around with resolv.conf
+tasks.modify_nm_resolv_conf_settings(host)
else:
tasks.install_topo(cls.topology,
cls.master, cls.replicas, cls.clients)
@@ -78,6 +81,8 @@ class IntegrationTest(object):
tasks.uninstall_master(replica)
for client in cls.clients:
tasks.uninstall_client(client)
+for host in cls.get_all_hosts():
+tasks.undo_nm_resolv_conf_settings(host)
IntegrationTest.log = log_mgr.get_logger(IntegrationTest())
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 27d2449ec71e0bf55a576cc495175a5ae41da1d6..b5e3d1a8106d389e2027fb2535726b3a93eb2d5c 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -91,7 +91,6 @@ def allow_sync_ptr(host):
def apply_common_fixes(host, fix_resolv=True):
fix_etc_hosts(host)
fix_hostname(host)
-modify_nm_resolv_conf_settings(host)
if fix_resolv:
fix_resolv_conf(host)
prepare_host(host)
@@ -175,11 +174,9 @@ def undo_nm_resolv_conf_settings(host):
def fix_resolv_conf(host):
backup_file(host, paths.RESOLV_CONF)
-lines = host.get_file_contents(paths.RESOLV_CONF).splitlines()
-lines = ['#' + l if l.startswith('nameserver') else l for l in lines]
-for other_host in host.domain.hosts:
-if other_host.role in ('master', 'replica'):
-lines.append('nameserver %s' % other_host.ip)
+lines_read = host.get_file_contents(paths.RESOLV_CONF).splitlines()
+lines = ['#' + l if l.startswith('search') else l for l in lines_read]
+lines[0] = "search %s\n" % host.domain.name
contents = '\n'.join(lines)
log.debug('Writing the following to /etc/resolv.conf:\n%s', contents)
host.put_file_contents(paths.RESOLV_CONF, contents)
@@ -201,8 +198,6 @@ def fix_apache_semaphores(master):
def unapply_fixes(host):
restore_files(host)
restore_hostname(host)
-undo_nm_resolv_conf_settings(host)
-
# Clean up the test directory
host.run_command(['rm', '-rvf', host.config.test_dir])
--
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
