cloud-init-per mishandles commands with dashes

Bug #1812676 reported by Vitaly Kuznetsov on 2019-01-21
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
cloud-init
Undecided
Unassigned

Bug Description

It was found that when there is a dash in cloud-init-per command
name and cloud-init-per is executed through cloud-init's bootcmd, e.g:

bootcmd:
- cloud-init-per instance mycmd-bootcmd /usr/bin/mycmd

the command is executed on each boot. However, running the same
cloud-init-per command manually after boot doesn't reveal the issue. Turns
out the issue comes from 'migrator' cloud-init module which renames all
files in /var/lib/cloud/instance/sem/ replacing dashes with underscores. As
migrator runs before bootcmd it renames

/var/lib/cloud/instance/sem/bootper.mycmd-bootcmd.instance
to
/var/lib/cloud/instance/sem/bootper.mycmd_bootcmd.instance

so cloud-init-per doesn't see it and thinks that the comment was never ran
before. On next boot the sequence repeats.

There are multiple ways to resolve the issue. This patch takes the
following approach: 'canonicalize' sem names by replacing dashes with
underscores (this is consistent with post-'migrator' contents of
/var/lib/cloud/instance/sem/). We, however, need to be careful: in case
someone had a command with dashes before and he had migrator module enables
we need to see the old sem file (or the command will run again and this can
be as bad as formatting a partition!) so we add a small 'migrator' part to
cloud-init-per script itself checking for legacy sem names.

Related branches

Marc Tamsky (tamsky) wrote :

After reporting this in issue in 2015 (https://bugs.launchpad.net/cloud-init/+bug/1494040),
and describing the root cause being the migrator module coupled with the #cloud-config default of
 'migrate: true'

I'm left feeling that all of it violates the principle of least-surprise.

IMHO, this bug's (now-merged) patch is one-more-change in a parade of poor engineering decisions surrounding the migrator module.

I have been unable to find ANY justification, or story, that supports the existence of the migrator module.

Period.

Please just remove it, or set the default to 'migrate: false', so those who may need it can enable it, and the rest of us can stop losing time and hair??

Please?

Changed in cloud-init:
status: New → Fix Committed
Marc Tamsky (tamsky) wrote :

The current committed patch LGTM.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers