Merge lp:~blake-rouse/maas/fix-1646571 into lp:~maas-committers/maas/trunk

Proposed by Blake Rouse
Status: Rejected
Rejected by: MAAS Lander
Proposed branch: lp:~blake-rouse/maas/fix-1646571
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 12 lines (+1/-1)
1 file modified
src/maasserver/compose_preseed.py (+1/-1)
To merge this branch: bzr merge lp:~blake-rouse/maas/fix-1646571
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Abstain
Scott Moser (community) Needs Fixing
Review via email: mp+322643@code.launchpad.net

Commit message

Never preserve sources list for cloud-init.

This prevents cloud-init from warning saying that v3 of the apt configure is different then the v2 version. v3 always has this field set to False, same is used for v2 now so this warning is not shown.

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

I tested commissioning and deployment with this change and had no issues. Don't really know if there was ever a reason to have it set to True. Be in the new v3 apt core we always set it to False.

lp:~blake-rouse/maas/fix-1646571 updated
5985. By Blake Rouse

Link bug.

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

I'm on the fence between approving this branch and Needs Information. I think this is generally okay; it should work for standard Ubuntu images and will ensure the apt configuration is consistent.

Bu will this break custom images which use an apt configuration different than what cloud-init pulls down?

Here's where it originally landed:

http://bazaar.launchpad.net/~maas-committers/maas/trunk/revision/1172?start_revid=1172

So the original reasons were:

 - Not breaking custom (ARM) configurations.
 - Not overwriting "a local mirror that was specified somewhere else" (I assume specified in the image, as I suggested earlier, but might this cause a deployment to revert to MAAS repositories on future boots, even after someone deploys a node and customizes it?)

Just going to throw this out there, but if someone deploys a different Debian-based distribution with MAAS, would this change break them, too, by overwriting the Debian repositories with MAAS repositories?

Given that this is already used with the APT v3 core (when was that introduced? assuming we haven't heard any complaints?) then I have to say, this is probably safe. So I'll approve, but let's think about the above before landing this.

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

Yeah I was worried about the change as well. But with the new v3 we always say to never preserve the apt source. I think I am going to let this sit for a while before merging. I make wait until 2.3 to merge this one, seems dangerous for older versions of cloud-init in older releases.

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

I'm pretty sure your change here is wrong.
If you tell cloud-init:
  v2: {apt_preserve_sources_list: False}
or
  v3: apt: {preserve_sources_list: False}

"preserve sources list", means that cloud-init should preserve the content in /etc/apt/sources.list, rather than rendering it from its template with mirror information that it finds or is given.

When cloud-init executes in the *installed* environment you do not want it to render /etc/apt/sources.list, as you've already told curtin to write that file with the maas configured mirrors.

So the values you want to have:
a.) curtin (always): preserve_sources_list = True
b.) cloud-init (ephemeral environment): preserve_sources_list = False
c.) cloud-init (installed environment): preserve_sources_list = True

'c' is because 'a' already updated the list and you're not giving the installed environment information about local mirrors.

'b' is because you *are* providing the ephemeral environment with local mirror information, and you need cloud-init to use that information in order to install any packages (or configure the ephemeral /etc/apt/sources.list so that curtin can install dependencies).

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

Well, 'a' is kind of arguable, and that is the "custom image" path that you're talking about.
Right now you tell it 'false', which means curtin will do that.

if you tell curtin 'preserve_sources_list' of False, then it knows to preseed cloud-init with 'preserve_sources_list' True.

this makes sense, because you told the installer what the sources.list should have, so there is no reason for cloud-init to do its changes.

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

I'll defer to Scott's expertise here. Thanks Scott.

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

Thanks for the information Scott. I don't feel comfortable landing this until the 2.3 window opens. So I am going to mark it WIP for now.

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

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

5985. By Blake Rouse

Link bug.

5984. By Blake Rouse

Never preserve sources list.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/compose_preseed.py'
2--- src/maasserver/compose_preseed.py 2017-03-29 21:13:51 +0000
3+++ src/maasserver/compose_preseed.py 2017-04-17 21:03:18 +0000
4@@ -203,7 +203,7 @@
5 # will query the DNS server (and not return 127.0.0.1).
6 # See bug 1087183 for details.
7 "manage_etc_hosts": False,
8- "apt_preserve_sources_list": True,
9+ "apt_preserve_sources_list": False,
10 # Prevent the node from requesting cloud-init data on every reboot.
11 # This is done so a machine does not need to contact MAAS every time
12 # it reboots.