ec2-set-defaults should be 'run_once_per_ami'

The decision here is a tough call as we're trying to meet a couple major use cases:

1. User runs a base AMI; installs some software; rebundles to create another AMI which is them migrated to multiple regions. User wants (or wouldn't mind) instances of the new AMI to automatically detect the region and update local and sources.

2. User runs a base AMI; sets the locale and/or tweaks the apt/sources.list to her preferences; rebundles to create another AMI. User does *not* want the local and apt sources changes overwritten in instances of the new AMI.

If it's an either/or, then I would think that overwriting 2's customizations is a more serious violation than not updating the locale or apt/sources when the 1's fire off a rebundled, migrated AMI. I would wager that very few people are in bucket 1 anyway as most EC2 users probably stick with a single region.

Most 1's and 2's could be made happy by only overwriting the locale and/or apt sources if they haven't been changed since the first boot. Perhaps a copy (or hash) could be saved in /var/lib/ec2/ and compared on the boot of a new AMI.

The logic would then be something like:

once_per_ami:
  determine recommended apt/sources for current EC2 region
  if /var/lib/ec2/last-saved-apt-sources.list does not exist
    or /var/lib/ec2/last-saved-apt-sources.list is the same as /etc/apt/sources.list
  then
    install recommended to /etc/apt/sources.list
    save recommended to /var/lib/ec2/last-saved-apt-sources.list
  [same for locale]

There could be some additional logic to not overwrite /etc/apt/sources.list if it is the same as the recommended, but I figured that would just clutter the notes.

> The decision here is a tough call as we're trying to meet a couple major
> use cases:
>
> 1. User runs a base AMI; installs some software; rebundles to create
> another AMI which is them migrated to multiple regions. User wants (or
> wouldn't mind) instances of the new AMI to automatically detect the
> region and update local and sources.

This is, the one I was thinking of. It feels to me like this would be
more common. I think we should be striving for a OS that "just works",
and the user doesn't really even think about changing the OS behavior, but
rather adding their apps on top.

> 2. User runs a base AMI; sets the locale and/or tweaks the
> apt/sources.list to her preferences; rebundles to create another AMI.
> User does *not* want the local and apt sources changes overwritten in
> instances of the new AMI.

If ec2-set-defaults runs 'once-per-ami', the user can force their changes
by changing the template files in /etc/ec2-init/templates/. Ie, if they
want to add sources to the sources.list, or set the locale, they can
modify /etc/ec2-init/templates/sources.list.tmpl or locale.tmpl
respectively.

I realize this isn't well documented, but as it is right now thats how
that *could* work. This way, the user is more explicitly overriding "just
work" behavior.

The image will still "just work" if it is run in a different region without the locale and sources.list being changed to reflect that region.

The image will likely break if the user has made explicit changes which are then overwritten.

I say trust the user to know what they are doing if they have modified these settings, and don't wipe them out.

My proposed approach above lets the image reflect the best-guess rules if the user has not spoken, but lets the user's changes take precedence if they exist.

I've got a working implementation that checks if the user has changed the file, and if not, reruns the script. I'm adding unit tests, so it'll be another while before I'll upload.

I'm marking this Low based on my understanding of the importance metrics, but since this can break a user's EC2 image on rebundling, I'd love to see the fix released.

I'll make this argument one last time, then I promise to let it be.
I believe that the correct fix for this bug is the attached patch below.

What it does, is simply put comment headers in the files that are modified by ec2-set-defaults to inform the user that they should be modifying the template file if they want their changes to be carried through a re-bundle.

The reasons I think this is the best approach are:
a.) "by default" allows the user to take advantage of the run-time decisions that ec2-set-defaults is making. The example that I think of that doesn't fit "if the user changed this, then leave it be", is that if the user added a /etc/apt/sources.list entry, rebundles their' US region image with that change, and them publishes to EU region, they'll not use the EU region mirror.
b.) documents to the user the behavior in a obvious place (the config file they're changing)
c.) allows the user to ignore changes ec2-set-defaults would make (by removing the variables in the template)

simpler change, simply put the headers in the template files rather than generating them on the fly.

The issue with the above patches (comment 6 and comment 7) is that they possibly trump changes the user made that they intended to cross the re-bundling. If the user made the change with a tool, then they possibly wouldn't see the file and warning that was written in it. Such an example is that locale is changed/modified with 'update-locale' rather than by hand editing /etc/default/locale.

I think the gist of all of this is that the template system is a more powerful system, and has good reason for existance. Not re-generating the modified files on every "first boot", means the user loses the power that the templating system gained us. As i pointed out, simply overwriting only if the file is different from the one you *would* write isn't sufficient, as it doesn't take into consideration the fact that the user is only able to write one file, while the template system outputs multiple based on environment conditions.

At the moment, I think there is no obvious solution here. I think I'm pretty much at the conclusion that we should put a header in the template files below, and not change anything else.

If the user wants to have the template system run on re-bundle, then they should simply remove the lock file before rebundling:
  rm /var/lib/ec2/ec2-defaults.ever

If, for some reason they wanted to not re-run on the system that was being re-bundled on the next boot, then they could just:
 touch /var/lib/ec2/ec2-defaults.<ami_id>

Scott: What do you mean by "not change anything else"? If the user modifies sources.list would a rebundled AMI overwrite their changes or not?

I'm also not clear which <ami_id> is referenced in the last step. If it is the AMI being created, then the user does not know the id yet.

Here's where I've ended up. I think the most sane thing at the moment is to simply document the current behavior.

On Mon, 28 Sep 2009, Eric Hammond wrote:

> Scott: What do you mean by "not change anything else"? If the user
> modifies sources.list would a rebundled AMI overwrite their changes or
> not?

I meant that we should leave ec2-init behavior as it is right now, with
'run-once-ever' for ec2-set-defaults.

> I'm also not clear which <ami_id> is referenced in the last step. If it
> is the AMI being created, then the user does not know the id yet.

'<ami_id>' was referring to the current ami_id of the image being
rebundled. I just wanted to acknowledge that if you remove the '.ever'
file, and then reboot the instance any local changes you've made are going
to be blown away in *this* instance. Its probably not a big deal. But,
to demonstrate, something like:

$ echo "LANG=en_CA.UTF-8" > /etc/default/locale
$ rm /var/lib/ec2/ec2-defaults.ever
$ do-the-rebundle
$ reboot
...
$ cat /etc/default/locale
LANG=en_US.UTF-8

After 'reboot' on *this* current instance, /etc/default/locale would get
rewritten because ec2-set-defaults would run, because there was no '.ever'
file stopping it from running. I incorrectly thought that touching the
'.<ami_id>' file would prevent the run after reboot.

I'm removing the milestone for this. As I've personally come to the conclusion that it is mostly functioning as it should.

I've talked with soren, and he said he's fine with any solution here. As I've pointed out, I think that, at this point, this is reasonably working. I'm closing this as 'Invalid'.