Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Romanos, Sorry to keep bothering you :) Thanks again for your help. I looked to pull this in, and found that it has merge conflicts with master. I've rebased onto master and would just like you to review my changes. See https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/339373 With your approval there, I'll merge that. -- https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 Your team cloud-init commiters is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
[Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Romanos Skiadas has proposed merging ~rski/cloud-init:puppet_4 into
cloud-init:master.
Commit message:
Implement puppet 4 support.
Make puppet installation more configurable by:
- Adding a package_name parameter
- Exposing the puppet configuration and puppet ssl directories as
parameters These default to the previous values if unset, but can be set
to the new values puppetlabs requires for its puppet 4.x packages. This
way puppet 4 configuration is now possible.
LP: #1446804
Requested reviews:
Server Team CI bot (server-team-bot): continuous-integration
cloud-init commiters (cloud-init-dev)
Related bugs:
Bug #1446804 in cloud-init: "Puppet fails with puppet 4"
https://bugs.launchpad.net/cloud-init/+bug/1446804
For more details, see:
https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284
--
Your team cloud-init commiters is requested to review the proposed merge of
~rski/cloud-init:puppet_4 into cloud-init:master.
diff --git a/cloudinit/config/cc_puppet.py b/cloudinit/config/cc_puppet.py
index bfd630d..d5351f6 100644
--- a/cloudinit/config/cc_puppet.py
+++ b/cloudinit/config/cc_puppet.py
@@ -33,6 +33,13 @@ under ``version``, and defaults to ``none``, which selects the latest version
in the repos. If the ``puppet`` config key exists in the config archive, this
module will attempt to start puppet even if no installation was performed.
+The module also provides keys for configuring the new puppet 4 paths and
+installing the puppet package from the puppetlabs repositories:
+https://docs.puppet.com/puppet/4.2/reference/whered_it_go.html
+The keys are ``package_name``, ``conf_dir`` and ``ssl_dir``. If unset, their
+values will default to ones that work with puppet 3.x and with distributions
+that ship modified puppet 4.x that uses the old paths.
+
Puppet configuration can be specified under the ``conf`` key. The configuration
is specified as a dictionary which is converted into ``=`` format
and appended to ``puppet.conf`` under the ``[puppetd]`` section. The
@@ -54,6 +61,9 @@ yaml notation).
puppet:
install:
version:
+conf_dir: '/etc/puppet/'
+ssl_dir: '/var/lib/puppet/ssl'
+package_name: 'puppet'
conf:
server: "puppetmaster.example.org"
certname: "%i.%f"
@@ -71,10 +81,23 @@ import socket
from cloudinit import helpers
from cloudinit import util
-PUPPET_CONF_PATH = '/etc/puppet/puppet.conf'
-PUPPET_SSL_CERT_DIR = '/var/lib/puppet/ssl/certs/'
-PUPPET_SSL_DIR = '/var/lib/puppet/ssl'
-PUPPET_SSL_CERT_PATH = '/var/lib/puppet/ssl/certs/ca.pem'
+DEFAULT_PACKAGE_NAME = 'puppet'
+DEFAULT_SSL_DIR = '/var/lib/puppet/ssl'
+DEFAULT_CONF_DIR = '/etc/puppet'
+
+
+class PuppetConstants(object):
+
+def __init__(self,
+ puppet_conf_dir,
+ puppet_ssl_dir,
+ log):
+self.conf_dir = puppet_conf_dir
+self.conf_path = os.path.join(puppet_conf_dir, "puppet.conf")
+self.ssl_dir = puppet_ssl_dir
+self.ssl_cert_dir = os.path.join(puppet_ssl_dir, "certs")
+self.ssl_cert_path = os.path.join(self.ssl_cert_dir,
+ "ca.pem")
def _autostart_puppet(log):
@@ -101,22 +124,35 @@ def handle(name, cfg, cloud, log, _args):
return
puppet_cfg = cfg['puppet']
-
# Start by installing the puppet package if necessary...
install = util.get_cfg_option_bool(puppet_cfg, 'install', True)
version = util.get_cfg_option_str(puppet_cfg, 'version', None)
+package_name = util.get_cfg_option_str(puppet_cfg,
+ 'package_name',
+ DEFAULT_PACKAGE_NAME)
+conf_dir = util.get_cfg_option_str(puppet_cfg,
+ 'conf_dir',
+ DEFAULT_CONF_DIR)
+ssl_dir = util.get_cfg_option_str(puppet_cfg,
+ 'ssl_dir',
+ DEFAULT_SSL_DIR)
+
+p_constants = PuppetConstants(conf_dir,
+ ssl_dir,
+ log)
if not install and version:
log.warn(("Puppet install set false but version supplied,"
" doing nothing."))
elif install:
log.debug(("Attempting to install puppet %s,"),
version if version else 'latest')
-cloud.distro.install_packages(('puppet', version))
+
+cloud.distro.install_packages((package_name, version))
# ... and then update the puppet configuration
if 'conf' in puppet_cfg:
# Add all sections from the conf object to puppet.conf
-contents = util.load_file(PUPPET_CONF_PATH)
+contents = util.load_file(p_constants.conf_path)
# Create object for reading puppet.conf values
puppet_config = helpers.DefaultingConfigParser()
# Read puppet.conf values from
Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Verified that intracom telecom signed the cla. Thank you. -- https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 Your team cloud-init commiters is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
So, intracom telecom seems to have signed the CLA now. Can you verify that? In any case, you should be able to go ahead and use this patch. I won't be working on it, since I'd have to get permissions plus a signature from my current company which would at best delay things further. -- https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 Your team cloud-init commiters is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Bump? -- https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 Your team cloud-init commiters is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Romanos, Any progress ? I realize that often times things fall off the immediate radar. Feel free to ping me if you have any questions. Thanks, Scott -- https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 Your team cloud-init commiters is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
[Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
The proposal to merge ~rski/cloud-init:puppet_4 into cloud-init:master has been updated. Commit Message changed to: Implement puppet 4 support. Make puppet installation more configurable by: - Adding a package_name parameter - Exposing the puppet configuration and puppet ssl directories as parameters These default to the previous values if unset, but can be set to the new values puppetlabs requires for its puppet 4.x packages. This way puppet 4 configuration is now possible. LP: #1446804 For more details, see: https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 -- Your team cloud init development team is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Hi, I'm setting this to 'work in progress'. Please feel free to set it back to 'Needs Review' when you're ready. -- https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 Your team cloud init development team is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
[Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
The proposal to merge ~rski/cloud-init:puppet_4 into cloud-init:master has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 -- Your team cloud init development team is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
[Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
The proposal to merge ~rski/cloud-init:puppet_4 into cloud-init:master has been updated. Commit Message changed to: Implement puppet 4 support Make puppet installation more configurable by: - Adding a package_name parameter - Exposing the puppet configuration and puppet ssl directories as parameters These default to the previous values if unset, but can be set to the new values puppetlabs requires for its puppet 4.x packages. This way puppet 4 configuration is now possible. LP: #1446804 For more details, see: https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 -- Your team cloud init development team is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Romanos, Any progress? Scott -- https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 Your team cloud init development team is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Hi Scott, Thanks for the feedback. We're in the process of getting the CLA signed. In the meantime, I'll fix the things you pointed out in the code. -- https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284 Your team cloud init development team is requested to review the proposed merge of ~rski/cloud-init:puppet_4 into cloud-init:master. ___ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp
Re: [Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Romanos,
Over all it looks really good.
In order to take this, you'll need to sign the contributors agreement
(https://www.ubuntu.com/legal/contributors). See the hacking section on
readthedocs for more info
http://cloudinit.readthedocs.io/en/latest/topics/hacking.html .
Also, please feel free to ping me in Freenode #cloud-init or /query me, or via
email.
I've left a few minor questions inline also, but the 'needs fixing' is really
only wrt the contributors agreement.
Diff comments:
> diff --git a/cloudinit/config/cc_puppet.py b/cloudinit/config/cc_puppet.py
> index bfd630d..d5351f6 100644
> --- a/cloudinit/config/cc_puppet.py
> +++ b/cloudinit/config/cc_puppet.py
> @@ -71,10 +81,23 @@ import socket
> from cloudinit import helpers
> from cloudinit import util
>
> -PUPPET_CONF_PATH = '/etc/puppet/puppet.conf'
> -PUPPET_SSL_CERT_DIR = '/var/lib/puppet/ssl/certs/'
> -PUPPET_SSL_DIR = '/var/lib/puppet/ssl'
> -PUPPET_SSL_CERT_PATH = '/var/lib/puppet/ssl/certs/ca.pem'
> +DEFAULT_PACKAGE_NAME = 'puppet'
> +DEFAULT_SSL_DIR = '/var/lib/puppet/ssl'
> +DEFAULT_CONF_DIR = '/etc/puppet'
any reason why you've chosen to allow the directory, and assume/hard code the
conf to be in that directory at 'puppet.conf' rather than allowing the
configuration of the puppet config file path directly ?
> +
> +
> +class PuppetConstants(object):
> +
> +def __init__(self,
> + puppet_conf_dir,
> + puppet_ssl_dir,
> + log):
> +self.conf_dir = puppet_conf_dir
> +self.conf_path = os.path.join(puppet_conf_dir, "puppet.conf")
> +self.ssl_dir = puppet_ssl_dir
> +self.ssl_cert_dir = os.path.join(puppet_ssl_dir, "certs")
> +self.ssl_cert_path = os.path.join(self.ssl_cert_dir,
> + "ca.pem")
>
>
> def _autostart_puppet(log):
> @@ -101,22 +124,35 @@ def handle(name, cfg, cloud, log, _args):
> return
>
> puppet_cfg = cfg['puppet']
> -
> # Start by installing the puppet package if necessary...
> install = util.get_cfg_option_bool(puppet_cfg, 'install', True)
> version = util.get_cfg_option_str(puppet_cfg, 'version', None)
> +package_name = util.get_cfg_option_str(puppet_cfg,
> + 'package_name',
> + DEFAULT_PACKAGE_NAME)
> +conf_dir = util.get_cfg_option_str(puppet_cfg,
> + 'conf_dir',
> + DEFAULT_CONF_DIR)
> +ssl_dir = util.get_cfg_option_str(puppet_cfg,
> + 'ssl_dir',
> + DEFAULT_SSL_DIR)
> +
> +p_constants = PuppetConstants(conf_dir,
i'd just join these three to save vertical space:
p_constants = PuppetConstants(conf_dir, ssl_dir, log)
> + ssl_dir,
> + log)
> if not install and version:
> log.warn(("Puppet install set false but version supplied,"
>" doing nothing."))
> elif install:
> log.debug(("Attempting to install puppet %s,"),
>version if version else 'latest')
> -cloud.distro.install_packages(('puppet', version))
> +
> +cloud.distro.install_packages((package_name, version))
>
> # ... and then update the puppet configuration
> if 'conf' in puppet_cfg:
> # Add all sections from the conf object to puppet.conf
> -contents = util.load_file(PUPPET_CONF_PATH)
> +contents = util.load_file(p_constants.conf_path)
> # Create object for reading puppet.conf values
> puppet_config = helpers.DefaultingConfigParser()
> # Read puppet.conf values from original file in order to be able to
--
https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284
Your team cloud init development team is requested to review the proposed merge
of ~rski/cloud-init:puppet_4 into cloud-init:master.
___
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
[Cloud-init-dev] [Merge] ~rski/cloud-init:puppet_4 into cloud-init:master
Romanos Skiadas has proposed merging ~rski/cloud-init:puppet_4 into
cloud-init:master.
Requested reviews:
cloud init development team (cloud-init-dev)
Related bugs:
Bug #1446804 in cloud-init: "Puppet fails with puppet 4"
https://bugs.launchpad.net/cloud-init/+bug/1446804
For more details, see:
https://code.launchpad.net/~rski/cloud-init/+git/cloud-init/+merge/312284
--
Your team cloud init development team is requested to review the proposed merge
of ~rski/cloud-init:puppet_4 into cloud-init:master.
diff --git a/cloudinit/config/cc_puppet.py b/cloudinit/config/cc_puppet.py
index bfd630d..d5351f6 100644
--- a/cloudinit/config/cc_puppet.py
+++ b/cloudinit/config/cc_puppet.py
@@ -33,6 +33,13 @@ under ``version``, and defaults to ``none``, which selects the latest version
in the repos. If the ``puppet`` config key exists in the config archive, this
module will attempt to start puppet even if no installation was performed.
+The module also provides keys for configuring the new puppet 4 paths and
+installing the puppet package from the puppetlabs repositories:
+https://docs.puppet.com/puppet/4.2/reference/whered_it_go.html
+The keys are ``package_name``, ``conf_dir`` and ``ssl_dir``. If unset, their
+values will default to ones that work with puppet 3.x and with distributions
+that ship modified puppet 4.x that uses the old paths.
+
Puppet configuration can be specified under the ``conf`` key. The configuration
is specified as a dictionary which is converted into ``=`` format
and appended to ``puppet.conf`` under the ``[puppetd]`` section. The
@@ -54,6 +61,9 @@ yaml notation).
puppet:
install:
version:
+conf_dir: '/etc/puppet/'
+ssl_dir: '/var/lib/puppet/ssl'
+package_name: 'puppet'
conf:
server: "puppetmaster.example.org"
certname: "%i.%f"
@@ -71,10 +81,23 @@ import socket
from cloudinit import helpers
from cloudinit import util
-PUPPET_CONF_PATH = '/etc/puppet/puppet.conf'
-PUPPET_SSL_CERT_DIR = '/var/lib/puppet/ssl/certs/'
-PUPPET_SSL_DIR = '/var/lib/puppet/ssl'
-PUPPET_SSL_CERT_PATH = '/var/lib/puppet/ssl/certs/ca.pem'
+DEFAULT_PACKAGE_NAME = 'puppet'
+DEFAULT_SSL_DIR = '/var/lib/puppet/ssl'
+DEFAULT_CONF_DIR = '/etc/puppet'
+
+
+class PuppetConstants(object):
+
+def __init__(self,
+ puppet_conf_dir,
+ puppet_ssl_dir,
+ log):
+self.conf_dir = puppet_conf_dir
+self.conf_path = os.path.join(puppet_conf_dir, "puppet.conf")
+self.ssl_dir = puppet_ssl_dir
+self.ssl_cert_dir = os.path.join(puppet_ssl_dir, "certs")
+self.ssl_cert_path = os.path.join(self.ssl_cert_dir,
+ "ca.pem")
def _autostart_puppet(log):
@@ -101,22 +124,35 @@ def handle(name, cfg, cloud, log, _args):
return
puppet_cfg = cfg['puppet']
-
# Start by installing the puppet package if necessary...
install = util.get_cfg_option_bool(puppet_cfg, 'install', True)
version = util.get_cfg_option_str(puppet_cfg, 'version', None)
+package_name = util.get_cfg_option_str(puppet_cfg,
+ 'package_name',
+ DEFAULT_PACKAGE_NAME)
+conf_dir = util.get_cfg_option_str(puppet_cfg,
+ 'conf_dir',
+ DEFAULT_CONF_DIR)
+ssl_dir = util.get_cfg_option_str(puppet_cfg,
+ 'ssl_dir',
+ DEFAULT_SSL_DIR)
+
+p_constants = PuppetConstants(conf_dir,
+ ssl_dir,
+ log)
if not install and version:
log.warn(("Puppet install set false but version supplied,"
" doing nothing."))
elif install:
log.debug(("Attempting to install puppet %s,"),
version if version else 'latest')
-cloud.distro.install_packages(('puppet', version))
+
+cloud.distro.install_packages((package_name, version))
# ... and then update the puppet configuration
if 'conf' in puppet_cfg:
# Add all sections from the conf object to puppet.conf
-contents = util.load_file(PUPPET_CONF_PATH)
+contents = util.load_file(p_constants.conf_path)
# Create object for reading puppet.conf values
puppet_config = helpers.DefaultingConfigParser()
# Read puppet.conf values from original file in order to be able to
@@ -125,21 +161,21 @@ def handle(name, cfg, cloud, log, _args):
cleaned_lines = [i.lstrip() for i in contents.splitlines()]
cleaned_contents = '\n'.join(cleaned_lines)
puppet_config.readfp(StringIO(cleaned_contents),
- filename=PUPPET_CONF_PATH)
+ filename=p_constants.conf_path)
for (cfg_name, cfg) in
