Merge ~johnguthrie/cloud-init:instance_hostname_subst_for_chef into cloud-init:master

[John Guthrie]

Substitutions in cloud config data when using chef

In cloudinit/config/cc_puppet.py, the certname key will support substitutions for %i and %f. This patch adds similar support for chef. This patch adds two substitutions, %I and %i, that can each be used with the fqdn and node_name keys for chef. %I can be used for the instance ID of the machine. %i can be used for the "stripped" instance ID, that is the instance ID without an initial "i-". The reasoning for this is so that people aren't forced to use hostnames like ntp-i-abcd1234, and instead can have a hostname like ntp-abcd1234.

I know that right now, this is a bit AWS-centric, but I hope to add some more intelligence to determine stripped instance IDs for use with other services.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:0075d7fc4998acc008f5700720f440915dfc6890
https://jenkins.ubuntu.com/server/job/cloud-init-ci/389/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/389/rebuild

review: Needs Fixing (continuous-integration)

faba968... by John Guthrie <email address hidden> on 2017-10-06

Adding spaces after commas to make linter happy

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:faba9686ccfec7eafddb7d2b8f6f0d672a82a0ce
https://jenkins.ubuntu.com/server/job/cloud-init-ci/390/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/390/rebuild

review: Needs Fixing (continuous-integration)

38cafcc... by John Guthrie <email address hidden> on 2017-10-06

Need to address the case where we aren't necessarily in a cloud with an instance ID

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:38cafcc3feedfe652c015938a7d292acf85c9d07
https://jenkins.ubuntu.com/server/job/cloud-init-ci/391/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/391/rebuild

review: Needs Fixing (continuous-integration)

e9c9955... by John Guthrie <email address hidden> on 2017-10-06

The datasource could be None as well

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:e9c9955bcfa5be0f18ab296fa372f25fd8f9a060
https://jenkins.ubuntu.com/server/job/cloud-init-ci/392/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/392/rebuild

review: Approve (continuous-integration)

[John Guthrie (johnguthrie)]

I justs linked bug https://bugs.launchpad.net/cloud-init/+bug/1020695 since it was talking about variable substitutions as well, and it was referenced by this email thread, https://lists.launchpad.net/cloud-init/msg00013.html, which actually discusses the exact issue that this patch tries to fix. Now the above bug is talking about variable substitutions in the /etc/hosts manager, rather than the chef manager. So do we need to create some sort substitution framework?

[Chad Smith (chad.smith)]

Thanks John for this submittal I'll talk with the team tomorrow and see what folks think about this approach.

While this branch is trying to solve a corner case I feel like we may need to go with a more general framework as you suggested. I have a branch up that'd be the foundation for such a framework [1], as it provides standardized field names which I believe we could ultimately use to substitute cloud-init instance-data variables into any cloud-init module configuration template.

As per stripping the the leading 'i-', it feels very 'corner-casey'. I think we might need a more generic facility for manipulating instance data variables. I'll try to see if I can draw up something that we can poke holes in.

References:
[1] https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/330115

[Scott Moser (smoser)]

More references
 https://trello.com/c/AYaCdQyT

Thats our trello card for this.

[Chad Smith (chad.smith)]

Updated work-in-progress branch that I'll probably sort out next week on this. It's a more generic approach that let's you use jinja templates in any cloud config module
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/334030

The examples I posted were for custom scripts and puppet, but the same applies to chef.

The jinja template supports the following needs you expressed:
 - %i could be serviced in jinja with {{ v1.instance_id|replace('i-', '') }}
 - %f serviced by {{ v1.fqdn }}

the benefit here, is a generic application for all modules, not just chef

[Chad Smith (chad.smith)]

Thank you again for this sensible suggestion. As mentioned in the above comment ([1]),
 we'll make sure to fold in the use cases suggested here into the general jinja templater for cloud-config modules in the branch at [2]. I'll mark as disapprove/rejected as the general templater should provide the template variable you requested, and much more. Thanks again for getting the ball rolling here.

This means you can provide your cloud-config with the following:

## template: jinja
#cloud-config
chef:
   node_name: {{ v1.instance_id|replace('i-', '') }}

references:
 [1]: https://code.launchpad.net/~johnguthrie/cloud-init/+git/cloud-init/+merge/331905/comments/875174
 [2] https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/334030

review: Disapprove

Unmerged commits

e9c9955... by John Guthrie <email address hidden> on 2017-10-06

The datasource could be None as well

38cafcc... by John Guthrie <email address hidden> on 2017-10-06

Need to address the case where we aren't necessarily in a cloud with an instance ID

faba968... by John Guthrie <email address hidden> on 2017-10-06

Adding spaces after commas to make linter happy

0075d7f... by John Guthrie <email address hidden> on 2017-10-05

Adding support for %I and %i substitions in chef

diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py
index 46abedd..bf9e9c8 100644
--- a/cloudinit/config/cc_chef.py
+++ b/cloudinit/config/cc_chef.py
@@ -23,6 +23,11 @@ chef will have forked into its own process) then a post run function can
 run that can do finishing activities (such as removing the validation pem
 file).
 
+The fqdn and node_name keys will support substitutions for ``%I`` and ``$i``.
+``%I`` is for the instance ID, and ``%i`` is for the stripped instance ID, that
+is the instance ID without the initial ``i-``.  (This is AWS specific for
+now.  Hopefully, this will improve over time.)
+
 **Internal name:** ``cc_chef``
 
 **Module frequency:** per always
@@ -229,6 +234,11 @@ def handle(name, cfg, cloud, log, _args):
             if k in CHEF_RB_TPL_PATH_KEYS and v:
                 param_paths.add(os.path.dirname(v))
         util.ensure_dirs(param_paths)
+        stripped_iid = iid.replace('i-', '')
+        # expand %i
+        params['node_name'] = params['node_name'].replace("%i", stripped_iid)
+        # expand %I
+        params['node_name'] = params['node_name'].replace("%I", iid)
         templater.render_to_file(template_fn, CHEF_RB_PATH, params)
     else:
         log.warn("No template found, not rendering to %s",
diff --git a/cloudinit/util.py b/cloudinit/util.py
index e1290aa..f601391 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -1030,17 +1030,28 @@ def dos2unix(contents):
 
 
 def get_hostname_fqdn(cfg, cloud):
+    if cloud is None or cloud.datasource is None:
+        iid = ''
+    else:
+        iid = cloud.get_instance_id()
+    stripped_iid = iid.replace('i-', '')
+
     # return the hostname and fqdn from 'cfg'.  If not found in cfg,
     # then fall back to data from cloud
     if "fqdn" in cfg:
         # user specified a fqdn.  Default hostname then is based off that
         fqdn = cfg['fqdn']
+        # expand %I and %i
+        fqdn = fqdn.replace('%I', iid)
+        fqdn = fqdn.replace('%i', stripped_iid)
         hostname = get_cfg_option_str(cfg, "hostname", fqdn.split('.')[0])
     else:
         if "hostname" in cfg and cfg['hostname'].find('.') > 0:
             # user specified hostname, and it had '.' in it
             # be nice to them.  set fqdn and hostname from that
             fqdn = cfg['hostname']
+            fqdn = fqdn.replace('%I', iid)
+            fqdn = fqdn.replace('%i', stripped_iid)
             hostname = cfg['hostname'][:fqdn.find('.')]
         else:
             # no fqdn set, get fqdn from cloud.
@@ -1048,6 +1059,8 @@ def get_hostname_fqdn(cfg, cloud):
             fqdn = cloud.get_hostname(fqdn=True)
             if "hostname" in cfg:
                 hostname = cfg['hostname']
+                hostname = hostname.replace('%I', iid)
+                hostname = hostname.replace('%i', stripped_iid)
             else:
                 hostname = cloud.get_hostname()
     return (hostname, fqdn)