Merge lp:~cprov/core-image-watcher/proper-hostname into lp:core-image-watcher

[Celso Providelo]

Using /etc/hostname content directly for identifying service workers.

Using /etc/hostname content directly for identifying service workers.

It's required because juju-generated hostnames get too big due to our long environment names (<spec_name>-<MD5>).

Those names are to long to be qualified as proper hostnames and are not used in /etc/hosts, 'ubuntu' is used instead (for all nodes).

[Para Siva (psivaa)]

+1.

review: Approve

[Evan (ev)]

Don't let me block this from landing, but can you elaborate on this a bit? Where are we hitting a limit on the length of the hostname? In Logstash?

[Celso Providelo (cprov)]

Evan,

No, it's not Logstash or anything from the late 20th century, but
instead DNS label size, which is limited to 64 chars [1].

Linux seems to take it serious on /usr/include/linux/utsname.h and all
the other tools around it seems to assume that we should not have
labels larger than 64 chars.

I claim ignorance at this point ...

[1] http://www.ietf.org/rfc/rfc1035.txt

On Tue, Mar 31, 2015 at 6:34 AM, Evan Dandrea
<email address hidden> wrote:
> Don't let me block this from landing, but can you elaborate on this a bit? Where are we hitting a limit on the length of the hostname? In Logstash?
> --
> https://code.launchpad.net/~cprov/core-image-watcher/proper-hostname/+merge/254699
> You are the owner of lp:~cprov/core-image-watcher/proper-hostname.

--
Celso Providelo
<email address hidden>

[lp:~cprov/core-image-watcher/proper-hostname]

11. By Celso Providelo on 2015-03-31

Adding 'service' key to the logging extra information, so we can filter this properly in logstash.

[Evan (ev)]

I think the rationale for this should be documented in the code, but it otherwise looks good. Celso explained on IRC the 64 character limit for hostnames and how this lets us avoid a truncated or "ubuntu" hostname in the logs.

review: Approve

[lp:~cprov/core-image-watcher/proper-hostname]

12. By Celso Providelo on 2015-03-31

Adding a rationale for digging into /etc/hostname.

[Celso Providelo (cprov)]

Evan,

Rationale added in the docstring, thanks for pointing it.

Another blessing, please ?

[Francis Ginther (fginther)]

Thanks for the explanation. Also nice that this is easily migrated to other projects.

review: Approve

[Para Siva (psivaa)]

One inline question about 'SERVICE_NAME' in constants.py. Apart from that,+1.

review: Approve

[lp:~cprov/core-image-watcher/proper-hostname]

13. By Celso Providelo on 2015-03-31

Logging 'solution' & 'service' information.

=== modified file 'core_image_watcher/__init__.py'
--- core_image_watcher/__init__.py	2015-03-28 11:58:10 +0000
+++ core_image_watcher/__init__.py	2015-03-31 15:43:19 +0000
@@ -29,23 +29,33 @@
 import logstash
 
 
+from core_image_watcher import constants
+
+
 logger = logging.getLogger(__name__)
 
-API_VERSION = "v1"
+
+logging_extra = {
+    'solution': constants.SOLUTION_NAME,
+    'service': constants.SERVICE_NAME,
+    'hostname': constants.HOSTNAME,
+}
 
 
 def _enqueue(body, amqp_uris):
     """Enqueues a message of `body` to the rabbit queue"""
+    extra = logging_extra.copy()
+    extra.update(body)
     with kombu.Connection(amqp_uris) as connection:
-        logger.info('Triggering request: %s', body, extra=body)
+        logger.info('Triggering request: %s', body, extra=extra)
         try:
             queue = connection.SimpleQueue(
-                'core.image.{}'.format(API_VERSION))
+                'core.image.{}'.format(constants.API_VERSION))
             queue.put(body)
             queue.close()
         except Exception as exc:
-            logger.error(exc, exc_info=True, extra=body)
-        logger.info('Done!', extra=body)
+            logger.error(exc, exc_info=True, extra=extra)
+        logger.info('Done!', extra=extra)
 
 
 def _get_version_string_output(channel, device):
@@ -61,7 +71,7 @@
     try:
         images = subprocess.check_output(cmd)
     except subprocess.CalledProcessError as e:
-        logger.error(e, exc_info=True)
+        logger.error(e, exc_info=True, extra=logging_extra)
     finally:
         return images
 
@@ -74,7 +84,8 @@
     if (len(images) >= 2):
         return images[-2].decode('utf-8').replace(':', '')
 
-    logger.error('Could not get the image list by u-d-f: %s', images)
+    logger.error('Could not get the image list by u-d-f: %s',
+                 images, extra=logging_extra)
     return None
 
 
@@ -93,7 +104,8 @@
                 f.write(latest_version)
     except IOError as e:
         latest_version = None
-        logger.error('Writing the latest image info failed: %s', str(e))
+        logger.error('Writing the latest image info failed: %s',
+                     str(e), extra=logging_extra)
     finally:
         return latest_version
 
@@ -112,7 +124,7 @@
             # Do not progress
             return None
     except Exception as e:
-        logger.error(e, exc_info=True)
+        logger.error(e, exc_info=True, extra=logging_extra)
         return None
     body = {
         'image_name': latest_version,

=== added file 'core_image_watcher/constants.py'
--- core_image_watcher/constants.py	1970-01-01 00:00:00 +0000
+++ core_image_watcher/constants.py	2015-03-31 15:43:19 +0000
@@ -0,0 +1,47 @@
+# core-image-watcher
+# Copyright (C) 2015 Canonical
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+"""Constants for this service."""
+
+
+API_VERSION = "v1"
+
+SOLUTION_NAME = "core-image-testing"
+
+SERVICE_NAME = "core-image-watcher"
+
+def _get_hostname():
+    """Return sanitized contents of /etc/hostname.
+
+    It is necessary because current juju hostnames (juju-<env-name>-machine-#)
+    are too big due to our long environment names ('<spec_name>-<MD5>').
+    Linux (DNS for RFC1035, really) only supports labels up to 64 chars and
+    the fallback varies from tool to tool, `cloud-init` chokes on longer
+    names and sets 'ubuntu', `hostnamectl` (systemd) would truncate the
+    given data. 
+
+    None of this is ideal to our applications, that's why we will operate
+    on the pristine /etc/hostname and remove the 'juju-' and '-machine' terms
+    added by juju.
+    """
+    with open('/etc/hostname') as fd:
+        hostname = fd.read()
+    return hostname.replace('juju-', '').replace('-machine', '').strip()
+
+
+HOSTNAME = _get_hostname()
+