Merge ~ack/maas:set-snap-proxy into maas:master

[Alberto Donato]

set proxy for snapd when deploying machines

[Adam Collard (adam-collard)]

LGTM

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b set-snap-proxy lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/9581/console
COMMIT: cf111960ea305da946360e126763d6af153cdb28

review: Needs Fixing

[MAAS Lander (maas-lander)]

LANDING
-b set-snap-proxy lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/9583/consoleText

diff --git a/src/maasserver/compose_preseed.py b/src/maasserver/compose_preseed.py
index 37dffec..fb6ac65 100644
--- a/src/maasserver/compose_preseed.py
+++ b/src/maasserver/compose_preseed.py
@@ -303,6 +303,20 @@ def get_cloud_init_reporting(request, node, token):
     }
 
 
+def get_snap_config(request, node):
+    """Return the cloud-init snap configuration."""
+    proxy = get_apt_proxy(request, node.get_boot_rack_controller(), node)
+    if not proxy:
+        return {}
+    return {
+        "snap": {
+            "commands": [
+                f'snap set system proxy.http="{proxy}" proxy.https="{proxy}"',
+            ],
+        },
+    }
+
+
 def get_rsyslog_host_port(request, node):
     """Return the rsyslog host and port to use."""
     configs = Config.objects.get_configs(["remote_syslog", "maas_syslog_port"])
@@ -509,6 +523,8 @@ def _compose_cloud_init_preseed(
     cloud_config.update(
         get_archive_config(request, node, preserve_sources=False)
     )
+    # Add snaps configuration
+    cloud_config.update(get_snap_config(request, node))
 
     enable_ssh = (
         node.status in {NODE_STATUS.COMMISSIONING, NODE_STATUS.TESTING}
diff --git a/src/maasserver/tests/test_compose_preseed.py b/src/maasserver/tests/test_compose_preseed.py
index 3f8369b..a7e634b 100644
--- a/src/maasserver/tests/test_compose_preseed.py
+++ b/src/maasserver/tests/test_compose_preseed.py
@@ -1,9 +1,6 @@
-# Copyright 2012-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Tests for `maasserver.compose_preseed`."""
-
-
 import random
 
 from django.urls import reverse
@@ -473,6 +470,14 @@ class TestComposePreseed(MAASServerTestCase):
                 }
             ),
         )
+        self.assertEqual(
+            config["snap"],
+            {
+                "commands": [
+                    f'snap set system proxy.http="{apt_proxy}" proxy.https="{apt_proxy}"',
+                ],
+            },
+        )
 
     def test_compose_preseed_for_commissioning_node_skips_apt_proxy(self):
         # Disable boot source cache signals.
@@ -524,6 +529,14 @@ class TestComposePreseed(MAASServerTestCase):
         )
         self.assertThat(preseed["rsyslog"]["remotes"], KeysEqual("maas"))
         self.assertAptConfig(preseed, apt_proxy)
+        self.assertEqual(
+            preseed["snap"],
+            {
+                "commands": [
+                    f'snap set system proxy.http="{apt_proxy}" proxy.https="{apt_proxy}"',
+                ],
+            },
+        )
 
     def test_compose_preseed_for_commissioning_node_has_header(self):
         rack_controller = factory.make_RackController()
@@ -870,6 +883,14 @@ class TestComposePreseed(MAASServerTestCase):
             preseed["datasource"]["MAAS"]["metadata_url"],
         )
         self.assertAptConfig(preseed, expected_apt_proxy)
+        self.assertEqual(
+            preseed["snap"],
+            {
+                "commands": [
+                    f'snap set system proxy.http="{expected_apt_proxy}" proxy.https="{expected_apt_proxy}"',
+                ],
+            },
+        )
 
     def test_compose_preseed_with_curtin_installer_skips_apt_proxy(self):
         # Disable boot source cache signals.
@@ -890,6 +911,7 @@ class TestComposePreseed(MAASServerTestCase):
         )
 
         self.assertNotIn("proxy", preseed["apt"])
+        self.assertNotIn("snap", preseed)
 
     # LP: #1743966 - Test for archive key work around
     def test_compose_preseed_for_curtin_and_trusty_aptsources(self):
diff --git a/src/metadataserver/tests/test_api.py b/src/metadataserver/tests/test_api.py
index 36a599e..50d25a5 100644
--- a/src/metadataserver/tests/test_api.py
+++ b/src/metadataserver/tests/test_api.py
@@ -28,7 +28,6 @@ from testtools.matchers import (
     ContainsAll,
     ContainsDict,
     Equals,
-    KeysEqual,
     StartsWith,
 )
 import yaml
@@ -981,9 +980,9 @@ class TestMetadataCommon(MAASServerTestCase):
         content = yaml.safe_load(response.content)
         self.assertThat(response, HasStatusCode(http.client.OK))
         self.assertThat(content, LooksLikeCloudInit)
-        self.assertThat(
+        self.assertCountEqual(
             yaml.safe_load(content["cloud-init"]),
-            KeysEqual("system_info", "runcmd"),
+            ["runcmd", "snap", "system_info"],
         )
 
     def test_vendor_data_node_without_def_user_includes_no_system_info(self):
@@ -998,8 +997,9 @@ class TestMetadataCommon(MAASServerTestCase):
         content = yaml.safe_load(response.content)
         self.assertThat(response, HasStatusCode(http.client.OK))
         self.assertThat(content, LooksLikeCloudInit)
-        self.assertThat(
-            yaml.safe_load(content["cloud-init"]), KeysEqual("runcmd")
+        self.assertCountEqual(
+            yaml.safe_load(content["cloud-init"]),
+            ["runcmd", "snap"],
         )
 
     def test_vendor_data_for_node_without_owner_includes_no_system_info(self):
@@ -1010,8 +1010,9 @@ class TestMetadataCommon(MAASServerTestCase):
         content = yaml.safe_load(response.content)
         self.assertThat(response, HasStatusCode(http.client.OK))
         self.assertThat(content, LooksLikeCloudInit)
-        self.assertThat(
-            yaml.safe_load(content["cloud-init"]), KeysEqual("runcmd")
+        self.assertCountEqual(
+            yaml.safe_load(content["cloud-init"]),
+            ["runcmd", "snap"],
         )
 
     def test_vendor_data_calls_through_to_get_vendor_data(self):
diff --git a/src/metadataserver/tests/test_vendor_data.py b/src/metadataserver/tests/test_vendor_data.py
index 3b5e328..36fc86b 100644
--- a/src/metadataserver/tests/test_vendor_data.py
+++ b/src/metadataserver/tests/test_vendor_data.py
@@ -32,6 +32,7 @@ from metadataserver.vendor_data import (
     generate_ephemeral_deployment_network_configuration,
     generate_ntp_configuration,
     generate_rack_controller_configuration,
+    generate_snap_configuration,
     generate_system_info,
     get_vendor_data,
 )
@@ -116,6 +117,30 @@ class TestGenerateSystemInfo(MAASServerTestCase):
         )
 
 
+class TestGenerateSnapConfiguration(MAASServerTestCase):
+    def test_no_proxy(self):
+        node = factory.make_Node()
+        config = generate_snap_configuration(node, None)
+        self.assertEqual(list(config), [])
+
+    def test_proxy(self):
+        node = factory.make_Node()
+        config = generate_snap_configuration(node, "http://proxy.example.com/")
+        self.assertEqual(
+            list(config),
+            [
+                (
+                    "snap",
+                    {
+                        "commands": [
+                            'snap set system proxy.http="http://proxy.example.com/" proxy.https="http://proxy.example.com/"'
+                        ],
+                    },
+                ),
+            ],
+        )
+
+
 class TestGenerateNTPConfiguration(MAASServerTestCase):
     """Tests for `generate_ntp_configuration`."""
 
@@ -203,84 +228,84 @@ class TestGenerateRackControllerConfiguration(MAASServerTestCase):
 
     def test_yields_nothing_when_node_is_not_netboot_disabled(self):
         configuration = generate_rack_controller_configuration(
-            node=factory.make_Node(osystem="ubuntu"),
-            proxy="http://proxy.example.com/",
+            node=factory.make_Node(osystem="ubuntu")
         )
-        self.assertThat(dict(configuration), Equals({}))
+        self.assertEqual(list(configuration), [])
 
     def test_yields_nothing_when_node_is_not_ubuntu(self):
         tag = factory.make_Tag(name="switch")
         node = factory.make_Node(osystem="centos", netboot=False)
         node.tags.add(tag)
-        configuration = generate_rack_controller_configuration(
-            node, proxy="http://proxy.example.com/"
-        )
-        self.assertThat(dict(configuration), Equals({}))
+        configuration = generate_rack_controller_configuration(node)
+        self.assertEqual(list(configuration), [])
 
     def test_yields_configuration_with_ubuntu(self):
         tag = factory.make_Tag(name="wedge100")
         node = factory.make_Node(osystem="ubuntu", netboot=False)
         node.tags.add(tag)
-        configuration = generate_rack_controller_configuration(
-            node, proxy="http://proxy.example.com/"
-        )
+        configuration = generate_rack_controller_configuration(node)
         secret = "1234"
         Config.objects.set_config("rpc_shared_secret", secret)
         channel = version.get_maas_version_track_channel()
         maas_url = "http://%s:5240/MAAS" % get_maas_facing_server_host(
             node.get_boot_rack_controller()
         )
-        cmd = "/bin/snap/maas init --mode rack"
-
-        self.assertThat(
-            dict(configuration),
-            KeysEqual(
-                {
-                    "runcmd": [
-                        f"snap install maas --channel={channel}",
-                        "%s --maas-url %s --secret %s"
-                        % (cmd, maas_url, secret),
-                    ]
-                }
-            ),
+        self.assertEqual(
+            list(configuration),
+            [
+                (
+                    "runcmd",
+                    [
+                        ["snap", "install", "maas", f"--channel={channel}"],
+                        [
+                            "/snap/bin/maas",
+                            "init",
+                            "rack",
+                            "--maas-url",
+                            maas_url,
+                            "--secret",
+                            secret,
+                        ],
+                    ],
+                ),
+            ],
         )
 
     def test_yields_nothing_when_machine_install_rackd_false(self):
         node = factory.make_Node(osystem="ubuntu", netboot=False)
         node.install_rackd = False
-        configuration = generate_rack_controller_configuration(
-            node, proxy="http://proxy.example.com/"
-        )
-        self.assertThat(dict(configuration), Equals({}))
+        configuration = generate_rack_controller_configuration(node)
+        self.assertEqual(list(configuration), [])
 
     def test_yields_configuration_when_machine_install_rackd_true(self):
         node = factory.make_Node(osystem="ubuntu", netboot=False)
         node.install_rackd = True
-        proxy = "http://proxy.example.com/"
-        configuration = generate_rack_controller_configuration(
-            node, proxy=proxy
-        )
+        configuration = generate_rack_controller_configuration(node)
         secret = "1234"
         Config.objects.set_config("rpc_shared_secret", secret)
         channel = version.get_maas_version_track_channel()
         maas_url = "http://%s:5240/MAAS" % get_maas_facing_server_host(
             node.get_boot_rack_controller()
         )
-        cmd = "/bin/snap/maas init --mode rack"
-
-        self.assertThat(
-            dict(configuration),
-            KeysEqual(
-                {
-                    "runcmd": [
-                        "snap set system proxy.http=%s proxy.https=%s"
-                        % (proxy, proxy),
-                        f"snap install maas --channel={channel}",
-                        "%s --maas-url %s --secret %s"
-                        % (cmd, maas_url, secret),
-                    ]
-                }
-            ),
+        self.assertEqual(
+            list(configuration),
+            [
+                (
+                    "runcmd",
+                    [
+                        ["snap", "install", "maas", f"--channel={channel}"],
+                        [
+                            "/snap/bin/maas",
+                            "init",
+                            "rack",
+                            "--maas-url",
+                            maas_url,
+                            "--secret",
+                            secret,
+                        ],
+                    ],
+                ),
+            ],
         )
 
     def test_yields_configuration_when_machine_install_kvm_true(self):
@@ -415,7 +440,7 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
         )
         config = get_vendor_data(node, None)
         self.assertThat(mock_get_configs, MockNotCalled())
-        self.assertDictEqual({}, config)
+        self.assertNotIn(config, "write_files")
 
     def test_returns_nothing_if_no_values_set(self):
         node = factory.make_Node(
@@ -425,7 +450,7 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
         )
         node.nodemetadata_set.create(key="vcenter_registration", value="True")
         config = get_vendor_data(node, None)
-        self.assertDictEqual({}, config)
+        self.assertNotIn(config, "write_files")
 
     def test_returns_vcenter_yaml(self):
         node = factory.make_Node(
@@ -435,24 +460,22 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
         )
         node.nodemetadata_set.create(key="vcenter_registration", value="True")
         vcenter = {
+            "vcenter_datacenter": factory.make_name("vcenter_datacenter"),
             "vcenter_server": factory.make_name("vcenter_server"),
-            "vcenter_username": factory.make_name("vcenter_username"),
             "vcenter_password": factory.make_name("vcenter_password"),
-            "vcenter_datacenter": factory.make_name("vcenter_datacenter"),
+            "vcenter_username": factory.make_name("vcenter_username"),
         }
         for key, value in vcenter.items():
             Config.objects.set_config(key, value)
         config = get_vendor_data(node, None)
-        self.assertDictEqual(
-            {
-                "write_files": [
-                    {
-                        "content": yaml.safe_dump(vcenter),
-                        "path": "/altbootbank/maas/vcenter.yaml",
-                    }
-                ]
-            },
-            config,
+        self.assertEqual(
+            config["write_files"],
+            [
+                {
+                    "content": yaml.safe_dump(vcenter),
+                    "path": "/altbootbank/maas/vcenter.yaml",
+                },
+            ],
         )
 
     def test_returns_vcenter_yaml_if_rbac_admin(self):
@@ -466,24 +489,22 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
         rbac.store.add_pool(node.pool)
         rbac.store.allow(node.owner.username, node.pool, "admin-machines")
         vcenter = {
+            "vcenter_datacenter": factory.make_name("vcenter_datacenter"),
             "vcenter_server": factory.make_name("vcenter_server"),
-            "vcenter_username": factory.make_name("vcenter_username"),
             "vcenter_password": factory.make_name("vcenter_password"),
-            "vcenter_datacenter": factory.make_name("vcenter_datacenter"),
+            "vcenter_username": factory.make_name("vcenter_username"),
         }
         for key, value in vcenter.items():
             Config.objects.set_config(key, value)
         config = get_vendor_data(node, None)
-        self.assertDictEqual(
-            {
-                "write_files": [
-                    {
-                        "content": yaml.safe_dump(vcenter),
-                        "path": "/altbootbank/maas/vcenter.yaml",
-                    }
-                ]
-            },
-            config,
+        self.assertEqual(
+            config["write_files"],
+            [
+                {
+                    "content": yaml.safe_dump(vcenter),
+                    "path": "/altbootbank/maas/vcenter.yaml",
+                },
+            ],
         )
 
     def test_returns_nothing_if_rbac_user(self):
@@ -497,23 +518,23 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
         rbac.store.add_pool(node.pool)
         rbac.store.allow(node.owner.username, node.pool, "deploy-machines")
         vcenter = {
+            "vcenter_datacenter": factory.make_name("vcenter_datacenter"),
+            "vcenter_password": factory.make_name("vcenter_password"),
             "vcenter_server": factory.make_name("vcenter_server"),
             "vcenter_username": factory.make_name("vcenter_username"),
-            "vcenter_password": factory.make_name("vcenter_password"),
-            "vcenter_datacenter": factory.make_name("vcenter_datacenter"),
         }
         for key, value in vcenter.items():
             Config.objects.set_config(key, value)
         config = get_vendor_data(node, None)
-        self.assertDictEqual({}, config)
+        self.assertNotIn(config, "write_files")
 
     def test_returns_nothing_if_no_user(self):
         node = factory.make_Node(status=NODE_STATUS.DEPLOYING, osystem="esxi")
-        for i in ["server", "username", "password", "datacenter"]:
+        for i in ["datacenter", "password", "server", "username"]:
             key = "vcenter_%s" % i
             Config.objects.set_config(key, factory.make_name(key))
         config = get_vendor_data(node, None)
-        self.assertDictEqual({}, config)
+        self.assertNotIn(config, "write_files")
 
     def test_returns_nothing_if_user(self):
         node = factory.make_Node(
@@ -525,7 +546,7 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
             key = "vcenter_%s" % i
             Config.objects.set_config(key, factory.make_name(key))
         config = get_vendor_data(node, None)
-        self.assertDictEqual({}, config)
+        self.assertNotIn(config, "write_files")
 
     def test_returns_nothing_if_vcenter_registration_not_set(self):
         node = factory.make_Node(
@@ -537,4 +558,4 @@ class TestGenerateVcenterConfiguration(MAASServerTestCase):
             key = "vcenter_%s" % i
             Config.objects.set_config(key, factory.make_name(key))
         config = get_vendor_data(node, None)
-        self.assertDictEqual({}, config)
+        self.assertNotIn(config, "write_files")
diff --git a/src/metadataserver/vendor_data.py b/src/metadataserver/vendor_data.py
index 4da5c8c..b914df5 100644
--- a/src/metadataserver/vendor_data.py
+++ b/src/metadataserver/vendor_data.py
@@ -28,8 +28,9 @@ def get_vendor_data(node, proxy):
     return dict(
         chain(
             generate_system_info(node),
+            generate_snap_configuration(node, proxy),
             generate_ntp_configuration(node),
-            generate_rack_controller_configuration(node, proxy),
+            generate_rack_controller_configuration(node),
             generate_kvm_pod_configuration(node),
             generate_ephemeral_netplan_lock_removal(node),
             generate_ephemeral_deployment_network_configuration(node),
@@ -49,6 +50,17 @@ def generate_system_info(node):
         }
 
 
+def generate_snap_configuration(node, proxy):
+    """Generate cloud-init configuration for snapd."""
+    if not proxy:
+        return
+    yield "snap", {
+        "commands": [
+            f'snap set system proxy.http="{proxy}" proxy.https="{proxy}"',
+        ],
+    }
+
+
 def generate_ntp_configuration(node):
     """Generate cloud-init configuration for NTP servers.
 
@@ -76,7 +88,7 @@ def generate_ntp_configuration(node):
         yield "ntp", {"servers": servers, "pools": pools}
 
 
-def generate_rack_controller_configuration(node, proxy):
+def generate_rack_controller_configuration(node):
     """Generate cloud-init configuration to install the rack controller."""
 
     # FIXME: For now, we are using a tag ('switch') to deploy the rack
@@ -103,20 +115,10 @@ def generate_rack_controller_configuration(node, proxy):
         secret = Config.objects.get_config("rpc_shared_secret")
         source = get_maas_version_track_channel()
         yield "runcmd", [
-            [
-                "snap",
-                "set",
-                "system",
-                f"proxy.http={proxy}",
-                f"proxy.https={proxy}",
-            ],
             ["snap", "install", "maas", f"--channel={source}"],
-            ["systemctl", "restart", "snapd"],
-            ["export", "PATH=$PATH"],
             [
                 "/snap/bin/maas",
                 "init",
-                "--mode",
                 "rack",
                 "--maas-url",
                 maas_url,