On 10.8.2016 08:21, Jan Cholasta wrote:
> On 1.8.2016 17:42, Petr Spacek wrote:
>> On 1.8.2016 08:27, Jan Cholasta wrote:
>>> On 28.7.2016 16:55, Petr Spacek wrote:
>>>> On 28.7.2016 16:44, Jan Cholasta wrote:
>>>>> On 28.7.2016 16:37, Petr Spacek wrote:
>>>>>> On 28.7.2016 16:35, Jan Cholasta wrote:
>>>>>>> On 28.7.2016 16:20, Petr Spacek wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> install: Call hostnamectl set-hostname only if --hostname option is 
>>>>>>>> used
>>>>>>>>
>>>>>>>> This commit also splits hostname backup and configuration into two
>>>>>>>> separate
>>>>>>>> functions. This allows us to backup hostname without setting it at the
>>>>>>>> same time.
>>>>>>>>
>>>>>>>> https://fedorahosted.org/freeipa/ticket/6071
>>>>>>>
>>>>>>> Note that you can set ca_host in cfg unconditionally, the value is only
>>>>>>> valid
>>>>>>> during install and is not written anywhere.
>>>>>>
>>>>>> I prefer not to set it so the code blows up when we attempt to touch
>>>>>> variables
>>>>>> we should reference in particular setups. I.e. Take this as a first step
>>>>>> towards api.env without invalid values :-)
>>>>>
>>>>> OK. Use the proper condition then ("if setup_ca:").
>>>>
>>>> Oh, I'm probably blind. Here is revised version.
>>>
>>> But you used "if not setup_ca:" rather than "if setup_ca:" :-)
>>
>> This patch set is cursed!
> 
> The host name should not be backed up in server install if we are not changing
> it, so that it's not attempted to be changed on uninstall.
> 
> Otherwise works for me.

Okay, now I see that ipa-backup/restore is storing the host name in backup
header so we do not need it in sysrestore.

New patches are attached.

-- 
Petr^2 Spacek
From 96010d6f9d480c6eac39816f449bc647fd6b36c1 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Tue, 12 Jul 2016 17:42:40 +0200
Subject: [PATCH] server-install: Fix --hostname option to always override
 api.env values

Attempts to compare local hostname with user-provided values are error
prone as we found out in #5794. This patch removes comparison and makes
the env values deterministic.

https://fedorahosted.org/freeipa/ticket/6071
---
 ipaserver/install/server/install.py | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 65f9318201e648b30a3c13626e807ac6f3a9416d..1e925595b93ff95a98b3e6c3d0c357b1766dc1dc 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -560,7 +560,12 @@ def install_check(installer):
     cfg = dict(
         context='installer',
         in_server=True,
+        # make sure host name specified by user is used instead of default
+        host=host_name,
     )
+    if setup_ca:
+        # we have an IPA-integrated CA
+        cfg['ca_host'] = host_name
 
     # Create the management framework config file and finalize api
     target_fname = paths.IPA_DEFAULT_CONF
@@ -586,14 +591,6 @@ def install_check(installer):
     # Must be readable for everyone
     os.chmod(target_fname, 0o644)
 
-    system_hostname = get_fqdn()
-    if host_name != system_hostname:
-        root_logger.debug("Chosen hostname (%s) differs from system hostname "
-                          "(%s) - change it" % (host_name, system_hostname))
-        # update `api.env.ca_host` to correct hostname
-        # https://fedorahosted.org/freeipa/ticket/4936
-        api.env.ca_host = host_name
-
     api.bootstrap(**cfg)
     api.finalize()
 
-- 
2.7.4

From d546592834ea7b08908acf66d668c05203734c76 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Thu, 28 Jul 2016 16:13:55 +0200
Subject: [PATCH] install: Call hostnamectl set-hostname only if --hostname
 option is used

This commit also splits hostname backup and configuration into two separate
functions. This allows us to backup hostname without setting it at the
same time.

https://fedorahosted.org/freeipa/ticket/6071
---
 client/ipa-client-install           |  3 ++-
 doc/guide/guide.org                 | 10 +++++-----
 ipaplatform/base/tasks.py           |  7 ++-----
 ipaplatform/redhat/tasks.py         | 13 ++-----------
 ipaserver/install/server/install.py | 10 +++++-----
 5 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 05b6b6e0da07353750d0dca4e6df9d1f58d69c35..4a263b3d0e25960fc57d198e3efb24447cbed98e 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -2525,7 +2525,8 @@ def install(options, env, fstore, statestore):
     if options.hostname and not options.on_master:
         # skip this step when run by ipa-server-install as it always configures
         # hostname
-        tasks.backup_and_replace_hostname(fstore, statestore, options.hostname)
+        tasks.backup_hostname(fstore, statestore)
+        tasks.set_hostname(options.hostname)
 
     ntp_srv_servers = []
     if not options.on_master and options.conf_ntp:
diff --git a/doc/guide/guide.org b/doc/guide/guide.org
index 6d181559f0af90e7be7089aa94ab4900fa4e90b5..2e852a964991781ef5dd7b93ac481891897e1ed0 100644
--- a/doc/guide/guide.org
+++ b/doc/guide/guide.org
@@ -1039,14 +1039,14 @@ def restore_context_default(filepath):
 # version in platform services
 restore_context = restore_context_default
 
-# Default implementation of backup and replace hostname that does nothing
-def backup_and_replace_hostname_default(fstore, statestore, hostname):
+# Default implementation of backup hostname that does nothing
+def backup_hostname_default(fstore, statestore):
     return
 
-# Backup and replace system's hostname
-# Since many platforms have their own way how to store system's hostname, this method must be
+# Backup system's hostname
+# Since many platforms have their own way of handling system's hostname, this method must be
 # implemented in platform services
-backup_and_replace_hostname = backup_and_replace_hostname_default
+backup_hostname = backup_hostname_default
 
 from ipapython.platform.SUPPORTED_PLATFORM import *
 #+END_SRC
diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index c6860ce47ad3d043f1561a690c401537f5e2fdb7..1e687b6181fcff20303b50ac18bfde66280f8bfd 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -45,14 +45,11 @@ class BaseTaskNamespace(object):
 
         raise NotImplementedError()
 
-    def backup_and_replace_hostname(self, fstore, statestore, hostname):
+    def backup_hostname(self, fstore, statestore):
         """
         Backs up the current hostname in the statestore (so that it can be
         restored by the restore_hostname platform task).
 
-        Makes sure that new hostname (passed via hostname argument) is set
-        as a new pemanent hostname for this host.
-
         No return value expected.
         """
 
@@ -109,7 +106,7 @@ class BaseTaskNamespace(object):
     def restore_hostname(self, fstore, statestore):
         """
         Restores the original hostname as backed up in the
-        backup_and_replace_hostname platform task.
+        backup_hostname platform task.
         """
 
         raise NotImplementedError()
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 8ac88511e94d640f077c7a0e202bc545ec8bcbbe..dbe005abb0ecbcb398368789fee52895c6d6e980 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -332,22 +332,13 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 
         return result
 
-    def backup_and_replace_hostname(self, fstore, statestore, hostname):
-        old_hostname = socket.gethostname()
-        try:
-            self.set_hostname(hostname)
-        except ipautil.CalledProcessError as e:
-            root_logger.debug(traceback.format_exc())
-            root_logger.error(
-                "Failed to set this machine hostname to %s (%s).",
-                old_hostname, e
-            )
-
+    def backup_hostname(self, fstore, statestore):
         filepath = paths.ETC_HOSTNAME
         if os.path.exists(filepath):
             fstore.backup_file(filepath)
 
         # store old hostname
+        old_hostname = socket.gethostname()
         statestore.backup_state('network', 'hostname', old_hostname)
 
     def restore_hostname(self, fstore, statestore):
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 1e925595b93ff95a98b3e6c3d0c357b1766dc1dc..94698898934844350488d5fc52d6e1e567624502 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -651,6 +651,7 @@ def install_check(installer):
     options.dm_password = dm_password
     options.master_password = master_password
     options.admin_password = admin_password
+    options._host_name_overridden = bool(options.host_name)
     options.host_name = host_name
     options.ip_addresses = ip_addresses
 
@@ -702,11 +703,10 @@ def install(installer):
         print("Please wait until the prompt is returned.")
         print("")
 
-    # configure /etc/sysconfig/network to contain the custom hostname
-    tasks.backup_and_replace_hostname(fstore, sstore, host_name)
-
-    # set hostname (we need both transient and static)
-    tasks.set_hostname(host_name)
+    # set hostname (transient and static) if user instructed us to do so
+    if options._host_name_overridden:
+        tasks.backup_hostname(fstore, sstore)
+        tasks.set_hostname(host_name)
 
     if installer._update_hosts_file:
         update_hosts_file(ip_addresses, host_name, fstore)
-- 
2.7.4

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