Merge ~smoser/maas:cleanup/render-curtin-config-internally into maas:master

Proposed by Scott Moser
Status: Rejected
Rejected by: Blake Rouse
Proposed branch: ~smoser/maas:cleanup/render-curtin-config-internally
Merge into: maas:master
Diff against target: 640 lines (+224/-127)
7 files modified
contrib/preseeds_v2/curtin_userdata (+18/-42)
contrib/preseeds_v2/curtin_userdata_centos (+3/-9)
contrib/preseeds_v2/curtin_userdata_windows (+2/-10)
src/maasserver/node_action.py (+2/-2)
src/maasserver/preseed.py (+133/-6)
src/maasserver/tests/test_node_action.py (+21/-21)
src/maasserver/tests/test_preseed.py (+45/-37)
Reviewer Review Type Date Requested Status
MAAS Lander Needs Fixing
Mike Pontillo (community) Needs Information
Andres Rodriguez Pending
Review via email: mp+332573@code.launchpad.net

Commit message

Internally handle most of curtin_userdata leaving templates to user.

The 'curtin_userdata' file was scary to look at, and hard to work
with for a user. Adding early_commands or late_commands meant that they
would have to edit an existing entry, and failure to do so would
break the install.

This takes what was previously in that those files and handles them
internally to maas. Thus, the user now has a blank slate to work on,
and an example of what could go there.

One change along the way was to render the writing of the binary
tmp file with the apt key via base64 rather than hex encoding.

Description of the change

This is a work in progress, I'm looking to see if it the general concept
is acceptable.

It is untested at this point, but I believe should be (close to) functional but does break third party drivers currently.

To post a comment you must log in.
Revision history for this message
Blake Rouse (blake-rouse) wrote :

I think the general idea is great, but we need to think about the upgrade path.

What happens if they made changes to the sections that you have not made internal? Will there changes override the internal changes?

Also this is more of a future thing, but I would prefer to see the templates move into the database with an API, because at the moment with HA, these are a problem.

Revision history for this message
Andres Rodriguez (andreserl) wrote :

That's my only concern, we need to make sure the upgrade path doesn't break
and this continuous to work with those that modify their curtin scripts.
Which is a lot of people atm.

On Mon, Oct 23, 2017 at 9:16 AM, Blake Rouse <email address hidden>
wrote:

> I think the general idea is great, but we need to think about the upgrade
> path.
>
> What happens if they made changes to the sections that you have not made
> internal? Will there changes override the internal changes?
>
> Also this is more of a future thing, but I would prefer to see the
> templates move into the database with an API, because at the moment with
> HA, these are a problem.
> --
> https://code.launchpad.net/~smoser/maas/+git/maas/+merge/332573
> Your team MAAS Committers is subscribed to branch maas:master.
>

--
Andres Rodriguez
Engineering Manager, MAAS
Canonical USA, Inc.

Revision history for this message
Scott Moser (smoser) wrote :

Summary: over all, it will do reasonably well on "upgrade".

What is the upgrade path anyway, for *any* change to that file?
Do you have something in place other than debian conf-file upgrades?

If all you have is debian conf-file, then the admin running 'apt-get
dist-upgrade' is going to get prompted and see that those files are being
replaced and have the ability to diff them.

I'd consider that there are 2 likely paths:
a.) user does nothing, leaving the whole file in place as it was before.
b.) user looks at it, sees their changes and generally correctly merges
them by creating the file with *just* their changes inside.

So 'b' is the easy path, that should work or is generally not fixable
other than by the user.

for 'a', all the "builtin" config values are dictionaries.
When curtin merges dictionaries, the later one wins.
So as long as we keep the same keys in all cases, then nothing
would get run more than once and all should be well.
By 'key', i mean:
  early_commands/driver_*
and
  debconf_selections/maas

If you really wanted to safe-guard you could check the loaded file
and explicitly ignore the previously named keys, but that too could
actually cause breakage, if the user had modified those keys to
their liking.

Revision history for this message
Scott Moser (smoser) wrote :

Actually, what I think i'd do and suggest is to
a.) rename the keys from the names that did exist in that file
  early_commands/driver_* to early_commands/30_driver_*
  debconf_selections/maas_* to debconf_selections/maas_internal

b.) "clean" out the old names if they were present in the loaded config

this would mean that user's modifications to that file that modified *those* things would actually fail to have affect. But generally speaking they should not have done that, or we'd like to have bugs to see what was wrong.

then just warn in log that it is ignoring those things.
that'd keep their file basically working as expected other than some things
in it not actually having any affect.

Revision history for this message
Andres Rodriguez (andreserl) wrote :

For 2.3, this is too late, so we will target this for 2.4 once it opens.

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b cleanup/render-curtin-config-internally lp:~smoser/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/519/console
COMMIT: d313a298a0eeb39535e8017b777db8481010f5e1

review: Needs Fixing
Revision history for this message
Ante Karamatić (ivoks) :
Revision history for this message
Scott Moser (smoser) wrote :

I addressed Ante's comment and updated to merge without conflicts on current master.

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b cleanup/render-curtin-config-internally lp:~smoser/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/812/console
COMMIT: addfc9a36144635a35e1465f10cb7de905ae387d

review: Needs Fixing
Revision history for this message
Scott Moser (smoser) wrote :

I think this is ready for review at this point.
I've fixed up all the unit tests.

Revision history for this message
Mike Pontillo (mpontillo) wrote :

This branch is looking much better, but where are the tests that cover the new code?

review: Needs Information
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b cleanup/render-curtin-config-internally lp:~smoser/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/823/console
COMMIT: 296fab455858b1b3970137fb23f27351db5b9d8e

review: Needs Fixing
Revision history for this message
Scott Moser (smoser) wrote :

well, we can add tests for the new code, but this is really just code motion (replace of template rendering with internal).
Ie, the net change is the same behavior. Whatever tests were there before (that I adjusted) should give the same total code coverage.

What specific things are you wanting to be tested?

Revision history for this message
Scott Moser (smoser) wrote :

Also, I can't see the results of the unit tests above.
roaksoax gave me a paste of them, but I'm not sure what woudl be causing the 'url'' to not get set (some help?)

Revision history for this message
Andres Rodriguez (andreserl) wrote :

Hi Scott,

We'll take a look at it next week. Right now the team is running on reduced
capacity.

On Fri, Dec 1, 2017 at 10:53 AM, Scott Moser <email address hidden>
wrote:

> Also, I can't see the results of the unit tests above.
> roaksoax gave me a paste of them, but I'm not sure what woudl be causing
> the 'url'' to not get set (some help?)
>
> --
> https://code.launchpad.net/~smoser/maas/+git/maas/+merge/332573
> You are reviewing the proposed merge of ~smoser/maas:cleanup/render-curtin-config-internally
> into maas:master.
>

--
Andres Rodriguez
Engineering Manager, MAAS
Canonical USA, Inc.

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b cleanup/render-curtin-config-internally lp:~smoser/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/868/console
COMMIT: d33d9f6b6da6b2306e167357579fe6e4ae8764dd

review: Needs Fixing
Revision history for this message
Scott Moser (smoser) wrote :

Bump?

Revision history for this message
Mike Pontillo (mpontillo) wrote :

@smoser, I'll take a closer look. Thanks for getting this work going.

Revision history for this message
Blake Rouse (blake-rouse) wrote :

This has been sitting for a long time, closing up the old queue.

Unmerged commits

d33d9f6... by Scott Moser

fix per make format

1936d8c... by Scott Moser

fix flake8

bc88c60... by Scott Moser

drop key_b64 variable

ff2cc32... by Scott Moser

drop 'subst' helper, no longer used

cafa3eb... by Scott Moser

remove debug print

7d1215b... by Scott Moser

tests pass.

5930dfe... by Scott Moser

replace old templates with stock empty

51bd856... by Scott Moser

Internally handle most of curtin_userdata leaving templates to user.

The 'curtin_userdata' file was scary to look at, and hard to work
with for a user. Adding early_commands or late_commands meant that they
would have to edit an existing entry, and failure to do so would
break the install.

This takes what was previously in that those files and handles them
internally to maas. Thus, the user now has a blank slate to work on,
and an example of what could go there.

LP: #1734391

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/contrib/preseeds_v2/curtin_userdata b/contrib/preseeds_v2/curtin_userdata
index 4caddc4..a22a775 100644
--- a/contrib/preseeds_v2/curtin_userdata
+++ b/contrib/preseeds_v2/curtin_userdata
@@ -1,42 +1,18 @@
1#cloud-config1#curtin-config
2debconf_selections:2## Here you can put custom data that will go to each node.
3 maas: |3## The rendered output must be valid YAML.
4 {{for line in str(curtin_preseed).splitlines()}}4##
5 {{line}}5## This file will be rendered as a tempita template.
6 {{endfor}}6## variables available for the rendering are
7early_commands:7## osystem: "ubuntu", "suse", "windows", ...
8{{if third_party_drivers and driver}}8## release: "xenial", "trusty", ...
9 {{py: key_string = ''.join(['\\x%x' % x for x in driver['key_binary']])}}9## server_host: the maas facing server host
10 {{if driver['key_binary'] and driver['repository'] and driver['package']}}10## server_url: ...
11 driver_00_get_key: /bin/echo -en '{{key_string}}' > /tmp/maas-{{driver['package']}}.gpg11## syslog_host_port: ...
12 driver_01_add_key: ["apt-key", "add", "/tmp/maas-{{driver['package']}}.gpg"]12## metadata_enlist_url: ...
13 {{endif}}13## node: the 'node' object
14 {{if driver['repository']}}14## license_key
15 driver_02_add: ["add-apt-repository", "-y", "deb {{driver['repository']}} {{node.get_distro_series()}} main"]15##
16 {{endif}}16## As a simple example, consider:
17 {{if driver['package']}}17#late_commands:
18 driver_03_update_install: ["sh", "-c", "apt-get update --quiet && apt-get --assume-yes install {{driver['package']}}"]18# - ["curtin", "in-target", "--", "apt-get", "dist-upgrade"]
19 {{endif}}
20 {{if driver['module']}}
21 driver_04_load: ["sh", "-c", "depmod && modprobe {{driver['module']}} || echo 'Warning: Failed to load module: {{driver['module']}}'"]
22 {{endif}}
23{{else}}
24 driver_00: ["sh", "-c", "echo third party drivers not installed or necessary."]
25{{endif}}
26late_commands:
27 maas: [wget, '--no-proxy', {{node_disable_pxe_url|escape.json}}, '--post-data', {{node_disable_pxe_data|escape.json}}, '-O', '/dev/null']
28{{if third_party_drivers and driver}}
29 {{if driver['key_binary'] and driver['repository'] and driver['package']}}
30 driver_00_key_get: curtin in-target -- sh -c "/bin/echo -en '{{key_string}}' > /tmp/maas-{{driver['package']}}.gpg"
31 driver_02_key_add: ["curtin", "in-target", "--", "apt-key", "add", "/tmp/maas-{{driver['package']}}.gpg"]
32 {{endif}}
33 {{if driver['repository']}}
34 driver_03_add: ["curtin", "in-target", "--", "add-apt-repository", "-y", "deb {{driver['repository']}} {{node.get_distro_series()}} main"]
35 {{endif}}
36 driver_04_update_install: ["curtin", "in-target", "--", "apt-get", "update", "--quiet"]
37 {{if driver['package']}}
38 driver_05_install: ["curtin", "in-target", "--", "apt-get", "-y", "install", "{{driver['package']}}"]
39 {{endif}}
40 driver_06_depmod: ["curtin", "in-target", "--", "depmod"]
41 driver_07_update_initramfs: ["curtin", "in-target", "--", "update-initramfs", "-u"]
42{{endif}}
diff --git a/contrib/preseeds_v2/curtin_userdata_centos b/contrib/preseeds_v2/curtin_userdata_centos
index 3de9c99..2dd2023 100644
--- a/contrib/preseeds_v2/curtin_userdata_centos
+++ b/contrib/preseeds_v2/curtin_userdata_centos
@@ -1,9 +1,3 @@
1#cloud-config1#curtin-config
2debconf_selections:2## This file is used only for centos deployments.
3 maas: |3## See curtin_userdata for more information.
4 {{for line in str(curtin_preseed).splitlines()}}
5 {{line}}
6 {{endfor}}
7
8late_commands:
9 maas: [wget, '--no-proxy', '{{node_disable_pxe_url}}', '--post-data', '{{node_disable_pxe_data}}', '-O', '/dev/null']
diff --git a/contrib/preseeds_v2/curtin_userdata_windows b/contrib/preseeds_v2/curtin_userdata_windows
index 77e4a65..4b7d891 100644
--- a/contrib/preseeds_v2/curtin_userdata_windows
+++ b/contrib/preseeds_v2/curtin_userdata_windows
@@ -1,11 +1,3 @@
1#cloud-config1#cloud-config
2debconf_selections:2## This file is used only for windows deployments.
3 maas: |3## See curtin_userdata for more information.
4 {{for line in str(curtin_preseed).splitlines()}}
5 {{line}}
6 {{endfor}}
7
8late_commands:
9 maas: [wget, '--no-proxy', '{{node_disable_pxe_url}}', '--post-data', '{{node_disable_pxe_data}}', '-O', '/dev/null']
10
11license_key: {{node.get_effective_license_key()}}
diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py
index 74c8df7..1830e2f 100644
--- a/src/maasserver/node_action.py
+++ b/src/maasserver/node_action.py
@@ -44,7 +44,7 @@ from maasserver.node_status import (
44 is_failed_status,44 is_failed_status,
45 NON_MONITORED_STATUSES,45 NON_MONITORED_STATUSES,
46)46)
47from maasserver.preseed import get_curtin_config47from maasserver.preseed import get_curtin_configs
48from maasserver.utils.orm import post_commit_do48from maasserver.utils.orm import post_commit_do
49from maasserver.utils.osystems import (49from maasserver.utils.osystems import (
50 validate_hwe_kernel,50 validate_hwe_kernel,
@@ -347,7 +347,7 @@ class Deploy(NodeAction):
347 raise NodeActionError(e)347 raise NodeActionError(e)
348348
349 try:349 try:
350 get_curtin_config(self.node)350 get_curtin_configs(self.node)
351 except Exception as e:351 except Exception as e:
352 raise NodeActionError(352 raise NodeActionError(
353 "Failed to retrieve curtin config: %s" % e)353 "Failed to retrieve curtin config: %s" % e)
diff --git a/src/maasserver/preseed.py b/src/maasserver/preseed.py
index f2ed29c..d983a80 100644
--- a/src/maasserver/preseed.py
+++ b/src/maasserver/preseed.py
@@ -15,6 +15,7 @@ __all__ = [
15 'OS_WITH_IPv6_SUPPORT',15 'OS_WITH_IPv6_SUPPORT',
16 ]16 ]
1717
18import base64
18from collections import namedtuple19from collections import namedtuple
19import json20import json
20import os.path21import os.path
@@ -344,7 +345,8 @@ def compose_curtin_verbose_preseed():
344345
345def get_curtin_yaml_config(node, default_region_ip=None):346def get_curtin_yaml_config(node, default_region_ip=None):
346 """Return the curtin configration for the node."""347 """Return the curtin configration for the node."""
347 main_config = get_curtin_config(node)348 main_config = get_curtin_configs(node)
349 custom_config = get_curtin_custom_config(node)
348 cloud_config = compose_curtin_cloud_config(node)350 cloud_config = compose_curtin_cloud_config(node)
349 archive_config = compose_curtin_archive_config(node)351 archive_config = compose_curtin_archive_config(node)
350 reporter_config = compose_curtin_maas_reporter(node)352 reporter_config = compose_curtin_maas_reporter(node)
@@ -389,9 +391,9 @@ def get_curtin_yaml_config(node, default_region_ip=None):
389 storage_config = []391 storage_config = []
390392
391 return (393 return (
392 storage_config + [main_config] + archive_config + reporter_config +394 storage_config + main_config + archive_config + reporter_config +
393 network_config + swap_config + kernel_config + verbose_config +395 network_config + swap_config + kernel_config + verbose_config +
394 cloud_config)396 cloud_config + [custom_config])
395397
396398
397def get_curtin_merged_config(node):399def get_curtin_merged_config(node):
@@ -475,7 +477,7 @@ def get_curtin_installer_url(node):
475 return url_prepend + url477 return url_prepend + url
476478
477479
478def get_curtin_config(node, default_region_ip=None):480def get_curtin_preseed_context(node, default_region_ip=None):
479 """Return the curtin configuration to be used by curtin.pack_install.481 """Return the curtin configuration to be used by curtin.pack_install.
480482
481 :param node: The node for which to generate the configuration.483 :param node: The node for which to generate the configuration.
@@ -483,8 +485,6 @@ def get_curtin_config(node, default_region_ip=None):
483 """485 """
484 osystem = node.get_osystem()486 osystem = node.get_osystem()
485 series = node.get_distro_series()487 series = node.get_distro_series()
486 template = load_preseed_template(
487 node, USERDATA_TYPE.CURTIN, osystem, series)
488 rack_controller = node.get_boot_rack_controller()488 rack_controller = node.get_boot_rack_controller()
489 context = get_preseed_context(489 context = get_preseed_context(
490 osystem, series, rack_controller=rack_controller,490 osystem, series, rack_controller=rack_controller,
@@ -497,6 +497,131 @@ def get_curtin_config(node, default_region_ip=None):
497 get_curtin_context(497 get_curtin_context(
498 node, rack_controller=rack_controller,498 node, rack_controller=rack_controller,
499 default_region_ip=default_region_ip))499 default_region_ip=default_region_ip))
500 return context
501
502
503def get_third_party_drivers_config_data(enabled, key, repo, package, module,
504 series):
505 ret = {'early_commands': {}, 'late_commands': {}}
506 pre = 'driver_0'
507
508 if not enabled:
509 msg = "echo third party drivers not installed or necessary."
510 ret['early_commands'][pre + '0'] = ["echo", msg]
511 ret['late_commands'][pre + '0'] = ["echo", msg]
512 return ret
513
514 early = ret['early_commands']
515 late = ret['late_commands']
516
517 in_target = ["curtin", "in-target", "--"]
518 if key and repo and package:
519 tmpf = "/tmp/maas-" + package
520 echo_key = [
521 'sh', '-c',
522 'echo %s | base64 --decode > %s' % (base64.b64encode(key).decode(),
523 tmpf)]
524 apt_add_key = ['apt-key', 'add', tmpf]
525 early[pre + '0_get_key'] = echo_key
526 early[pre + '1_add_key'] = apt_add_key
527 late[pre + '0_key_get'] = in_target + echo_key
528 late[pre + '2_key_add'] = in_target + apt_add_key
529 if repo:
530 add_apt_repo = ["add-apt-repository", "-y",
531 "deb %s %s main" % (repo, series)]
532 early[pre + '2_add'] = add_apt_repo
533 late[pre + '3_add'] = in_target + add_apt_repo
534 if package:
535 early[pre + '3_update_install'] = [
536 "sh", "-c", ("apt-get update --quiet && "
537 "apt-get --assume-yes install " + package)]
538 late[pre + '4_update_install'] = in_target + [
539 "apt-get", "update", "--quite"]
540 late[pre + '5_install'] = in_target + [
541 "apt-get", "-y", "install", package]
542 if module:
543 early[pre + '4_load'] = [
544 "sh", "-c",
545 ("depmod && modprobe %s || " % module +
546 "echo 'Warning: Failed to load module: %s" % module)]
547
548 late[pre + '6_depmod'] = in_target + ["depmod"]
549 late[pre + '7_update_initramfs'] = in_target + ["update-initramfs", "-u"]
550 return ret
551
552
553def _get_curtin_base_config_data(context):
554 config = {'debconf_selections': {'maas': context['curtin_preseed']}}
555 config['late_commands'] = {
556 'maas': ["wget", "--no-proxy", context["node_disable_pxe_url"],
557 "--post-data", context["node_disable_pxe_data"],
558 "-O", "/dev/null"]}
559
560 # Ensure we always set debconf_selections for grub to ensure it doesn't
561 # overwrite the config sent by MAAS. See LP: #1642298
562 grub2_debconf = {'grub2': 'grub2 grub2/update_nvram boolean false'}
563 if 'debconf_selections' in config:
564 config['debconf_selections'].update(grub2_debconf)
565 else:
566 config['debconf_selections'] = grub2_debconf
567
568 # Precise does not support cloud-init performing the reboot, so curtin
569 # must have this statement.
570 if context['release'] == "precise":
571 config['power_state'] = {'mode': 'reboot'}
572
573 return config
574
575
576def get_curtin_configs_ubuntu(node):
577 """Return primary curtin config for ubuntu."""
578 context = get_curtin_preseed_context(node)
579 driver = context.get('driver', {})
580 series = node.get_distro_series()
581 return [yaml.safe_dump(c) for c in [
582 _get_curtin_base_config_data(context),
583 get_third_party_drivers_config_data(
584 enabled=context.get('third_party_drivers'),
585 key=driver.get('key_binary'),
586 repo=driver.get('repository'),
587 package=driver.get('package'),
588 module=driver.get('module'),
589 series=series)]]
590
591
592def get_curtin_configs_other(node):
593 """Return primary curtin config for non-ubuntu."""
594 context = get_curtin_preseed_context(node)
595 config = _get_curtin_base_config_data(context)
596 if context['osystem'] == "windows":
597 config['license_key'] = node.get_effective_license_key()
598
599 return [yaml.safe_dump(config)]
600
601
602def get_curtin_configs(node):
603 """Return the primary curtin configuration.
604
605 :param node: The node for which to generate the configuration.
606 :rtype: unicode."""
607 if node.get_osystem() == "ubuntu":
608 return get_curtin_configs_ubuntu(node)
609 return get_curtin_configs_other(node)
610
611
612def get_curtin_custom_config(node):
613 """Return the curtin configuration to be used by curtin.pack_install.
614
615 This reads and renders the template files in /etc/maas/preseeds.
616
617 :param node: The node for which to generate the configuration.
618 :rtype: unicode.
619 """
620 context = get_curtin_preseed_context(node)
621 osystem = node.get_osystem()
622 series = node.get_distro_series()
623 template = load_preseed_template(
624 node, USERDATA_TYPE.CURTIN, osystem, series)
500 deprecated_context_variables = [625 deprecated_context_variables = [
501 'main_archive_hostname', 'main_archive_directory',626 'main_archive_hostname', 'main_archive_directory',
502 'ports_archive_hostname', 'ports_archive_directory',627 'ports_archive_hostname', 'ports_archive_directory',
@@ -507,6 +632,8 @@ def get_curtin_config(node, default_region_ip=None):
507 deprecated_context_variables.remove(var)632 deprecated_context_variables.remove(var)
508 context.update(get_node_deprecated_preseed_context())633 context.update(get_node_deprecated_preseed_context())
509 config = yaml.safe_load(template.substitute(**context))634 config = yaml.safe_load(template.substitute(**context))
635 if not config:
636 config = {}
510 # Remove deprecated config from the curtin preseed.637 # Remove deprecated config from the curtin preseed.
511 if 'power_state' in config:638 if 'power_state' in config:
512 del config['power_state']639 del config['power_state']
diff --git a/src/maasserver/tests/test_node_action.py b/src/maasserver/tests/test_node_action.py
index a9368e0..79c46ce 100644
--- a/src/maasserver/tests/test_node_action.py
+++ b/src/maasserver/tests/test_node_action.py
@@ -514,12 +514,12 @@ class TestDeployAction(MAASServerTestCase):
514 node = factory.make_Node(514 node = factory.make_Node(
515 interface=True, status=NODE_STATUS.ALLOCATED,515 interface=True, status=NODE_STATUS.ALLOCATED,
516 power_type='manual', owner=user)516 power_type='manual', owner=user)
517 mock_get_curtin_config = self.patch(517 mock_get_curtin_configs = self.patch(
518 node_action_module, 'get_curtin_config')518 node_action_module, 'get_curtin_configs')
519 mock_node_start = self.patch(node, 'start')519 mock_node_start = self.patch(node, 'start')
520 Deploy(node, user).execute()520 Deploy(node, user).execute()
521 self.expectThat(521 self.expectThat(
522 mock_get_curtin_config, MockCalledOnceWith(node))522 mock_get_curtin_configs, MockCalledOnceWith(node))
523 self.expectThat(523 self.expectThat(
524 mock_node_start, MockCalledOnceWith(user))524 mock_node_start, MockCalledOnceWith(user))
525525
@@ -528,9 +528,9 @@ class TestDeployAction(MAASServerTestCase):
528 node = factory.make_Node(528 node = factory.make_Node(
529 interface=True, status=NODE_STATUS.ALLOCATED,529 interface=True, status=NODE_STATUS.ALLOCATED,
530 power_type='manual', owner=user)530 power_type='manual', owner=user)
531 mock_get_curtin_config = self.patch(531 mock_get_curtin_configs = self.patch(
532 node_action_module, 'get_curtin_config')532 node_action_module, 'get_curtin_configs')
533 mock_get_curtin_config.side_effect = NodeActionError('error')533 mock_get_curtin_configs.side_effect = NodeActionError('error')
534 error = self.assertRaises(534 error = self.assertRaises(
535 NodeActionError, Deploy(node, user).execute)535 NodeActionError, Deploy(node, user).execute)
536 self.assertEqual(536 self.assertEqual(
@@ -559,8 +559,8 @@ class TestDeployAction(MAASServerTestCase):
559 node = factory.make_Node(559 node = factory.make_Node(
560 interface=True, status=NODE_STATUS.ALLOCATED,560 interface=True, status=NODE_STATUS.ALLOCATED,
561 power_type='manual', owner=user)561 power_type='manual', owner=user)
562 mock_get_curtin_config = self.patch(562 mock_get_curtin_configs = self.patch(
563 node_action_module, 'get_curtin_config')563 node_action_module, 'get_curtin_configs')
564 mock_node_start = self.patch(node, 'start')564 mock_node_start = self.patch(node, 'start')
565 osystem = make_usable_osystem(self)565 osystem = make_usable_osystem(self)
566 os_name = osystem["name"]566 os_name = osystem["name"]
@@ -571,7 +571,7 @@ class TestDeployAction(MAASServerTestCase):
571 }571 }
572 Deploy(node, user).execute(**extra)572 Deploy(node, user).execute(**extra)
573 self.expectThat(573 self.expectThat(
574 mock_get_curtin_config, MockCalledOnceWith(node))574 mock_get_curtin_configs, MockCalledOnceWith(node))
575 self.expectThat(575 self.expectThat(
576 mock_node_start, MockCalledOnceWith(user))576 mock_node_start, MockCalledOnceWith(user))
577 self.expectThat(node.osystem, Equals(os_name))577 self.expectThat(node.osystem, Equals(os_name))
@@ -583,8 +583,8 @@ class TestDeployAction(MAASServerTestCase):
583 node = factory.make_Node(583 node = factory.make_Node(
584 interface=True, status=NODE_STATUS.ALLOCATED,584 interface=True, status=NODE_STATUS.ALLOCATED,
585 power_type='manual', owner=user)585 power_type='manual', owner=user)
586 mock_get_curtin_config = self.patch(586 mock_get_curtin_configs = self.patch(
587 node_action_module, 'get_curtin_config')587 node_action_module, 'get_curtin_configs')
588 mock_node_start = self.patch(node, 'start')588 mock_node_start = self.patch(node, 'start')
589 osystem = make_usable_osystem(self)589 osystem = make_usable_osystem(self)
590 os_name = osystem["name"]590 os_name = osystem["name"]
@@ -595,7 +595,7 @@ class TestDeployAction(MAASServerTestCase):
595 }595 }
596 Deploy(node, user).execute(**extra)596 Deploy(node, user).execute(**extra)
597 self.expectThat(597 self.expectThat(
598 mock_get_curtin_config, MockCalledOnceWith(node))598 mock_get_curtin_configs, MockCalledOnceWith(node))
599 self.expectThat(599 self.expectThat(
600 mock_node_start, MockCalledOnceWith(user))600 mock_node_start, MockCalledOnceWith(user))
601 self.expectThat(node.osystem, Equals(os_name))601 self.expectThat(node.osystem, Equals(os_name))
@@ -607,8 +607,8 @@ class TestDeployAction(MAASServerTestCase):
607 node = factory.make_Node(607 node = factory.make_Node(
608 interface=True, status=NODE_STATUS.ALLOCATED,608 interface=True, status=NODE_STATUS.ALLOCATED,
609 power_type='manual', owner=user)609 power_type='manual', owner=user)
610 mock_get_curtin_config = self.patch(610 mock_get_curtin_configs = self.patch(
611 node_action_module, 'get_curtin_config')611 node_action_module, 'get_curtin_configs')
612 mock_node_start = self.patch(node, 'start')612 mock_node_start = self.patch(node, 'start')
613 osystem = make_osystem_with_releases(self)613 osystem = make_osystem_with_releases(self)
614 extra = {614 extra = {
@@ -616,7 +616,7 @@ class TestDeployAction(MAASServerTestCase):
616 }616 }
617 Deploy(node, user).execute(**extra)617 Deploy(node, user).execute(**extra)
618 self.expectThat(618 self.expectThat(
619 mock_get_curtin_config, MockCalledOnceWith(node))619 mock_get_curtin_configs, MockCalledOnceWith(node))
620 self.expectThat(620 self.expectThat(
621 mock_node_start, MockCalledOnceWith(user))621 mock_node_start, MockCalledOnceWith(user))
622 self.expectThat(node.osystem, Equals(""))622 self.expectThat(node.osystem, Equals(""))
@@ -627,8 +627,8 @@ class TestDeployAction(MAASServerTestCase):
627 node = factory.make_Node(627 node = factory.make_Node(
628 interface=True, status=NODE_STATUS.ALLOCATED,628 interface=True, status=NODE_STATUS.ALLOCATED,
629 power_type='manual', owner=user)629 power_type='manual', owner=user)
630 mock_get_curtin_config = self.patch(630 mock_get_curtin_configs = self.patch(
631 node_action_module, 'get_curtin_config')631 node_action_module, 'get_curtin_configs')
632 mock_node_start = self.patch(node, 'start')632 mock_node_start = self.patch(node, 'start')
633 osystem = make_osystem_with_releases(self)633 osystem = make_osystem_with_releases(self)
634 extra = {634 extra = {
@@ -636,7 +636,7 @@ class TestDeployAction(MAASServerTestCase):
636 }636 }
637 Deploy(node, user).execute(**extra)637 Deploy(node, user).execute(**extra)
638 self.expectThat(638 self.expectThat(
639 mock_get_curtin_config, MockCalledOnceWith(node))639 mock_get_curtin_configs, MockCalledOnceWith(node))
640 self.expectThat(640 self.expectThat(
641 mock_node_start, MockCalledOnceWith(user))641 mock_node_start, MockCalledOnceWith(user))
642 self.expectThat(node.osystem, Equals(""))642 self.expectThat(node.osystem, Equals(""))
@@ -645,14 +645,14 @@ class TestDeployAction(MAASServerTestCase):
645 def test_Deploy_allocates_node_if_node_not_already_allocated(self):645 def test_Deploy_allocates_node_if_node_not_already_allocated(self):
646 user = factory.make_User()646 user = factory.make_User()
647 node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)647 node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
648 mock_get_curtin_config = self.patch(648 mock_get_curtin_configs = self.patch(
649 node_action_module, 'get_curtin_config')649 node_action_module, 'get_curtin_configs')
650 mock_node_start = self.patch(node, 'start')650 mock_node_start = self.patch(node, 'start')
651 action = Deploy(node, user)651 action = Deploy(node, user)
652 action.execute()652 action.execute()
653653
654 self.expectThat(654 self.expectThat(
655 mock_get_curtin_config, MockCalledOnceWith(node))655 mock_get_curtin_configs, MockCalledOnceWith(node))
656 self.expectThat(656 self.expectThat(
657 mock_node_start, MockCalledOnceWith(user))657 mock_node_start, MockCalledOnceWith(user))
658 self.expectThat(user, Equals(node.owner))658 self.expectThat(user, Equals(node.owner))
diff --git a/src/maasserver/tests/test_preseed.py b/src/maasserver/tests/test_preseed.py
index 6f5e7d6..bde4bc9 100644
--- a/src/maasserver/tests/test_preseed.py
+++ b/src/maasserver/tests/test_preseed.py
@@ -13,6 +13,7 @@ from textwrap import dedent
13from unittest.mock import sentinel13from unittest.mock import sentinel
14from urllib.parse import urlparse14from urllib.parse import urlparse
1515
16from curtin.config import merge_config
16from django.conf import settings17from django.conf import settings
17from maasserver import preseed as preseed_module18from maasserver import preseed as preseed_module
18from maasserver.clusterrpc.testing.boot_images import make_rpc_boot_image19from maasserver.clusterrpc.testing.boot_images import make_rpc_boot_image
@@ -47,7 +48,7 @@ from maasserver.preseed import (
47 curtin_maas_reporter,48 curtin_maas_reporter,
48 GENERIC_FILENAME,49 GENERIC_FILENAME,
49 get_curtin_cloud_config,50 get_curtin_cloud_config,
50 get_curtin_config,51 get_curtin_configs,
51 get_curtin_context,52 get_curtin_context,
52 get_curtin_image,53 get_curtin_image,
53 get_curtin_installer_url,54 get_curtin_installer_url,
@@ -126,6 +127,14 @@ class BootImageHelperMixin:
126 preseed_module,127 preseed_module,
127 'get_boot_images_for').return_value = [boot_image]128 'get_boot_images_for').return_value = [boot_image]
128129
130 def get_merged_config(self, result):
131 """Return the fully merged config from the list of yaml_blobs
132 that is provided in result (as returned by get_curtin_configs)."""
133 ret = {}
134 for yaml_blob in result:
135 merge_config(ret, yaml.safe_load(yaml_blob))
136 return ret
137
129138
130class TestSplitSubArch(MAASServerTestCase):139class TestSplitSubArch(MAASServerTestCase):
131 """Tests for `split_subarch`."""140 """Tests for `split_subarch`."""
@@ -977,8 +986,9 @@ class TestRenderCurtinUserdataWithThirdPartyDrivers(
977 get_third_party_driver = self.patch(986 get_third_party_driver = self.patch(
978 preseed_module, "get_third_party_driver")987 preseed_module, "get_third_party_driver")
979 get_third_party_driver.return_value = self.driver988 get_third_party_driver.return_value = self.driver
980 curtin_config_text = get_curtin_config(node)989 config = self.get_merged_config(get_curtin_configs(node))
981 config = yaml.safe_load(curtin_config_text)990 self.assertThat(config, Contains('early_commands'))
991
982 self.assertThat(992 self.assertThat(
983 config['early_commands'], Contains('driver_00_get_key'))993 config['early_commands'], Contains('driver_00_get_key'))
984 self.assertThat(994 self.assertThat(
@@ -1035,11 +1045,11 @@ class TestCurtinUtilities(
1035 node = factory.make_Node_with_Interface_on_Subnet(1045 node = factory.make_Node_with_Interface_on_Subnet(
1036 primary_rack=self.rpc_rack_controller)1046 primary_rack=self.rpc_rack_controller)
1037 self.configure_get_boot_images_for_node(node, 'xinstall')1047 self.configure_get_boot_images_for_node(node, 'xinstall')
1038 config = get_curtin_config(node)1048 config = self.get_merged_config(get_curtin_configs(node))
1039 self.assertThat(1049 self.assertIsNotNone(config.get('early_commands'),
1040 config,1050 "Did not find 'early_commands' entry")
1041 Contains("debconf_selections:"))1051 self.assertNotEqual(
1042 self.assertThat(config, Not(Contains('mode: reboot')))1052 config.get('power_state', {}).get('mode', None), 'reboot')
10431053
1044 def test_get_curtin_config_removes_power_state(self):1054 def test_get_curtin_config_removes_power_state(self):
1045 node = factory.make_Node_with_Interface_on_Subnet(1055 node = factory.make_Node_with_Interface_on_Subnet(
@@ -1051,8 +1061,9 @@ class TestCurtinUtilities(
1051 """)1061 """)
1052 self.patch(preseed_module, "get_preseed_template").return_value = (1062 self.patch(preseed_module, "get_preseed_template").return_value = (
1053 factory.make_name("filename"), power_state_template)1063 factory.make_name("filename"), power_state_template)
1054 config = get_curtin_config(node)1064 config = self.get_merged_config(get_curtin_configs(node))
1055 self.assertThat(config, Not(Contains('mode: reboot')))1065 self.assertNotEqual(
1066 'reboot', config.get('power_state', {}).get('mode'), 'reboot')
10561067
1057 def test_get_curtin_config_removes_apt_mirrors(self):1068 def test_get_curtin_config_removes_apt_mirrors(self):
1058 node = factory.make_Node_with_Interface_on_Subnet(1069 node = factory.make_Node_with_Interface_on_Subnet(
@@ -1065,9 +1076,9 @@ class TestCurtinUtilities(
1065 """)1076 """)
1066 self.patch(preseed_module, "get_preseed_template").return_value = (1077 self.patch(preseed_module, "get_preseed_template").return_value = (
1067 factory.make_name("filename"), apt_mirrors_template)1078 factory.make_name("filename"), apt_mirrors_template)
1068 config = get_curtin_config(node)1079 config = self.get_merged_config(get_curtin_configs(node))
1069 self.assertThat(config, Not(Contains('ubuntu_archive')))1080 self.assertNotIn('ubuntu_archive', config.get('apt_mirrors', {}))
1070 self.assertThat(config, Not(Contains('ubuntu_security')))1081 self.assertNotIn('ubuntu_security', config.get('apt_mirrors', {}))
10711082
1072 def test_get_curtin_config_removes_apt_proxy(self):1083 def test_get_curtin_config_removes_apt_proxy(self):
1073 node = factory.make_Node_with_Interface_on_Subnet(1084 node = factory.make_Node_with_Interface_on_Subnet(
@@ -1078,8 +1089,9 @@ class TestCurtinUtilities(
1078 """)1089 """)
1079 self.patch(preseed_module, "get_preseed_template").return_value = (1090 self.patch(preseed_module, "get_preseed_template").return_value = (
1080 factory.make_name("filename"), apt_proxy_template)1091 factory.make_name("filename"), apt_proxy_template)
1081 config = get_curtin_config(node)1092 config = self.get_merged_config(get_curtin_configs(node))
1082 self.assertThat(config, Not(Contains('127.0.0.1')))1093 self.assertThat(config.get('apt_proxy', ''),
1094 Not(Contains('127.0.0.1')))
10831095
1084 def test_get_curtin_config_contains_reboot_for_precise(self):1096 def test_get_curtin_config_contains_reboot_for_precise(self):
1085 node = factory.make_Node_with_Interface_on_Subnet(1097 node = factory.make_Node_with_Interface_on_Subnet(
@@ -1087,8 +1099,8 @@ class TestCurtinUtilities(
1087 node.distro_series = "precise"1099 node.distro_series = "precise"
1088 node.save()1100 node.save()
1089 self.configure_get_boot_images_for_node(node, 'xinstall')1101 self.configure_get_boot_images_for_node(node, 'xinstall')
1090 config = get_curtin_config(node)1102 config = self.get_merged_config(get_curtin_configs(node))
1091 self.assertThat(config, Contains('mode: reboot'))1103 self.assertEqual(config['power_state']['mode'], "reboot")
10921104
1093 def test_get_curtin_config_with_ipv4_rack_url(self):1105 def test_get_curtin_config_with_ipv4_rack_url(self):
1094 primary_rack = self.rpc_rack_controller1106 primary_rack = self.rpc_rack_controller
@@ -1097,13 +1109,12 @@ class TestCurtinUtilities(
1097 node = factory.make_Node_with_Interface_on_Subnet(1109 node = factory.make_Node_with_Interface_on_Subnet(
1098 primary_rack=primary_rack)1110 primary_rack=primary_rack)
1099 self.configure_get_boot_images_for_node(node, 'xinstall')1111 self.configure_get_boot_images_for_node(node, 'xinstall')
1100 config = get_curtin_config(node)1112 config = self.get_merged_config(get_curtin_configs(node))
1101 yaml_conf = yaml.safe_load(config)
1102 self.assertEqual(1113 self.assertEqual(
1103 "%smetadata/latest/by-id/%s/" % (1114 "%smetadata/latest/by-id/%s/" % (
1104 primary_rack.url, node.system_id),1115 primary_rack.url, node.system_id),
1105 yaml_conf['late_commands']['maas'][2])1116 config['late_commands']['maas'][2])
1106 self.assertTrue('debconf_selections' in yaml_conf)1117 self.assertTrue('debconf_selections' in config)
11071118
1108 def test_get_curtin_config_with_ipv6_rack_url(self):1119 def test_get_curtin_config_with_ipv6_rack_url(self):
1109 primary_rack = self.rpc_rack_controller1120 primary_rack = self.rpc_rack_controller
@@ -1112,13 +1123,12 @@ class TestCurtinUtilities(
1112 node = factory.make_Node_with_Interface_on_Subnet(1123 node = factory.make_Node_with_Interface_on_Subnet(
1113 primary_rack=primary_rack)1124 primary_rack=primary_rack)
1114 self.configure_get_boot_images_for_node(node, 'xinstall')1125 self.configure_get_boot_images_for_node(node, 'xinstall')
1115 config = get_curtin_config(node)1126 config = self.get_merged_config(get_curtin_configs(node))
1116 yaml_conf = yaml.safe_load(config)
1117 self.assertEqual(1127 self.assertEqual(
1118 "%smetadata/latest/by-id/%s/" % (1128 "%smetadata/latest/by-id/%s/" % (
1119 primary_rack.url, node.system_id),1129 primary_rack.url, node.system_id),
1120 yaml_conf['late_commands']['maas'][2])1130 config['late_commands']['maas'][2])
1121 self.assertTrue('debconf_selections' in yaml_conf)1131 self.assertTrue('debconf_selections' in config)
11221132
1123 def test_get_curtin_config_with_name_rack_url(self):1133 def test_get_curtin_config_with_name_rack_url(self):
1124 primary_rack = self.rpc_rack_controller1134 primary_rack = self.rpc_rack_controller
@@ -1127,13 +1137,12 @@ class TestCurtinUtilities(
1127 node = factory.make_Node_with_Interface_on_Subnet(1137 node = factory.make_Node_with_Interface_on_Subnet(
1128 primary_rack=primary_rack)1138 primary_rack=primary_rack)
1129 self.configure_get_boot_images_for_node(node, 'xinstall')1139 self.configure_get_boot_images_for_node(node, 'xinstall')
1130 config = get_curtin_config(node)1140 config = self.get_merged_config(get_curtin_configs(node))
1131 yaml_conf = yaml.safe_load(config)
1132 self.assertEqual(1141 self.assertEqual(
1133 "%smetadata/latest/by-id/%s/" % (1142 "%smetadata/latest/by-id/%s/" % (
1134 primary_rack.url, node.system_id),1143 primary_rack.url, node.system_id),
1135 yaml_conf['late_commands']['maas'][2])1144 config['late_commands']['maas'][2])
1136 self.assertTrue('debconf_selections' in yaml_conf)1145 self.assertTrue('debconf_selections' in config)
11371146
1138 def test_get_curtin_config_with_quote_rack_url(self):1147 def test_get_curtin_config_with_quote_rack_url(self):
1139 primary_rack = self.rpc_rack_controller1148 primary_rack = self.rpc_rack_controller
@@ -1142,22 +1151,21 @@ class TestCurtinUtilities(
1142 node = factory.make_Node_with_Interface_on_Subnet(1151 node = factory.make_Node_with_Interface_on_Subnet(
1143 primary_rack=primary_rack)1152 primary_rack=primary_rack)
1144 self.configure_get_boot_images_for_node(node, 'xinstall')1153 self.configure_get_boot_images_for_node(node, 'xinstall')
1145 config = get_curtin_config(node)1154 config = self.get_merged_config(get_curtin_configs(node))
1146 yaml_conf = yaml.safe_load(config)
1147 self.assertEqual(1155 self.assertEqual(
1148 "%smetadata/latest/by-id/%s/" % (primary_rack.url, node.system_id),1156 "%smetadata/latest/by-id/%s/" % (primary_rack.url, node.system_id),
1149 yaml_conf['late_commands']['maas'][2])1157 config['late_commands']['maas'][2])
1150 self.assertTrue('debconf_selections' in yaml_conf)1158 self.assertTrue('debconf_selections' in config)
11511159
1152 def test_get_curtin_config_has_grub2_debconf_selections(self):1160 def test_get_curtin_config_has_grub2_debconf_selections(self):
1153 node = factory.make_Node_with_Interface_on_Subnet(1161 node = factory.make_Node_with_Interface_on_Subnet(
1154 primary_rack=self.rpc_rack_controller)1162 primary_rack=self.rpc_rack_controller)
1155 node.save()1163 node.save()
1156 self.configure_get_boot_images_for_node(node, 'xinstall')1164 self.configure_get_boot_images_for_node(node, 'xinstall')
1157 config = get_curtin_config(node)1165 config = self.get_merged_config(get_curtin_configs(node))
1158 self.assertThat(1166 self.assertEqual(
1159 config,1167 config['debconf_selections']['grub2'].split(),
1160 Contains('grub2: grub2 grub2/update_nvram boolean false'))1168 ['grub2', 'grub2/update_nvram', 'boolean', 'false'])
11611169
1162 def make_fastpath_node(self, main_arch=None):1170 def make_fastpath_node(self, main_arch=None):
1163 """Return a `Node`, with FPI enabled, and the given main architecture.1171 """Return a `Node`, with FPI enabled, and the given main architecture.

Subscribers

People subscribed via source and target branches