Merge lp:~rvb/maas/option-to-disable-image-update into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Add option to disable the auto import of images.

[Gavin Panella (allenap)]

Looks good.

review: Approve

[Raphaël Badin (rvb)]

All fixed, thanks for the review Gavin!

=== modified file 'src/maasserver/bootresources.py'
--- src/maasserver/bootresources.py	2015-05-07 18:14:38 +0000
+++ src/maasserver/bootresources.py	2015-05-13 16:38:46 +0000
@@ -13,10 +13,11 @@
 
 __metaclass__ = type
 __all__ = [
-    "ImportResourcesProgressService",
     "ensure_boot_source_definition",
     "get_simplestream_endpoint",
+    "ImportResourcesProgressService",
     "ImportResourcesService",
+    "IMPORT_RESOURCES_SERVICE_PERIOD",
     "is_import_resources_running",
     "simplestreams_file_handler",
     "simplestreams_stream_handler",
@@ -59,6 +60,7 @@
     BootResource,
     BootResourceFile,
     BootResourceSet,
+    Config,
     LargeFile,
     NodeGroup,
 )
@@ -994,16 +996,33 @@
     return locks.import_images.is_locked()
 
 
+def import_resources_periodically():
+    """Import boot resources.
+
+    This is called by the ImportResourcesService to import the boot resources
+    periodically.  It will simply call _import_resources_in_thread unless
+    the periodic image import mechanism has been disabled.
+    """
+    if Config.objects.get_config('boot_images_auto_import'):
+        return _import_resources_in_thread()
+    else:
+        maaslog.debug(
+            "Skipping periodic import of boot resources as it is disabled.")
+
+
+# How often the import service runs.
+IMPORT_RESOURCES_SERVICE_PERIOD = timedelta(hours=1)
+
+
 class ImportResourcesService(TimerService, object):
     """Service to periodically import boot resources.
 
     This will run immediately when it's started, then once again every hour,
     though the interval can be overridden by passing it to the constructor.
     """
-
-    def __init__(self, interval=timedelta(hours=1)):
+    def __init__(self, interval=IMPORT_RESOURCES_SERVICE_PERIOD):
         super(ImportResourcesService, self).__init__(
-            interval.total_seconds(), _import_resources_in_thread)
+            interval.total_seconds(), import_resources_periodically)
 
 
 class ImportResourcesProgressService(TimerService, object):

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2015-05-11 18:57:33 +0000
+++ src/maasserver/forms.py	2015-05-13 16:38:46 +0000
@@ -1264,6 +1264,9 @@
     """A base class for forms that save the content of their fields into
     Config objects.
     """
+    # List of fields that should be considered configuration fields.
+    # Consider all the fields as configuration fields if this is None.
+    config_fields = None
 
     def __init__(self, *args, **kwargs):
         super(ConfigForm, self).__init__(*args, **kwargs)
@@ -1280,9 +1283,14 @@
     def clean(self):
         cleaned_data = super(Form, self).clean()
         for config_name in cleaned_data.keys():
-            if config_name not in CONFIG_ITEMS_KEYS:
-                self._errors[config_name] = self.error_class([
-                    INVALID_SETTING_MSG_TEMPLATE % config_name])
+            consider_field = (
+                self.config_fields is None or
+                config_name in self.config_fields
+            )
+            if consider_field:
+                if config_name not in CONFIG_ITEMS_KEYS:
+                    self._errors[config_name] = self.error_class([
+                        INVALID_SETTING_MSG_TEMPLATE % config_name])
         return cleaned_data
 
     def save(self):
@@ -1299,7 +1307,12 @@
             return False
         else:
             for name, value in self.cleaned_data.items():
-                Config.objects.set_config(name, value)
+                consider_field = (
+                    self.config_fields is None or
+                    name in self.config_fields
+                )
+                if consider_field:
+                    Config.objects.set_config(name, value)
             return True
 
 
@@ -1379,8 +1392,11 @@
     kernel_opts = get_config_field('kernel_opts')
 
 
-class BootSourceSettingsForm(Form):
+class BootSourceSettingsForm(ConfigForm):
     """Settings page, Boot Images section."""
+    config_fields = ['boot_images_auto_import']
+
+    boot_images_auto_import = get_config_field('boot_images_auto_import')
     boot_source_url = forms.CharField(
         label="Sync URL", required=True,
         help_text=(
@@ -1420,7 +1436,7 @@
             sucessfully saved into the detabase.
         :rtype: boolean
         """
-        self.full_clean()
+        super(BootSourceSettingsForm, self).save()
         if self._errors:
             return False
         boot_source = BootSource.objects.first()

=== modified file 'src/maasserver/forms_settings.py'
--- src/maasserver/forms_settings.py	2015-05-07 18:14:38 +0000
+++ src/maasserver/forms_settings.py	2015-05-13 16:38:46 +0000
@@ -25,6 +25,7 @@
 
 from django import forms
 from django.core.exceptions import ValidationError
+from maasserver.bootresources import IMPORT_RESOURCES_SERVICE_PERIOD
 from maasserver.models.config import (
     Config,
     DEFAULT_OS,
@@ -293,6 +294,17 @@
                 "Erase nodes' disks prior to releasing.")
         }
     },
+    'boot_images_auto_import': {
+        'default': True,
+        'form': forms.BooleanField,
+        'form_kwargs': {
+            'required': False,
+            'label': (
+                "Automatically import/refresh the boot images "
+                "every %d minutes" %
+                (IMPORT_RESOURCES_SERVICE_PERIOD.total_seconds() / 60.0))
+        }
+    },
 }
 
 

=== modified file 'src/maasserver/models/config.py'
--- src/maasserver/models/config.py	2015-05-07 18:14:38 +0000
+++ src/maasserver/models/config.py	2015-05-13 16:38:46 +0000
@@ -63,6 +63,8 @@
         # RPC configuration.
         'rpc_region_certificate': None,
         'rpc_shared_secret': None,
+        # Images.
+        'boot_images_auto_import': True,
         # Third Party
         'enable_third_party_drivers': True,
         'enable_disk_erasing_on_release': False,

=== modified file 'src/maasserver/tests/test_bootresources.py'
--- src/maasserver/tests/test_bootresources.py	2015-05-07 18:14:38 +0000
+++ src/maasserver/tests/test_bootresources.py	2015-05-13 16:38:46 +0000
@@ -1240,10 +1240,30 @@
     def test__calls__import_resources_in_thread(self):
         service = bootresources.ImportResourcesService()
         self.assertEqual(
-            (bootresources._import_resources_in_thread, (), {}),
+            (bootresources.import_resources_periodically, (), {}),
             service.call)
 
 
+class TestImportResourcesPeriodically(MAASTestCase):
+    """Tests for `import_resources_periodically`."""
+
+    def test__calls__import_resources_in_thread_if_auto(self):
+        Config.objects.set_config('boot_images_auto_import', True)
+        mock__import_resources_in_thread = self.patch(
+            bootresources, '_import_resources_in_thread')
+        bootresources.import_resources_periodically()
+        self.assertThat(
+            mock__import_resources_in_thread, MockCalledOnceWith())
+
+    def test__doesnt_call__import_resources_in_thread_if_no_auto(self):
+        Config.objects.set_config('boot_images_auto_import', False)
+        mock__import_resources_in_thread = self.patch(
+            bootresources, '_import_resources_in_thread')
+        bootresources.import_resources_periodically()
+        self.assertThat(
+            mock__import_resources_in_thread, MockNotCalled())
+
+
 class TestImportResourcesProgressService(MAASServerTestCase):
     """Tests for `ImportResourcesProgressService`."""
 

=== modified file 'src/maasserver/tests/test_forms_config.py'
--- src/maasserver/tests/test_forms_config.py	2014-08-26 03:50:06 +0000
+++ src/maasserver/tests/test_forms_config.py	2015-05-13 16:38:46 +0000
@@ -31,6 +31,13 @@
     maas_name = forms.CharField(label="Field 1", max_length=10)
 
 
+class TestCompositeForm(ConfigForm):
+    config_fields = ['maas_name']
+
+    maas_name = forms.CharField(label="Field 1", max_length=10)
+    non_config_field = forms.CharField(label="Field 2", max_length=10)
+
+
 class ConfigFormTest(MAASServerTestCase):
 
     def test_form_valid_saves_into_db(self):
@@ -38,7 +45,7 @@
         form = TestValidOptionForm({'maas_name': value})
         result = form.save()
 
-        self.assertTrue(result)
+        self.assertTrue(result, form._errors)
         self.assertEqual(value, Config.objects.get_config('maas_name'))
 
     def test_form_rejects_unknown_settings(self):
@@ -47,7 +54,7 @@
         form = TestOptionForm({'field1': value, 'field2': value2})
         valid = form.is_valid()
 
-        self.assertFalse(valid)
+        self.assertFalse(valid, form._errors)
         self.assertIn('field1', form._errors)
         self.assertIn('field2', form._errors)
 
@@ -56,7 +63,7 @@
         form = TestOptionForm({'field1': value_too_long, 'field2': False})
         result = form.save()
 
-        self.assertFalse(result)
+        self.assertFalse(result, form._errors)
         self.assertIn('field1', form._errors)
         self.assertIsNone(Config.objects.get_config('field1'))
         self.assertIsNone(Config.objects.get_config('field2'))
@@ -76,3 +83,13 @@
 
         self.assertItemsEqual(['field1'], form.initial)
         self.assertEqual(value, form.initial['field1'])
+
+    def test_validates_composite_form(self):
+        value1 = factory.make_string(5)
+        value2 = factory.make_string(5)
+        form = TestCompositeForm(
+            {'maas_name': value1, 'non_config_field': value2})
+        result = form.save()
+
+        self.assertTrue(result, form._errors)
+        self.assertEqual(value1, Config.objects.get_config('maas_name'))