Merge ~blake-rouse/maas:remote-syslog into maas:master

[Blake Rouse]

Add ability for an administrator to configure a remote syslog server.

[Mike Pontillo (mpontillo)]

Looks good to me!

One suggestion: I would just make it clearer in the help text how to set the syslog settings back to the default, and what the default behavior actually is.

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b remote-syslog lp:~blake-rouse/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 54cc56121a328004c6cd12b7ba04d116d92a5037

review: Approve

b67e2f7... by Blake Rouse on 2018-09-04

Add message about reseting back to default.

diff --git a/src/maasserver/compose_preseed.py b/src/maasserver/compose_preseed.py
index f693ed0..1441a85 100644
--- a/src/maasserver/compose_preseed.py
+++ b/src/maasserver/compose_preseed.py
@@ -266,7 +266,11 @@ def get_cloud_init_reporting(request, node, token):
 
 def get_rsyslog_host_port(request, node):
     """Return the rsyslog host and port to use."""
-    return "%s:%d" % (node.boot_cluster_ip, RSYSLOG_PORT)
+    syslog = Config.objects.get_config('remote_syslog')
+    if syslog:
+        return syslog
+    else:
+        return "%s:%d" % (node.boot_cluster_ip, RSYSLOG_PORT)
 
 
 def get_system_info():
diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index 94e52c6..69440fa 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -1519,6 +1519,11 @@ class NTPForm(ConfigForm):
     ntp_external_only = get_config_field('ntp_external_only')
 
 
+class SyslogForm(ConfigForm):
+    """Settings page, Syslog section."""
+    remote_syslog = get_config_field('remote_syslog')
+
+
 class NetworkDiscoveryForm(ConfigForm):
     """Settings page, Network Discovery section."""
     network_discovery = get_config_field('network_discovery')
diff --git a/src/maasserver/forms/settings.py b/src/maasserver/forms/settings.py
index a6ce778..d99152a 100644
--- a/src/maasserver/forms/settings.py
+++ b/src/maasserver/forms/settings.py
@@ -42,6 +42,7 @@ from maasserver.utils.osystems import (
     release_a_newer_than_b,
 )
 from provisioningserver.utils.text import normalise_whitespace
+from provisioningserver.utils.url import splithost
 
 
 INVALID_URL_MESSAGE = "Enter a valid url (e.g. http://host.example.com)."
@@ -231,6 +232,21 @@ def make_maas_internal_domain_field(*args, **kwargs):
         **kwargs)
 
 
+class RemoteSyslogField(forms.CharField):
+    """
+    A `CharField` that formats the input into the expected value for syslog.
+    """
+
+    def clean(self, value):
+        value = super(RemoteSyslogField, self).clean(value)
+        if not value:
+            return None
+        host, port = splithost(value)
+        if not port:
+            port = 514
+        return '%s:%d' % (host, port)
+
+
 CONFIG_ITEMS = {
     'maas_name': {
         'default': gethostname(),
@@ -405,6 +421,20 @@ CONFIG_ITEMS = {
             """),
         }
     },
+    'remote_syslog': {
+        'default': None,
+        'form': RemoteSyslogField,
+        'form_kwargs': {
+            'label': "Remote syslog server to forward machine logs",
+            'required': False,
+            'help_text': normalise_whitespace("""\
+                A remote syslog server that MAAS will set on enlisting,
+                commissioning, testing, and deploying machines to send all
+                log messages. Clearing this value will restore the default
+                behaviour of forwarding syslog to MAAS.
+            """),
+        }
+    },
     'network_discovery': {
         'default': 'enabled',
         'form': make_network_discovery_field,
diff --git a/src/maasserver/forms/tests/test_settings.py b/src/maasserver/forms/tests/test_settings.py
index a8804d9..ed8368f 100644
--- a/src/maasserver/forms/tests/test_settings.py
+++ b/src/maasserver/forms/tests/test_settings.py
@@ -65,3 +65,18 @@ class TestSpecificConfigSettings(MAASServerTestCase):
         ips2 = [factory.make_ip_address() for _ in range(3)]
         input = ' '.join(ips1) + ' ' + ','.join(ips2)
         self.assertEqual(' '.join(ips1 + ips2), field.clean(input))
+
+
+class TestRemoteSyslogConfigSettings(MAASServerTestCase):
+
+    def test_sets_empty_to_none(self):
+        field = get_config_field('remote_syslog')
+        self.assertIsNone(field.clean('   '))
+
+    def test_adds_port(self):
+        field = get_config_field('remote_syslog')
+        self.assertEqual('192.168.1.1:514', field.clean('192.168.1.1'))
+
+    def test_wraps_ipv6(self):
+        field = get_config_field('remote_syslog')
+        self.assertEqual('[::ffff]:514', field.clean('::ffff'))
diff --git a/src/maasserver/models/config.py b/src/maasserver/models/config.py
index 9a0d6c2..8551a81 100644
--- a/src/maasserver/models/config.py
+++ b/src/maasserver/models/config.py
@@ -84,6 +84,8 @@ def get_default_config():
         'ntp_servers': 'ntp.ubuntu.com',
         'ntp_external_only': False,
         'omapi_key': '',
+        # Syslog settings
+        'remote_syslog': None,
         # Network discovery.
         'network_discovery': 'enabled',
         'active_discovery_interval': int(timedelta(hours=3).total_seconds()),
diff --git a/src/maasserver/preseed.py b/src/maasserver/preseed.py
index 3056f00..e795f96 100644
--- a/src/maasserver/preseed.py
+++ b/src/maasserver/preseed.py
@@ -753,12 +753,15 @@ def get_preseed_context(
         rack_controller=rack_controller, default_region_ip=region_ip)
     server_url = request.build_absolute_uri(reverse('machines_handler'))
     metadata_enlist_url = request.build_absolute_uri(reverse('enlist'))
+    syslog = Config.objects.get_config('remote_syslog')
+    if not syslog:
+        syslog = '%s:%d' % (server_host, RSYSLOG_PORT)
     return {
         'osystem': osystem,
         'release': release,
         'server_host': server_host,
         'server_url': server_url,
-        'syslog_host_port': '%s:%d' % (server_host, RSYSLOG_PORT),
+        'syslog_host_port': syslog,
         'metadata_enlist_url': metadata_enlist_url,
         }
 
diff --git a/src/maasserver/rpc/boot.py b/src/maasserver/rpc/boot.py
index 7ee36fe..7755761 100644
--- a/src/maasserver/rpc/boot.py
+++ b/src/maasserver/rpc/boot.py
@@ -45,6 +45,7 @@ from provisioningserver.utils.twisted import (
     synchronous,
     undefined,
 )
+from provisioningserver.utils.url import splithost
 
 
 DEFAULT_ARCH = 'i386'
@@ -283,6 +284,7 @@ def get_config(
         # for arch detection.
         raise BootConfigNoResponse()
 
+    # Get all required configuration objects in a single query.
     configs = Config.objects.get_configs([
         'commissioning_osystem',
         'commissioning_distro_series',
@@ -293,7 +295,16 @@ def get_config(
         'kernel_opts',
         'use_rack_proxy',
         'maas_internal_domain',
+        'remote_syslog',
     ])
+
+    # Compute the syslog server.
+    log_host, log_port = local_ip, RSYSLOG_PORT
+    if configs['remote_syslog']:
+        log_host, log_port = splithost(configs['remote_syslog'])
+        if log_port is None:
+            log_port = 514  # Fallback to default UDP syslog port.
+
     if machine is not None:
         # Update the last interface, last access cluster IP address, and
         # the last used BIOS boot method.
@@ -362,8 +373,8 @@ def get_config(
                 "domain": domain,
                 "preseed_url": preseed_url,
                 "fs_host": local_ip,
-                "log_host": local_ip,
-                "log_port": RSYSLOG_PORT,
+                "log_host": log_host,
+                "log_port": log_port,
                 "extra_opts": '',
                 "http_boot": True,
             }
@@ -484,8 +495,8 @@ def get_config(
         "domain": domain,
         "preseed_url": preseed_url,
         "fs_host": local_ip,
-        "log_host": local_ip,
-        "log_port": RSYSLOG_PORT,
+        "log_host": log_host,
+        "log_port": log_port,
         "extra_opts": '' if extra_kernel_opts is None else extra_kernel_opts,
         # As of MAAS 2.4 only HTTP boot is supported. This ensures MAAS 2.3
         # rack controllers use HTTP boot as well.
diff --git a/src/maasserver/templates/maasserver/settings_network.html b/src/maasserver/templates/maasserver/settings_network.html
index a9d3429..ab1de79 100644
--- a/src/maasserver/templates/maasserver/settings_network.html
+++ b/src/maasserver/templates/maasserver/settings_network.html
@@ -104,6 +104,24 @@
 
     <div class="p-strip is-bordered">
         <div class="row">
+            <div id="syslog" class="col-8">
+                <h2 class="p-heading--four">Syslog</h2>
+                <form action="{% url 'settings_network' %}" method="post">
+                    {% csrf_token %}
+                    <ul class="p-list">
+                        {% for field in syslog_form %}
+                            {% include "maasserver/form_field.html" %}
+                        {% endfor %}
+                    </ul>
+                    <input type="hidden" name="syslog_submit" value="1" />
+                    <button type="submit" class="p-button--positive u-float--right">Save</button>
+                </form>
+            </div>
+        </div>
+    </div>
+
+    <div class="p-strip is-bordered">
+        <div class="row">
             <div id="network" class="col-8">
                 <h2 class="p-heading--four">Network Discovery</h2>
                 <form action="{% url 'settings_network' %}" method="post">
diff --git a/src/maasserver/tests/test_compose_preseed.py b/src/maasserver/tests/test_compose_preseed.py
index 2ab0836..4332a1a 100644
--- a/src/maasserver/tests/test_compose_preseed.py
+++ b/src/maasserver/tests/test_compose_preseed.py
@@ -420,6 +420,24 @@ class TestComposePreseed(MAASServerTestCase):
                 reverse('metadata-status', args=[node.system_id])),
             preseed['reporting']['maas']['endpoint'])
 
+    def test_compose_preseed_uses_remote_syslog(self):
+        remote_syslog = '192.168.1.1:514'
+        Config.objects.set_config('remote_syslog', remote_syslog)
+        rack_controller = factory.make_RackController(url='')
+        node = factory.make_Node(
+            interface=True, status=NODE_STATUS.COMMISSIONING)
+        nic = node.get_boot_interface()
+        nic.vlan.dhcp_on = True
+        nic.vlan.primary_rack = rack_controller
+        nic.vlan.save()
+        request = make_HttpRequest()
+        preseed = yaml.safe_load(
+            compose_preseed(
+                request, PRESEED_TYPE.COMMISSIONING, node))
+        self.assertEqual(
+            remote_syslog,
+            preseed['rsyslog']['remotes']['maas'])
+
     def test_compose_preseed_for_rescue_mode_does_not_include_poweroff(self):
         rack_controller = factory.make_RackController()
         node = factory.make_Node(
diff --git a/src/maasserver/tests/test_preseed.py b/src/maasserver/tests/test_preseed.py
index 8ca5a2e..d77cff7 100644
--- a/src/maasserver/tests/test_preseed.py
+++ b/src/maasserver/tests/test_preseed.py
@@ -464,6 +464,12 @@ class TestPreseedContext(MAASServerTestCase):
              'server_url', 'syslog_host_port'],
             context.keys())
 
+    def test_get_preseed_context_includes_remote_syslog(self):
+        remote_syslog = '192.168.1.1:514'
+        Config.objects.set_config('remote_syslog', remote_syslog)
+        context = get_preseed_context(make_HttpRequest())
+        self.assertEquals(remote_syslog, context['syslog_host_port'])
+
 
 class TestNodeDeprecatedPreseedContext(
         PreseedRPCMixin, BootImageHelperMixin, MAASTransactionServerTestCase):
diff --git a/src/maasserver/views/settings.py b/src/maasserver/views/settings.py
index 7087e48..e020480 100644
--- a/src/maasserver/views/settings.py
+++ b/src/maasserver/views/settings.py
@@ -44,6 +44,7 @@ from maasserver.forms import (
     NTPForm,
     ProxyForm,
     StorageSettingsForm,
+    SyslogForm,
     ThirdPartyDriversForm,
     UbuntuForm,
     WindowsForm,
@@ -362,6 +363,13 @@ def network(request):
     if response is not None:
         return response
 
+    # Process the Syslog form.
+    syslog_form, response = process_form(
+        request, SyslogForm, reverse('settings_network'), 'syslog',
+        "Configuration updated.")
+    if response is not None:
+        return response
+
     # Process the network discovery form.
     network_discovery_form, response = process_form(
         request, NetworkDiscoveryForm, reverse('settings_network'),
@@ -376,6 +384,7 @@ def network(request):
             'proxy_form': proxy_form,
             'dns_form': dns_form,
             'ntp_form': ntp_form,
+            'syslog_form': syslog_form,
             'network_discovery_form': network_discovery_form,
             'show_license_keys': show_license_keys(),
         })
diff --git a/src/provisioningserver/utils/tests/test_url.py b/src/provisioningserver/utils/tests/test_url.py
index f498b22..744cf9a 100644
--- a/src/provisioningserver/utils/tests/test_url.py
+++ b/src/provisioningserver/utils/tests/test_url.py
@@ -9,7 +9,10 @@ from random import randint
 
 from maastesting.factory import factory
 from maastesting.testcase import MAASTestCase
-from provisioningserver.utils.url import compose_URL
+from provisioningserver.utils.url import (
+    compose_URL,
+    splithost,
+)
 
 
 class TestComposeURL(MAASTestCase):
@@ -96,3 +99,32 @@ class TestComposeURL(MAASTestCase):
         self.assertEqual(
             'https://%s:%s/' % (hostname, port),
             compose_URL('https://:%s/' % port, hostname))
+
+
+class TestSplithost(MAASTestCase):
+
+    scenarios = (
+        ('ipv4', {
+            'host': '192.168.1.1:21',
+            'result': ('192.168.1.1', 21)
+        }),
+        ('ipv6', {
+            'host': '[::f]:21',
+            'result': ('[::f]', 21)
+        }),
+        ('ipv4_no_port', {
+            'host': '192.168.1.1',
+            'result': ('192.168.1.1', None)
+        }),
+        ('ipv6_no_port', {
+            'host': '[::f]',
+            'result': ('[::f]', None)
+        }),
+        ('ipv6_no_bracket', {
+            'host': '::ffff',
+            'result': ('[::ffff]', None)
+        }),
+    )
+
+    def test__result(self):
+        self.assertEqual(self.result, splithost(self.host))
diff --git a/src/provisioningserver/utils/url.py b/src/provisioningserver/utils/url.py
index b841c80..7935525 100644
--- a/src/provisioningserver/utils/url.py
+++ b/src/provisioningserver/utils/url.py
@@ -43,3 +43,21 @@ def compose_URL(base_url, host):
     else:
         netloc = '%s:%d' % (netloc_host, parsed_url.port)
     return urlunparse(parsed_url._replace(netloc=netloc))
+
+
+def splithost(host):
+    """Split `host` into hostname and port.
+
+    If no :port is in `host` the port with return as None.
+    """
+    parsed = urlparse('//' + host)
+    hostname = parsed.hostname
+    if hostname is None:
+        # This only occurs when the `host` is an IPv6 address without brakets.
+        # Lets try again but add the brackets.
+        parsed = urlparse('//[%s]' % host)
+        hostname = parsed.hostname
+    if ':' in hostname:
+        # IPv6 hostname, place back into brackets.
+        hostname = '[%s]' % hostname
+    return hostname, parsed.port