From 8246642a45563772a7142188d5bca564982d4aaf Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 3 Jun 2016 12:11:31 +0200 Subject: [PATCH 1/7] Bugfix: ssh mux controlpath too long on some envs. --- scripts/cdist | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/cdist b/scripts/cdist index 6baa28f3..55113be0 100755 --- a/scripts/cdist +++ b/scripts/cdist @@ -26,8 +26,11 @@ def inspect_ssh_mux_opts(control_path_dir="~/.ssh/"): import subprocess import os - control_path = os.path.join(control_path_dir, - "cdist.socket.master-%l-%r@%h:%p") + # socket is always local to each cdist run, it is created in + # temp directory that is created at starting cdist, so + # control_path file name can be static, it will always be + # unique due to unique temp directory name + control_path = os.path.join(control_path_dir, "ssh-control-path") wanted_mux_opts = { "ControlPath": control_path, "ControlMaster": "auto", @@ -147,7 +150,7 @@ def commandline(): # didn't specify command line options nor env vars: # inspect multiplexing options for default cdist.REMOTE_COPY/EXEC if args_dict['remote_copy'] is None or args_dict['remote_exec'] is None: - control_path_dir = tempfile.mkdtemp() + control_path_dir = tempfile.mkdtemp(prefix="cdist") import atexit atexit.register(lambda: shutil.rmtree(control_path_dir)) mux_opts = inspect_ssh_mux_opts(control_path_dir) From fd8e10e12adaf9c4dce88868ea26bb5ee859db8e Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 3 Jun 2016 12:20:28 +0200 Subject: [PATCH 2/7] Improve error reporting for local and remote run. --- cdist/__init__.py | 4 ++-- cdist/exec/local.py | 13 ++++++++----- cdist/exec/remote.py | 13 ++++++++----- cdist/exec/util.py | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 12 deletions(-) create mode 100644 cdist/exec/util.py diff --git a/cdist/__init__.py b/cdist/__init__.py index 4454a3ac..e789499c 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -41,8 +41,8 @@ BANNER = """ "P' "" "" """ -REMOTE_COPY = "scp -o User=root -q" -REMOTE_EXEC = "ssh -o User=root -q" +REMOTE_COPY = "scp -o User=root" +REMOTE_EXEC = "ssh -o User=root" class Error(Exception): """Base exception class for this project""" diff --git a/cdist/exec/local.py b/cdist/exec/local.py index c0554831..15f49a89 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -33,6 +33,7 @@ import tempfile import cdist import cdist.message from cdist import core +import cdist.exec.util as exec_util class Local(object): """Execute commands locally. @@ -203,12 +204,14 @@ class Local(object): env.update(message.env) try: + output = exec_util.call_get_output(command, env=env) + self.log.debug("Local output: {}".format(output)) if return_output: - return subprocess.check_output(command, env=env).decode() - else: - subprocess.check_call(command, env=env) - except subprocess.CalledProcessError: - raise cdist.Error("Command failed: " + " ".join(command)) + return output.decode() + except subprocess.CalledProcessError as e: + raise cdist.Error("Command failed: " + " ".join(command) + + " with returncode: {} and output: {}".format( + e.returncode, e.output)) except OSError as error: raise cdist.Error(" ".join(command) + ": " + error.args[1]) finally: diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index 77e2c8be..c78f02cb 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -26,6 +26,7 @@ import sys import glob import subprocess import logging +import cdist.exec.util as exec_util import cdist @@ -167,12 +168,14 @@ class Remote(object): self.log.debug("Remote run: %s", command) try: + output = exec_util.call_get_output(command, env=os_environ) + self.log.debug("Remote output: {}".format(output)) if return_output: - return subprocess.check_output(command, env=os_environ).decode() - else: - subprocess.check_call(command, env=os_environ) - except subprocess.CalledProcessError: - raise cdist.Error("Command failed: " + " ".join(command)) + return output.decode() + except subprocess.CalledProcessError as e: + raise cdist.Error("Command failed: " + " ".join(command) + + " with returncode: {} and output: {}".format( + e.returncode, e.output)) except OSError as error: raise cdist.Error(" ".join(command) + ": " + error.args[1]) except UnicodeDecodeError: diff --git a/cdist/exec/util.py b/cdist/exec/util.py new file mode 100644 index 00000000..983f455c --- /dev/null +++ b/cdist/exec/util.py @@ -0,0 +1,39 @@ +# -*- coding: utf-8 -*- +# +# 2016 Darko Poljak (darko.poljak at gmail.com) +# +# This file is part of cdist. +# +# cdist is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# cdist is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with cdist. If not, see . +# +# + +import subprocess +from tempfile import TemporaryFile + +import cdist + +def call_get_output(command, env=None): + """Run the given command with the given environment. + Return the stdout and stderr output as a byte string. + """ + assert isinstance(command, (list, tuple)), "list or tuple argument expected, got: {}".format(command) + + with TemporaryFile() as fout: + subprocess.check_call(command, env=env, + stdout=fout, stderr=subprocess.STDOUT) + fout.seek(0) + output = fout.read() + + return output From 88b20610cbed739a8d6b7e6edda05457d7ffdf6e Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Mon, 6 Jun 2016 22:21:17 +0200 Subject: [PATCH 3/7] Update changelog. --- docs/changelog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/changelog b/docs/changelog index dada1d90..b0a8be4c 100644 --- a/docs/changelog +++ b/docs/changelog @@ -1,6 +1,9 @@ Changelog --------- +next: + * Core: Improve error reporting for local and remote run command (Darko Poljak) + 4.1.0: 2016-05-27 * Documentation: Migrate to reStructuredText format and sphinx (Darko Poljak) * Core: Add -f option to read additional hosts from file/stdin (Darko Poljak) From e4fe8e8f37e9c9a3da90094cdd49fdd160862987 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Tue, 7 Jun 2016 13:44:35 +0200 Subject: [PATCH 4/7] Implement bash and zsh completions. --- completions/bash/cdist-completion.bash | 59 ++++++++++++++++++++++++++ completions/zsh/_cdist | 48 +++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 completions/bash/cdist-completion.bash create mode 100644 completions/zsh/_cdist diff --git a/completions/bash/cdist-completion.bash b/completions/bash/cdist-completion.bash new file mode 100644 index 00000000..3d396bb4 --- /dev/null +++ b/completions/bash/cdist-completion.bash @@ -0,0 +1,59 @@ +_cdist() +{ + local cur prev prevprev opts cmds projects + COMPREPLY=() + cur="${COMP_WORDS[COMP_CWORD]}" + prev="${COMP_WORDS[COMP_CWORD-1]}" + prevprev="${COMP_WORDS[COMP_CWORD-2]}" + opts="-h --help -d --debug -v --verbose -V --version" + cmds="banner shell config" + + case "${prevprev}" in + shell) + case "${prev}" in + -s|--shell) + shells=$(grep -v '^#' /etc/shells) + COMPREPLY=( $(compgen -W "${shells}" -- ${cur}) ) + return 0 + ;; + esac + ;; + esac + + case "${prev}" in + -*) + COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) + return 0 + ;; + banner) + opts="-h --help -d --debug -v --verbose" + COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) + return 0 + ;; + shell) + opts="-h --help -d --debug -v --verbose -s --shell" + COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) + return 0 + ;; + config) + opts="-h --help -d --debug -v --verbose \ + -c --conf-dir -f --file -i --initial-manifest -n --dry-run \ + -o --out-dir -p --parallel -s --sequential --remote-copy \ + --remote-exec" + COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) + return 0 + ;; + *) + ;; + esac + + if [[ ${cur} == -* ]]; then + COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) + return 0 + fi + + COMPREPLY=( $(compgen -W "${cmds}" -- ${cur}) ) + return 0 +} + +complete -F _cdist cdist diff --git a/completions/zsh/_cdist b/completions/zsh/_cdist new file mode 100644 index 00000000..2bf324a6 --- /dev/null +++ b/completions/zsh/_cdist @@ -0,0 +1,48 @@ +#compdef cdist + +_cdist() +{ + local curcontext="$curcontext" state line + typeset -A opt_args + + _arguments \ + '1: :->opts_cmds'\ + '*: :->opts' + + case $state in + opts_cmds) + _arguments '1:Options and commands:(banner config shell -h --help -d --debug -v --verbose -V --version)' + ;; + *) + case $words[2] in + -*) + opts=(-h --help -d --debug -v --verbose -V --version) + compadd "$@" -- $opts + ;; + banner) + opts=(-h --help -d --debug -v --verbose) + compadd "$@" -- $opts + ;; + shell) + case $words[3] in + -s|--shell) + shells=($(grep -v '^#' /etc/shells)) + compadd "$@" -- $shells + ;; + *) + opts=(-h --help -d --debug -v --verbose -s --shell) + compadd "$@" -- $opts + ;; + esac + ;; + config) + opts=(-h --help -d --debug -v --verbose -c --conf-dir -f --file -i --initial-manifest -n --dry-run -o --out-dir -p --parallel -s --sequential --remote-copy --remote-exec) + compadd "$@" -- $opts + ;; + *) + ;; + esac + esac +} + +_cdist "$@" From bd9008794c5f620ae3957af78881887c9e85d3cc Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Fri, 10 Jun 2016 07:50:07 +0200 Subject: [PATCH 5/7] Conflicting requirements bugfix. --- cdist/emulator.py | 37 +++++++++++++++++++--- cdist/test/emulator/__init__.py | 55 +++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/cdist/emulator.py b/cdist/emulator.py index 3f553412..f5a9f645 100644 --- a/cdist/emulator.py +++ b/cdist/emulator.py @@ -77,6 +77,9 @@ class Emulator(object): self.type_name = os.path.basename(argv[0]) self.cdist_type = core.CdistType(self.type_base_path, self.type_name) + # if set then object already exists and this var holds existing + # requirements + self._existing_reqs = None self.__init_log() @@ -150,10 +153,18 @@ class Emulator(object): self.parameters[key] = value if self.cdist_object.exists and not 'CDIST_OVERRIDE' in self.env: + # make existing requirements a set, so we can compare it + # later with new requirements + self._existing_reqs = set(self.cdist_object.requirements) if self.cdist_object.parameters != self.parameters: - raise cdist.Error("Object %s already exists with conflicting parameters:\n%s: %s\n%s: %s" - % (self.cdist_object.name, " ".join(self.cdist_object.source), self.cdist_object.parameters, self.object_source, self.parameters) - ) + errmsg = ("Object %s already exists with conflicting " + "parameters:\n%s: %s\n%s: %s" % (self.cdist_object.name, + " ".join(self.cdist_object.source), + self.cdist_object.parameters, + self.object_source, + self.parameters)) + self.log.error(errmsg) + raise cdist.Error(errmsg) else: if self.cdist_object.exists: self.log.debug('Object %s override forced with CDIST_OVERRIDE',self.cdist_object.name) @@ -210,7 +221,7 @@ class Emulator(object): # if no second last line, we are on the first type, so do not set a requirement pass - + reqs = set() if "require" in self.env: requirements = self.env['require'] self.log.debug("reqs = " + requirements) @@ -234,6 +245,24 @@ class Emulator(object): # (__file//bar => __file/bar) # This ensures pattern matching is done against sanitised list self.cdist_object.requirements.append(cdist_object.name) + reqs.add(cdist_object.name) + if self._existing_reqs is not None: + # if object exists then compare existing and new requirements + self.log.debug("OBJ: {} {}".format(self.cdist_type, self.object_id)) + self.log.debug("EXISTING REQS: {}".format(self._existing_reqs)) + self.log.debug("REQS: {}".format(reqs)) + + if self._existing_reqs != reqs: + errmsg = ("Object {} already exists with conflicting " + "requirements:\n{}: {}\n{}: {}".format( + self.cdist_object.name, + " ".join(self.cdist_object.source), + self._existing_reqs, + self.object_source, + reqs)) + self.log.error(errmsg) + raise cdist.Error(errmsg) + def record_auto_requirements(self): """An object shall automatically depend on all objects that it defined in it's type manifest. diff --git a/cdist/test/emulator/__init__.py b/cdist/test/emulator/__init__.py index f90e5320..b91c1e8f 100644 --- a/cdist/test/emulator/__init__.py +++ b/cdist/test/emulator/__init__.py @@ -133,6 +133,56 @@ class EmulatorTestCase(test.CdistTestCase): self.assertEqual(list(file_object.requirements), ['__planet/mars']) # if we get here all is fine +class EmulatorConflictingRequirementsTestCase(test.CdistTestCase): + + def setUp(self): + self.temp_dir = self.mkdtemp() + handle, self.script = self.mkstemp(dir=self.temp_dir) + os.close(handle) + base_path = self.temp_dir + + self.local = local.Local( + target_host=self.target_host, + base_path=base_path, + exec_path=test.cdist_exec_path, + add_conf_dirs=[conf_dir]) + self.local.create_files_dirs() + + self.manifest = core.Manifest(self.target_host, self.local) + self.env = self.manifest.env_initial_manifest(self.script) + self.env['__cdist_object_marker'] = self.local.object_marker_name + + def tearDown(self): + shutil.rmtree(self.temp_dir) + + def test_object_conflicting_requirements_req_none(self): + argv = ['__directory', 'spam'] + emu = emulator.Emulator(argv, env=self.env) + emu.run() + argv = ['__file', 'eggs'] + self.env['require'] = '__directory/spam' + emu = emulator.Emulator(argv, env=self.env) + emu.run() + argv = ['__file', 'eggs'] + if 'require' in self.env: + del self.env['require'] + emu = emulator.Emulator(argv, env=self.env) + self.assertRaises(cdist.Error, emu.run) + + def test_object_conflicting_requirements_none_req(self): + argv = ['__directory', 'spam'] + emu = emulator.Emulator(argv, env=self.env) + emu.run() + argv = ['__file', 'eggs'] + if 'require' in self.env: + del self.env['require'] + emu = emulator.Emulator(argv, env=self.env) + emu.run() + argv = ['__file', 'eggs'] + self.env['require'] = '__directory/spam' + emu = emulator.Emulator(argv, env=self.env) + self.assertRaises(cdist.Error, emu.run) + class AutoRequireEmulatorTestCase(test.CdistTestCase): @@ -366,3 +416,8 @@ class StdinTestCase(test.CdistTestCase): stdin_saved_by_emulator = fd.read() self.assertEqual(random_string, stdin_saved_by_emulator) + + +if __name__ == '__main__': + import unittest + unittest.main() From 2e1cc0a89b489abfa07d72a584eb45aafb19f2e9 Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Wed, 22 Jun 2016 12:20:34 +0200 Subject: [PATCH 6/7] Update changelog. --- docs/changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog b/docs/changelog index 8ad8e953..207125db 100644 --- a/docs/changelog +++ b/docs/changelog @@ -2,6 +2,7 @@ Changelog --------- next: + * Custom: Add bash and zsh completions (Darko Poljak) * New type: __jail_freebsd9: Handle jail management on FreeBSD <= 9.X (Jake Guffey) * New type: __jail_freebsd10: Handle jail management on FreeBSD >= 10.0 (Jake Guffey) * Type __jail: Dynamically select the correct jail subtype based on target host OS (Jake Guffey) From 5ee20f7886d81be81407c33cc205d82d1dc51c2e Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Wed, 22 Jun 2016 12:23:31 +0200 Subject: [PATCH 7/7] Update changelog. --- docs/changelog | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog b/docs/changelog index 8ad8e953..31f2bb35 100644 --- a/docs/changelog +++ b/docs/changelog @@ -2,6 +2,7 @@ Changelog --------- next: + * Core: Fix conflicting requirements (Darko Poljak) * New type: __jail_freebsd9: Handle jail management on FreeBSD <= 9.X (Jake Guffey) * New type: __jail_freebsd10: Handle jail management on FreeBSD >= 10.0 (Jake Guffey) * Type __jail: Dynamically select the correct jail subtype based on target host OS (Jake Guffey)