From 2d219af67ae6539197cd3c436f44618dfd6d28e6 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Fri, 24 Mar 2017 11:28:20 -0500 Subject: [PATCH 01/10] Don't use context caching for gathering systemd services In a single Salt run, when multiple pkg states (with accompanying service states) are run, the first service state establishes __context__['systemd.systemd_services']. Then, subsequent pkg states which add new packages will in effect add new systemd services, but systemd.py is oblivious to this change and further attempts to get the list of systemd services are returned from the (now outdated) cache. This commit resolves this by not using context caching for the list of systemd services. --- salt/modules/systemd.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/salt/modules/systemd.py b/salt/modules/systemd.py index 9916bf2fee..ccf3388b6f 100644 --- a/salt/modules/systemd.py +++ b/salt/modules/systemd.py @@ -125,8 +125,7 @@ def _clear_context(): # raise a RuntimeError. for key in list(__context__): try: - if key.startswith('systemd._systemctl_status.') \ - or key in ('systemd.systemd_services',): + if key.startswith('systemd._systemctl_status.'): __context__.pop(key) except AttributeError: continue @@ -178,9 +177,6 @@ def _get_systemd_services(): ''' Use os.listdir() to get all the unit files ''' - contextkey = 'systemd.systemd_services' - if contextkey in __context__: - return __context__[contextkey] ret = set() for path in SYSTEM_CONFIG_PATHS + (LOCAL_CONFIG_PATH,): # Make sure user has access to the path, and if the path is a link @@ -194,7 +190,6 @@ def _get_systemd_services(): continue if unit_type in VALID_UNIT_TYPES: ret.add(unit_name if unit_type == 'service' else fullname) - __context__[contextkey] = copy.deepcopy(ret) return ret From 8bcdf1a7611c9ac470b95757ae1f5ebe4d8b2344 Mon Sep 17 00:00:00 2001 From: Mike Place Date: Fri, 24 Mar 2017 13:05:21 -0600 Subject: [PATCH 02/10] Remove unused import for lint --- salt/modules/systemd.py | 1 - 1 file changed, 1 deletion(-) diff --git a/salt/modules/systemd.py b/salt/modules/systemd.py index ccf3388b6f..f63f82539c 100644 --- a/salt/modules/systemd.py +++ b/salt/modules/systemd.py @@ -12,7 +12,6 @@ Provides the service module for systemd ''' # Import python libs from __future__ import absolute_import -import copy import errno import glob import logging From 808ad764197e24789b7e80410addff194f1fd6f3 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Mon, 27 Mar 2017 11:23:12 -0500 Subject: [PATCH 03/10] systemd.py: when getting all services, don't repeat gathering of systemd services Now that we are not caching these in the context dunder, we should pass them into this function so that we don't repeat gathering the systemd services inside _get_sysv_services(). Also, this fixes an incorrect change to the mocking that was apparently made at some point to the get_all unit test. --- salt/modules/systemd.py | 9 +++++---- tests/unit/modules/systemd_test.py | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/salt/modules/systemd.py b/salt/modules/systemd.py index ccf3388b6f..67dc758669 100644 --- a/salt/modules/systemd.py +++ b/salt/modules/systemd.py @@ -12,7 +12,6 @@ Provides the service module for systemd ''' # Import python libs from __future__ import absolute_import -import copy import errno import glob import logging @@ -193,7 +192,7 @@ def _get_systemd_services(): return ret -def _get_sysv_services(): +def _get_sysv_services(systemd_services=None): ''' Use os.listdir() and os.access() to get all the initscripts ''' @@ -215,7 +214,9 @@ def _get_sysv_services(): ) return [] - systemd_services = _get_systemd_services() + if systemd_services is None: + systemd_services = _get_systemd_services() + ret = [] for sysv_service in sysv_services: if os.access(os.path.join(INITSCRIPT_PATH, sysv_service), os.X_OK): @@ -493,7 +494,7 @@ def get_all(): salt '*' service.get_all ''' ret = _get_systemd_services() - ret.update(set(_get_sysv_services())) + ret.update(set(_get_sysv_services(systemd_services=ret))) return sorted(ret) diff --git a/tests/unit/modules/systemd_test.py b/tests/unit/modules/systemd_test.py index f10bceae60..b4c8b276ba 100644 --- a/tests/unit/modules/systemd_test.py +++ b/tests/unit/modules/systemd_test.py @@ -168,8 +168,8 @@ class SystemdTestCase(TestCase): ''' listdir_mock = MagicMock(side_effect=[ ['foo.service', 'multi-user.target.wants', 'mytimer.timer'], + [], ['foo.service', 'multi-user.target.wants', 'bar.service'], - ['mysql', 'nginx', 'README'], ['mysql', 'nginx', 'README'] ]) access_mock = MagicMock( From 1e2a04cfc5ea9bca92b66dc090d86a06cf6c480d Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Sun, 2 Apr 2017 22:11:38 -0500 Subject: [PATCH 04/10] Backport auth and custom registry fixes from #40480 to 2016.3 branch --- salt/modules/dockerng.py | 597 +++++++++++----------------- tests/unit/modules/dockerng_test.py | 162 ++++---- 2 files changed, 333 insertions(+), 426 deletions(-) diff --git a/salt/modules/dockerng.py b/salt/modules/dockerng.py index cb86ef6878..60ee1fe0f3 100644 --- a/salt/modules/dockerng.py +++ b/salt/modules/dockerng.py @@ -44,80 +44,66 @@ Docker_. docker-py can easily be installed using :py:func:`pip.install Authentication -------------- -To push or pull images, credentials must be configured. Because a password must -be used, it is recommended to place this configuration in :ref:`Pillar -` data. The configuration schema is as follows: +If you have previously performed a ``docker login`` from the minion, then the +credentials saved in ``~/.docker/config.json`` will be used for any actions +which require authentication. If not, then credentials can be configured in +Pillar data. The configuration schema is as follows: .. code-block:: yaml docker-registries: : - email: - password: username: - reauth: + password: For example: .. code-block:: yaml docker-registries: - https://index.docker.io/v1/: - email: foo@foo.com - password: s3cr3t + hub: username: foo + password: s3cr3t -Reauth is an optional parameter that forces the docker login to reauthorize using -the credentials passed in the pillar data. Defaults to false. +.. note:: + As of the 2016.3.7, 2016.11.4, and Nitrogen releases of Salt, credentials + for the Docker Hub can be configured simply by specifying ``hub`` in place + of the registry URL. In earlier releases, it is necessary to specify the + actual registry URL for the Docker Hub (i.e. + ``https://index.docker.io/v1/``). -.. versionadded:: 2016.3.5,2016.11.1 - -For example: +More than one registry can be configured. Salt will look for Docker credentials +in the ``docker-registries`` Pillar key, as well as any key ending in +``-docker-registries``. For example: .. code-block:: yaml docker-registries: - https://index.docker.io/v1/: - email: foo@foo.com - password: s3cr3t + 'https://mydomain.tld/registry:5000': username: foo - reauth: True - -Mulitiple registries can be configured. This can be done in one of two ways. -The first way is to configure each registry under the ``docker-registries`` -pillar key. - -.. code-block:: yaml - - docker-registries: - https://index.foo.io/v1/: - email: foo@foo.com password: s3cr3t - username: foo - https://index.bar.io/v1/: - email: foo@foo.com - password: s3cr3t - username: foo - -The second way is to use separate pillar variables ending in -``-docker-registries``: - -.. code-block:: yaml foo-docker-registries: https://index.foo.io/v1/: - email: foo@foo.com - password: s3cr3t username: foo + password: s3cr3t bar-docker-registries: https://index.bar.io/v1/: - email: foo@foo.com - password: s3cr3t username: foo + password: s3cr3t + +To login to the configured registries, use the :py:func:`docker.login +` function. This only needs to be done once for a +given registry, and it will store/update the credentials in +``~/.docker/config.json``. + +.. note:: + For Salt releases before 2016.3.7 and 2016.11.4, :py:func:`docker.login + ` is not available. Instead, Salt will try to + authenticate using each of your configured registries for each push/pull, + behavior which is not correct and has been resolved in newer releases. -Both methods can be combined; any registry configured under -``docker-registries`` or ``*-docker-registries`` will be detected. Configuration Options --------------------- @@ -126,7 +112,8 @@ The following configuration options can be set to fine-tune how Salt uses Docker: - ``docker.url``: URL to the docker service (default: local socket). -- ``docker.version``: API version to use +- ``docker.version``: API version to use (should not need to be set manually in + the vast majority of cases) - ``docker.exec_driver``: Execution driver to use, one of ``nsenter``, ``lxc-attach``, or ``docker-exec``. See the :ref:`Executing Commands Within a Running Container ` section for more details on how @@ -135,68 +122,6 @@ Docker: These configuration options are retrieved using :py:mod:`config.get ` (click the link for further information). -Functions ---------- - -- Information Gathering - - :py:func:`dockerng.depends ` - - :py:func:`dockerng.diff ` - - :py:func:`dockerng.exists ` - - :py:func:`dockerng.history ` - - :py:func:`dockerng.images ` - - :py:func:`dockerng.info ` - - :py:func:`dockerng.inspect ` - - :py:func:`dockerng.inspect_container - ` - - :py:func:`dockerng.inspect_image ` - - :py:func:`dockerng.list_containers - ` - - :py:func:`dockerng.list_tags ` - - :py:func:`dockerng.logs ` - - :py:func:`dockerng.pid ` - - :py:func:`dockerng.port ` - - :py:func:`dockerng.ps ` - - :py:func:`dockerng.state ` - - :py:func:`dockerng.search ` - - :py:func:`dockerng.top ` - - :py:func:`dockerng.version ` -- Container Management - - :py:func:`dockerng.create ` - - :py:func:`dockerng.copy_from ` - - :py:func:`dockerng.copy_to ` - - :py:func:`dockerng.export ` - - :py:func:`dockerng.rm ` -- Management of Container State - - :py:func:`dockerng.kill ` - - :py:func:`dockerng.pause ` - - :py:func:`dockerng.restart ` - - :py:func:`dockerng.start ` - - :py:func:`dockerng.stop ` - - :py:func:`dockerng.unpause ` - - :py:func:`dockerng.wait ` -- Image Management - - :py:func:`dockerng.build ` - - :py:func:`dockerng.commit ` - - :py:func:`dockerng.dangling ` - - :py:func:`dockerng.import ` - - :py:func:`dockerng.load ` - - :py:func:`dockerng.pull ` - - :py:func:`dockerng.push ` - - :py:func:`dockerng.rmi ` - - :py:func:`dockerng.save ` - - :py:func:`dockerng.tag ` -- Network Management - - :py:func:`dockerng.networks ` - - :py:func:`dockerng.create_network ` - - :py:func:`dockerng.remove_network ` - - :py:func:`dockerng.inspect_network - ` - - :py:func:`dockerng.connect_container_to_network - ` - - :py:func:`dockerng.disconnect_container_from_network - ` - - .. _docker-execution-driver: @@ -286,8 +211,6 @@ import time from salt.exceptions import CommandExecutionError, SaltInvocationError from salt.ext.six.moves import map # pylint: disable=import-error,redefined-builtin from salt.utils.args import get_function_argspec as _argspec -from salt.utils.decorators \ - import identical_signature_wrapper as _mimic_signature import salt.utils # Import 3rd-party libs @@ -622,55 +545,55 @@ def _get_docker_py_versioninfo(): pass +def _get_client(**kwargs): + client_kwargs = {} + if 'client_timeout' in kwargs: + client_kwargs['timeout'] = kwargs.pop('client_timeout') + for key, val in (('base_url', 'docker.url'), + ('version', 'docker.version')): + param = __salt__['config.get'](val, NOTSET) + if param is not NOTSET: + client_kwargs[key] = param + + if 'base_url' not in client_kwargs and 'DOCKER_HOST' in os.environ: + # Check if the DOCKER_HOST environment variable has been set + client_kwargs['base_url'] = os.environ.get('DOCKER_HOST') + + if 'version' not in client_kwargs: + # Let docker-py auto detect docker version incase + # it's not defined by user. + client_kwargs['version'] = 'auto' + + docker_machine = __salt__['config.get']('docker.machine', NOTSET) + + if docker_machine is not NOTSET: + docker_machine_json = __salt__['cmd.run']( + ['docker-machine', 'inspect', docker_machine], + python_shell=False) + try: + docker_machine_json = json.loads(docker_machine_json) + docker_machine_tls = \ + docker_machine_json['HostOptions']['AuthOptions'] + docker_machine_ip = docker_machine_json['Driver']['IPAddress'] + client_kwargs['base_url'] = \ + 'https://' + docker_machine_ip + ':2376' + client_kwargs['tls'] = docker.tls.TLSConfig( + client_cert=(docker_machine_tls['ClientCertPath'], + docker_machine_tls['ClientKeyPath']), + ca_cert=docker_machine_tls['CaCertPath'], + assert_hostname=False, + verify=True) + except Exception as exc: + raise CommandExecutionError( + 'Docker machine {0} failed: {1}'.format(docker_machine, exc)) + try: + # docker-py 2.0 renamed this client attribute + return docker.APIClient(**client_kwargs) + except AttributeError: + return docker.Client(**client_kwargs) + + # Decorators -class _api_version(object): - ''' - Enforce a specific Docker Remote API version - ''' - def __init__(self, api_version): - self.api_version = api_version - - def __call__(self, func): - def wrapper(*args, **kwargs): - ''' - Get the current client version and check it against the one passed - ''' - _get_client() - current_api_version = __context__['docker.client'].api_version - if float(current_api_version) < self.api_version: - raise CommandExecutionError( - 'This function requires a Docker API version of at least ' - '{0}. API version in use is {1}.' - .format(self.api_version, current_api_version) - ) - return func(*args, **salt.utils.clean_kwargs(**kwargs)) - return _mimic_signature(func, wrapper) - - -class _client_version(object): - ''' - Enforce a specific Docker client version - ''' - def __init__(self, version): - self.version = distutils.version.StrictVersion(version) - - def __call__(self, func): - def wrapper(*args, **kwargs): - ''' - Get the current client version and check it against the one passed - ''' - _get_client() - current_version = '.'.join(map(str, _get_docker_py_versioninfo())) - if distutils.version.StrictVersion(current_version) < self.version: - raise CommandExecutionError( - 'This function requires a Docker Client version of at least ' - '{0}. Version in use is {1}.' - .format(self.version, current_version) - ) - return func(*args, **salt.utils.clean_kwargs(**kwargs)) - return _mimic_signature(func, wrapper) - - def _docker_client(wrapped): ''' Decorator to run a function that requires the use of a docker.Client() @@ -681,8 +604,8 @@ def _docker_client(wrapped): ''' Ensure that the client is present ''' - client_timeout = __context__.get('docker.timeout', CLIENT_TIMEOUT) - _get_client(timeout=client_timeout) + if 'docker.client' not in __context__: + __context__['docker.client'] = _get_client(**kwargs) return wrapped(*args, **salt.utils.clean_kwargs(**kwargs)) return wrapper @@ -750,7 +673,7 @@ def _clear_context(): # an exception will be raised if the size of the dict is modified during # iteration. keep_context = ( - 'docker.client', 'docker.exec_driver', 'dockerng._pull_status', + 'docker.exec_driver', 'dockerng._pull_status', 'docker.docker_version', 'docker.docker_py_version' ) for key in list(__context__): @@ -761,51 +684,6 @@ def _clear_context(): pass -def _get_client(timeout=None): - ''' - Obtains a connection to a docker API (socket or URL) based on config.get - mechanism (pillar -> grains) - - By default it will use the base docker-py defaults which - at the time of writing are using the local socket and - the 1.4 API - - Set those keys in your configuration tree somehow: - - - docker.url: URL to the docker service - - docker.version: API version to use (default: "auto") - ''' - # In some edge cases, the client instance is missing attributes. Don't use - # the cached client in those cases. - if 'docker.client' not in __context__ \ - or not hasattr(__context__['docker.client'], 'timeout'): - client_kwargs = {} - for key, val in (('base_url', 'docker.url'), - ('version', 'docker.version')): - param = __salt__['config.get'](val, NOTSET) - if param is not NOTSET: - client_kwargs[key] = param - - if 'base_url' not in client_kwargs and 'DOCKER_HOST' in os.environ: - # Check if the DOCKER_HOST environment variable has been set - client_kwargs['base_url'] = os.environ.get('DOCKER_HOST') - - if 'version' not in client_kwargs: - # Let docker-py auto detect docker version incase - # it's not defined by user. - client_kwargs['version'] = 'auto' - - try: - # docker-py 2.0 renamed this client attribute - __context__['docker.client'] = docker.APIClient(**client_kwargs) - except AttributeError: - __context__['docker.client'] = docker.Client(**client_kwargs) - - # Set a new timeout if one was passed - if timeout is not None and __context__['docker.client'].timeout != timeout: - __context__['docker.client'].timeout = timeout - - def _get_md5(name, path): ''' Get the MD5 checksum of a file from a container @@ -825,33 +703,6 @@ def _get_exec_driver(): Get the method to be used in shell commands ''' contextkey = 'docker.exec_driver' - ''' - docker-exec won't be used by default until we reach a version where it - supports running commands as a user other than the effective user of the - container. - - See: https://groups.google.com/forum/#!topic/salt-users/i6Eq4rf5ml0 - - if contextkey in __context__: - return __context__[contextkey] - - from_config = __salt__['config.get'](contextkey, None) - if from_config is not None: - __context__[contextkey] = from_config - else: - _version = version() - if 'VersionInfo' in _version: - if _version['VersionInfo'] >= (1, 3, 0): - __context__[contextkey] = 'docker-exec' - elif distutils.version.LooseVersion(version()['Version']) \ - >= distutils.version.LooseVersion('1.3.0'): - # LooseVersion is less preferable, but OK as a fallback. - __context__[contextkey] = 'docker-exec' - - # If the version_info tuple revealed a version < 1.3.0, the key will yet to - # have been set in __context__, so we'll check if it's there yet and if - # not, proceed with detecting execution driver from the output of info(). - ''' # pylint: disable=pointless-string-statement if contextkey not in __context__: from_config = __salt__['config.get'](contextkey, None) # This if block can be removed once we make docker-exec a default @@ -889,19 +740,24 @@ def _get_repo_tag(image, default_tag='latest'): ''' Resolves the docker repo:tag notation and returns repo name and tag ''' - if ':' in image: + if not isinstance(image, six.string_types): + image = str(image) + try: r_name, r_tag = image.rsplit(':', 1) - if not r_tag: - # Would happen if some wiseguy requests a tag ending in a colon - # (e.g. 'somerepo:') - log.warning( - 'Assuming tag \'{0}\' for repo \'{1}\'' - .format(default_tag, image) - ) - r_tag = default_tag - else: + except ValueError: r_name = image r_tag = default_tag + if not r_tag: + # Would happen if some wiseguy requests a tag ending in a colon + # (e.g. 'somerepo:') + log.warning( + 'Assuming tag \'%s\' for repo \'%s\'', default_tag, image + ) + r_tag = default_tag + elif '/' in r_tag: + # Public registry notation with no tag specified + # (e.g. foo.bar.com:5000/imagename) + return image, default_tag return r_name, r_tag @@ -952,15 +808,19 @@ def _size_fmt(num): @_docker_client def _client_wrapper(attr, *args, **kwargs): ''' - Common functionality for getting information from a container + Common functionality for running low-level API calls ''' catch_api_errors = kwargs.pop('catch_api_errors', True) - func = getattr(__context__['docker.client'], attr) + func = getattr(__context__['docker.client'], attr, None) if func is None: raise SaltInvocationError('Invalid client action \'{0}\''.format(attr)) err = '' try: - return func(*args, **kwargs) + log.debug( + 'Attempting to run docker-py\'s "%s" function ' + 'with args=%s and kwargs=%s', attr, args, kwargs + ) + ret = func(*args, **kwargs) except docker.errors.APIError as exc: if catch_api_errors: # Generic handling of Docker API errors @@ -971,77 +831,34 @@ def _client_wrapper(attr, *args, **kwargs): else: # Allow API errors to be caught further up the stack raise + except docker.errors.DockerException as exc: + # More general docker exception (catches InvalidVersion, etc.) + raise CommandExecutionError(exc.__str__()) except Exception as exc: - err = '{0}'.format(exc) + err = exc.__str__() + else: + if kwargs.get('stream', False): + api_events = [] + try: + for event in ret: + api_events.append(json.loads(event)) + except Exception as exc: + raise CommandExecutionError( + 'Unable to interpret API event: \'{0}\''.format(event), + info={'Error': exc.__str__()} + ) + return api_events + else: + return ret + + # If we're here, it's because an exception was caught earlier, and the + # API command failed. msg = 'Unable to perform {0}'.format(attr) if err: msg += ': {0}'.format(err) raise CommandExecutionError(msg) -@_docker_client -def _image_wrapper(attr, *args, **kwargs): - ''' - Wrapper to run a docker-py function and return a list of dictionaries - ''' - catch_api_errors = kwargs.pop('catch_api_errors', True) - - if kwargs.pop('client_auth', False): - # Set credentials - registry_auth_config = __pillar__.get('docker-registries', {}) - for key, data in six.iteritems(__pillar__): - if key.endswith('-docker-registries'): - registry_auth_config.update(data) - - err = ( - '{0} Docker credentials{1}. Please see the dockerng remote ' - 'execution module documentation for information on how to ' - 'configure authentication.' - ) - try: - for registry, creds in six.iteritems(registry_auth_config): - __context__['docker.client'].login( - creds['username'], - password=creds['password'], - email=creds.get('email'), - registry=registry, - reauth=creds.get('reauth', False)) - except KeyError: - raise SaltInvocationError( - err.format('Incomplete', ' for registry {0}'.format(registry)) - ) - client_timeout = kwargs.pop('client_timeout', None) - if client_timeout is not None: - __context__['docker.client'].timeout = client_timeout - - func = getattr(__context__['docker.client'], attr) - if func is None: - raise SaltInvocationError('Invalid client action \'{0}\''.format(attr)) - ret = [] - try: - output = func(*args, **kwargs) - if not kwargs.get('stream', False): - output = output.splitlines() - for line in output: - ret.append(json.loads(line)) - except docker.errors.APIError as exc: - if catch_api_errors: - # Generic handling of Docker API errors - raise CommandExecutionError( - 'Error {0}: {1}'.format(exc.response.status_code, - exc.explanation) - ) - else: - # Allow API errors to be caught further up the stack - raise - except Exception as exc: - raise CommandExecutionError( - 'Error occurred performing docker {0}: {1}'.format(attr, exc) - ) - - return ret - - def _build_status(data, item): ''' Process a status update from a docker build, updating the data structure @@ -1926,6 +1743,98 @@ def _validate_input(kwargs, pass +def login(*registries): + ''' + .. versionadded:: 2016.3.7,2016.11.4,Nitrogen + + Performs a ``docker login`` to authenticate to one or more configured + repositories. See the documentation at the top of this page to configure + authentication credentials. + + Multiple registry URLs (matching those configured in Pillar) can be passed, + and Salt will attempt to login to *just* those registries. If no registry + URLs are provided, Salt will attempt to login to *all* configured + registries. + + **RETURN DATA** + + A dictionary containing the following keys: + + - ``Results`` - A dictionary mapping registry URLs to the authentication + result. ``True`` means a successful login, ``False`` means a failed + login. + - ``Errors`` - A list of errors encountered during the course of this + function. + + CLI Example: + + .. code-block:: bash + + salt myminion docker.login + salt myminion docker.login hub + salt myminion docker.login hub https://mydomain.tld/registry/ + ''' + # NOTE: This function uses the "docker login" CLI command so that login + # information is added to the config.json, since docker-py isn't designed + # to do so. + registry_auth = __pillar__.get('docker-registries', {}) + ret = {} + errors = ret.setdefault('Errors', []) + if not isinstance(registry_auth, dict): + errors.append('\'docker-registries\' Pillar value must be a dictionary') + registry_auth = {} + for key, data in six.iteritems(__pillar__): + try: + if key.endswith('-docker-registries'): + try: + registry_auth.update(data) + except TypeError: + errors.append( + '\'{0}\' Pillar value must be a dictionary'.format(key) + ) + except AttributeError: + pass + + # If no registries passed, we will auth to all of them + if not registries: + registries = list(registry_auth) + + results = ret.setdefault('Results', {}) + for registry in registries: + if registry not in registry_auth: + errors.append( + 'No match found for registry \'{0}\''.format(registry) + ) + continue + try: + username = registry_auth[registry]['username'] + password = registry_auth[registry]['password'] + except TypeError: + errors.append( + 'Invalid configuration for registry \'{0}\''.format(registry) + ) + except KeyError as exc: + errors.append( + 'Missing {0} for registry \'{1}\''.format(exc, registry) + ) + else: + cmd = ['docker', 'login', '-u', username, '-p', password] + if registry.lower() != 'hub': + cmd.append(registry) + login_cmd = __salt__['cmd.run_all']( + cmd, + python_shell=False, + output_loglevel='quiet', + ) + results[registry] = login_cmd['retcode'] == 0 + if not results[registry]: + if login_cmd['stderr']: + errors.append(login_cmd['stderr']) + elif login_cmd['stdout']: + errors.append(login_cmd['stdout']) + return ret + + # Functions for information gathering def depends(name): ''' @@ -2199,7 +2108,6 @@ def images(verbose=False, **kwargs): return ret -@_docker_client def info(): ''' Returns a dictionary of system-wide information. Equivalent to running @@ -3061,7 +2969,7 @@ def create(image, # API v1.15 introduced HostConfig parameter # https://docs.docker.com/engine/reference/api/docker_remote_api_v1.15/#create-a-container if salt.utils.version_cmp(version()['ApiVersion'], '1.15') > 0: - client = __context__['docker.client'] + client = _get_client() host_config_args = get_client_args()['host_config'] create_kwargs['host_config'] = client.create_host_config( **dict((arg, create_kwargs.pop(arg, None)) for arg in host_config_args if arg != 'version') @@ -3770,10 +3678,10 @@ def import_(source, path = __salt__['container_resource.cache_file'](source) time_started = time.time() - response = _image_wrapper('import_image', - path, - repository=repo_name, - tag=repo_tag) + response = _client_wrapper('import_image', + path, + repository=repo_name, + tag=repo_tag) ret = {'Time_Elapsed': time.time() - time_started} _clear_context() @@ -3930,8 +3838,7 @@ def pull(image, api_response=False, client_timeout=CLIENT_TIMEOUT): ''' - Pulls an image from a Docker registry. See the documentation at the top of - this page to configure authenticated access. + Pulls an image from a Docker registry image Image to be pulled, in ``repo:tag`` notation. If just the repository @@ -3980,13 +3887,12 @@ def pull(image, repo_name, repo_tag = _get_repo_tag(image) kwargs = {'tag': repo_tag, 'stream': True, - 'client_auth': True, 'client_timeout': client_timeout} if insecure_registry: kwargs['insecure_registry'] = insecure_registry time_started = time.time() - response = _image_wrapper('pull', repo_name, **kwargs) + response = _client_wrapper('pull', repo_name, **kwargs) ret = {'Time_Elapsed': time.time() - time_started} _clear_context() @@ -4034,7 +3940,7 @@ def push(image, This is due to changes in the Docker Remote API. Pushes an image to a Docker registry. See the documentation at top of this - page to configure authenticated access. + page to configure authentication credentials. image Image to be pushed, in ``repo:tag`` notation. @@ -4085,13 +3991,12 @@ def push(image, kwargs = {'tag': repo_tag, 'stream': True, - 'client_auth': True, 'client_timeout': client_timeout} if insecure_registry: kwargs['insecure_registry'] = insecure_registry time_started = time.time() - response = _image_wrapper('push', repo_name, **kwargs) + response = _client_wrapper('push', repo_name, **kwargs) ret = {'Time_Elapsed': time.time() - time_started} _clear_context() @@ -4115,6 +4020,8 @@ def push(image, elif item_type == 'errorDetail': _error_detail(errors, item) + if errors: + ret['Errors'] = errors return ret @@ -4167,18 +4074,19 @@ def rmi(*names, **kwargs): catch_api_errors=False) except docker.errors.APIError as exc: if exc.response.status_code == 409: - err = ('Unable to remove image \'{0}\' because it is in ' - 'use by '.format(name)) + errors.append(exc.explanation) deps = depends(name) - if deps['Containers']: - err += 'container(s): {0}'.format( - ', '.join(deps['Containers']) - ) - if deps['Images']: + if deps['Containers'] or deps['Images']: + err = 'Image is in use by ' if deps['Containers']: - err += ' and ' - err += 'image(s): {0}'.format(', '.join(deps['Images'])) - errors.append(err) + err += 'container(s): {0}'.format( + ', '.join(deps['Containers']) + ) + if deps['Images']: + if deps['Containers']: + err += ' and ' + err += 'image(s): {0}'.format(', '.join(deps['Images'])) + errors.append(err) else: errors.append('Error {0}: {1}'.format(exc.response.status_code, exc.explanation)) @@ -4426,11 +4334,8 @@ def tag_(name, image, force=False): # Only non-error return case is a True return, so just return the response return response + # Network Management - - -@_api_version(1.21) -@_client_version('1.5.0') def networks(names=None, ids=None): ''' List existing networks @@ -4457,8 +4362,6 @@ def networks(names=None, ids=None): return response -@_api_version(1.21) -@_client_version('1.5.0') def create_network(name, driver=None): ''' Create a new network @@ -4481,8 +4384,6 @@ def create_network(name, driver=None): return response -@_api_version(1.21) -@_client_version('1.5.0') def remove_network(network_id): ''' Remove a network @@ -4502,8 +4403,6 @@ def remove_network(network_id): return response -@_api_version(1.21) -@_client_version('1.5.0') def inspect_network(network_id): ''' Inspect Network @@ -4523,8 +4422,6 @@ def inspect_network(network_id): return response -@_api_version(1.21) -@_client_version('1.5.0') def connect_container_to_network(container, network_id): ''' Connect container to network. @@ -4549,8 +4446,6 @@ def connect_container_to_network(container, network_id): return response -@_api_version(1.21) -@_client_version('1.5.0') def disconnect_container_from_network(container, network_id): ''' Disconnect container from network. @@ -4577,8 +4472,6 @@ def disconnect_container_from_network(container, network_id): # Volume Management -@_api_version(1.21) -@_client_version('1.5.0') def volumes(filters=None): ''' List existing volumes @@ -4600,8 +4493,6 @@ def volumes(filters=None): return response -@_api_version(1.21) -@_client_version('1.5.0') def create_volume(name, driver=None, driver_opts=None): ''' Create a new volume @@ -4630,8 +4521,6 @@ def create_volume(name, driver=None, driver_opts=None): return response -@_api_version(1.21) -@_client_version('1.5.0') def remove_volume(name): ''' Remove a volume @@ -4653,8 +4542,6 @@ def remove_volume(name): return response -@_api_version(1.21) -@_client_version('1.5.0') def inspect_volume(name): ''' Inspect Volume @@ -4707,7 +4594,6 @@ def kill(name): @_refresh_mine_cache -@_api_version(1.12) @_ensure_exists def pause(name): ''' @@ -4902,7 +4788,6 @@ def stop(name, timeout=STOP_TIMEOUT, **kwargs): @_refresh_mine_cache -@_api_version(1.12) @_ensure_exists def unpause(name): ''' diff --git a/tests/unit/modules/dockerng_test.py b/tests/unit/modules/dockerng_test.py index 0be5570d2c..bc9ff9a95a 100644 --- a/tests/unit/modules/dockerng_test.py +++ b/tests/unit/modules/dockerng_test.py @@ -23,7 +23,6 @@ ensure_in_syspath('../../') from salt.modules import dockerng as dockerng_mod from salt.exceptions import CommandExecutionError, SaltInvocationError -dockerng_mod.__context__ = {'docker.docker_version': ''} dockerng_mod.__salt__ = {} @@ -36,17 +35,25 @@ class DockerngTestCase(TestCase): ''' Validate dockerng module ''' + def setUp(self): + ''' + Ensure we aren't persisting context dunders between tests + ''' + dockerng_mod.__context__ = {'docker.docker_version': ''} def test_ps_with_host_true(self): ''' Check that dockerng.ps called with host is ``True``, include resutlt of ``network.interfaces`` command in returned result. ''' + client = Mock() + client.containers = MagicMock(return_value=[]) + get_client_mock = MagicMock(return_value=client) + network_interfaces = Mock(return_value={'mocked': None}) with patch.dict(dockerng_mod.__salt__, {'network.interfaces': network_interfaces}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': MagicMock()}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): ret = dockerng_mod.ps_(host=True) self.assertEqual(ret, {'host': {'interfaces': {'mocked': None}}}) @@ -55,9 +62,11 @@ class DockerngTestCase(TestCase): ''' Check that dockerng.ps accept filters parameter. ''' - client = MagicMock() - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + client = Mock() + client.containers = MagicMock(return_value=[]) + get_client_mock = MagicMock(return_value=client) + + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.ps_(filters={'label': 'KEY'}) client.containers.assert_called_once_with( all=True, @@ -84,14 +93,15 @@ class DockerngTestCase(TestCase): ): mine_send = Mock() command = getattr(dockerng_mod, command_name) - docker_client = MagicMock() - docker_client.api_version = '1.12' + client = MagicMock() + client.api_version = '1.12' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__salt__, {'mine.send': mine_send, 'container_resource.run': MagicMock(), 'cp.cache_file': MagicMock(return_value=False)}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': docker_client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): command('container', *args) mine_send.assert_called_with('dockerng.ps', verbose=True, all=True, host=True) @@ -114,10 +124,11 @@ class DockerngTestCase(TestCase): client.api_version = '1.19' client.create_host_config.return_value = host_config client.create_container.return_value = {} + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.create('image', cmd='ls', name='ctn') client.create_container.assert_called_once_with( command='ls', @@ -143,10 +154,11 @@ class DockerngTestCase(TestCase): client.api_version = '1.19' client.create_host_config.return_value = host_config client.create_container.return_value = {} + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.create('image', name='ctn', publish_all_ports=True) client.create_container.assert_called_once_with( host_config=host_config, @@ -171,10 +183,11 @@ class DockerngTestCase(TestCase): client.api_version = '1.19' client.create_host_config.return_value = host_config client.create_container.return_value = {} + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.create( 'image', name='ctn', @@ -206,10 +219,10 @@ class DockerngTestCase(TestCase): client.api_version = '1.19' client.create_host_config.return_value = host_config client.create_container.return_value = {} + get_client_mock = MagicMock(return_value=client) with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.create( 'image', name='ctn', @@ -241,10 +254,11 @@ class DockerngTestCase(TestCase): client.api_version = '1.19' client.create_host_config.return_value = host_config client.create_container.return_value = {} + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): self.assertRaises(SaltInvocationError, dockerng_mod.create, 'image', @@ -271,10 +285,11 @@ class DockerngTestCase(TestCase): client.api_version = '1.19' client.create_host_config.return_value = host_config client.create_container.return_value = {} + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.create( 'image', name='ctn', @@ -301,10 +316,11 @@ class DockerngTestCase(TestCase): host_config = {} client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.networks( names=['foo'], ids=['01234'], @@ -327,18 +343,13 @@ class DockerngTestCase(TestCase): host_config = {} client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): - dockerng_mod.create_network( - 'foo', - driver='bridge', - ) - client.create_network.assert_called_once_with( - 'foo', - driver='bridge', - ) + with patch.object(dockerng_mod, '_get_client', get_client_mock): + dockerng_mod.create_network('foo', driver='bridge') + client.create_network.assert_called_once_with('foo', driver='bridge') @skipIf(_docker_py_version() < (1, 5, 0), 'docker module must be installed to run this test or is too old. >=1.5.0') @@ -353,10 +364,11 @@ class DockerngTestCase(TestCase): host_config = {} client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.remove_network('foo') client.remove_network.assert_called_once_with('foo') @@ -373,10 +385,11 @@ class DockerngTestCase(TestCase): host_config = {} client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.inspect_network('foo') client.inspect_network.assert_called_once_with('foo') @@ -393,10 +406,11 @@ class DockerngTestCase(TestCase): host_config = {} client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.connect_container_to_network('container', 'foo') client.connect_container_to_network.assert_called_once_with( 'container', 'foo') @@ -414,10 +428,11 @@ class DockerngTestCase(TestCase): host_config = {} client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.disconnect_container_from_network('container', 'foo') client.disconnect_container_from_network.assert_called_once_with( 'container', 'foo') @@ -434,16 +449,13 @@ class DockerngTestCase(TestCase): } client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): - dockerng_mod.volumes( - filters={'dangling': [True]}, - ) - client.volumes.assert_called_once_with( - filters={'dangling': [True]}, - ) + with patch.object(dockerng_mod, '_get_client', get_client_mock): + dockerng_mod.volumes(filters={'dangling': [True]}) + client.volumes.assert_called_once_with(filters={'dangling': [True]}) @skipIf(_docker_py_version() < (1, 5, 0), 'docker module must be installed to run this test or is too old. >=1.5.0') @@ -457,10 +469,11 @@ class DockerngTestCase(TestCase): } client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.create_volume( 'foo', driver='bridge', @@ -484,10 +497,11 @@ class DockerngTestCase(TestCase): } client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.remove_volume('foo') client.remove_volume.assert_called_once_with('foo') @@ -503,10 +517,11 @@ class DockerngTestCase(TestCase): } client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) + with patch.dict(dockerng_mod.__dict__, {'__salt__': __salt__}): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod.inspect_volume('foo') client.inspect_volume.assert_called_once_with('foo') @@ -514,13 +529,14 @@ class DockerngTestCase(TestCase): client = Mock() client.api_version = '1.21' client.wait = Mock(return_value=0) + get_client_mock = MagicMock(return_value=client) + dockerng_inspect_container = Mock(side_effect=[ {'State': {'Running': True}}, {'State': {'Stopped': True}}]) with patch.object(dockerng_mod, 'inspect_container', dockerng_inspect_container): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod._clear_context() result = dockerng_mod.wait('foo') self.assertEqual(result, {'result': True, @@ -532,14 +548,15 @@ class DockerngTestCase(TestCase): client = Mock() client.api_version = '1.21' client.wait = Mock(return_value=0) + get_client_mock = MagicMock(return_value=client) + dockerng_inspect_container = Mock(side_effect=[ {'State': {'Stopped': True}}, {'State': {'Stopped': True}}, ]) with patch.object(dockerng_mod, 'inspect_container', dockerng_inspect_container): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod._clear_context() result = dockerng_mod.wait('foo') self.assertEqual(result, {'result': False, @@ -552,14 +569,15 @@ class DockerngTestCase(TestCase): client = Mock() client.api_version = '1.21' client.wait = Mock(return_value=0) + get_client_mock = MagicMock(return_value=client) + dockerng_inspect_container = Mock(side_effect=[ {'State': {'Stopped': True}}, {'State': {'Stopped': True}}, ]) with patch.object(dockerng_mod, 'inspect_container', dockerng_inspect_container): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod._clear_context() result = dockerng_mod.wait('foo', ignore_already_stopped=True) self.assertEqual(result, {'result': True, @@ -571,11 +589,12 @@ class DockerngTestCase(TestCase): def test_wait_success_absent_container(self): client = Mock() client.api_version = '1.21' + get_client_mock = MagicMock(return_value=client) dockerng_inspect_container = Mock(side_effect=CommandExecutionError) + with patch.object(dockerng_mod, 'inspect_container', dockerng_inspect_container): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod._clear_context() result = dockerng_mod.wait('foo', ignore_already_stopped=True) self.assertEqual(result, {'result': True, @@ -585,13 +604,14 @@ class DockerngTestCase(TestCase): client = Mock() client.api_version = '1.21' client.wait = Mock(return_value=1) + get_client_mock = MagicMock(return_value=client) dockerng_inspect_container = Mock(side_effect=[ {'State': {'Running': True}}, {'State': {'Stopped': True}}]) + with patch.object(dockerng_mod, 'inspect_container', dockerng_inspect_container): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod._clear_context() result = dockerng_mod.wait('foo', fail_on_exit_status=True) self.assertEqual(result, {'result': False, @@ -603,13 +623,14 @@ class DockerngTestCase(TestCase): client = Mock() client.api_version = '1.21' client.wait = Mock(return_value=1) + get_client_mock = MagicMock(return_value=client) dockerng_inspect_container = Mock(side_effect=[ {'State': {'Stopped': True}}, {'State': {'Stopped': True}}]) + with patch.object(dockerng_mod, 'inspect_container', dockerng_inspect_container): - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod._clear_context() result = dockerng_mod.wait('foo', ignore_already_stopped=True, @@ -632,8 +653,9 @@ class DockerngTestCase(TestCase): {'Id': 'sha256:abcdef'}, {'Id': 'sha256:abcdefg', 'RepoTags': ['image:latest']}]) - with patch.dict(dockerng_mod.__context__, - {'docker.client': client}): + get_client_mock = MagicMock(return_value=client) + + with patch.object(dockerng_mod, '_get_client', get_client_mock): dockerng_mod._clear_context() result = dockerng_mod.images() self.assertEqual(result, From dcef1e0d4b023867562e6eca42bd5ddd1dff8006 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Mon, 3 Apr 2017 13:30:10 -0500 Subject: [PATCH 05/10] Make sure we keep the cached client when clearing context --- salt/modules/dockerng.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/dockerng.py b/salt/modules/dockerng.py index 60ee1fe0f3..59a3bb27c2 100644 --- a/salt/modules/dockerng.py +++ b/salt/modules/dockerng.py @@ -673,7 +673,7 @@ def _clear_context(): # an exception will be raised if the size of the dict is modified during # iteration. keep_context = ( - 'docker.exec_driver', 'dockerng._pull_status', + 'docker.client', 'docker.exec_driver', 'dockerng._pull_status', 'docker.docker_version', 'docker.docker_py_version' ) for key in list(__context__): From 6e2f9080ca6cf678fa0ef23c26bca772fe10e6cf Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Mon, 3 Apr 2017 16:03:40 -0500 Subject: [PATCH 06/10] update docs for logging handlers --- doc/ref/configuration/logging/handlers/index.rst | 4 +++- .../logging/handlers/salt.log.handlers.fluent_mod.rst | 5 +++++ .../logging/handlers/salt.log.handlers.log4mongo_mod.rst | 5 +++++ .../logging/handlers/salt.log.handlers.logstash_mod.rst | 6 +++++- .../logging/handlers/salt.log.handlers.sentry_mod.rst | 6 +++++- 5 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 doc/ref/configuration/logging/handlers/salt.log.handlers.fluent_mod.rst create mode 100644 doc/ref/configuration/logging/handlers/salt.log.handlers.log4mongo_mod.rst diff --git a/doc/ref/configuration/logging/handlers/index.rst b/doc/ref/configuration/logging/handlers/index.rst index d6c614ca01..e1051bad72 100644 --- a/doc/ref/configuration/logging/handlers/index.rst +++ b/doc/ref/configuration/logging/handlers/index.rst @@ -9,5 +9,7 @@ External Logging Handlers :toctree: :template: autosummary.rst.tmpl + fluent_mod + log4mongo_mod logstash_mod - sentry_mod \ No newline at end of file + sentry_mod diff --git a/doc/ref/configuration/logging/handlers/salt.log.handlers.fluent_mod.rst b/doc/ref/configuration/logging/handlers/salt.log.handlers.fluent_mod.rst new file mode 100644 index 0000000000..e29dfac1d2 --- /dev/null +++ b/doc/ref/configuration/logging/handlers/salt.log.handlers.fluent_mod.rst @@ -0,0 +1,5 @@ +============================ +salt.log.handlers.fluent_mod +============================ + +.. automodule:: salt.log.handlers.fluent_mod diff --git a/doc/ref/configuration/logging/handlers/salt.log.handlers.log4mongo_mod.rst b/doc/ref/configuration/logging/handlers/salt.log.handlers.log4mongo_mod.rst new file mode 100644 index 0000000000..c0ac829ab9 --- /dev/null +++ b/doc/ref/configuration/logging/handlers/salt.log.handlers.log4mongo_mod.rst @@ -0,0 +1,5 @@ +=============================== +salt.log.handlers.log4mongo_mod +=============================== + +.. automodule:: salt.log.handlers.log4mongo_mod diff --git a/doc/ref/configuration/logging/handlers/salt.log.handlers.logstash_mod.rst b/doc/ref/configuration/logging/handlers/salt.log.handlers.logstash_mod.rst index 7d286b3608..096d3150bf 100644 --- a/doc/ref/configuration/logging/handlers/salt.log.handlers.logstash_mod.rst +++ b/doc/ref/configuration/logging/handlers/salt.log.handlers.logstash_mod.rst @@ -1 +1,5 @@ -.. automodule:: salt.log.handlers.logstash_mod \ No newline at end of file +============================== +salt.log.handlers.logstash_mod +============================== + +.. automodule:: salt.log.handlers.logstash_mod diff --git a/doc/ref/configuration/logging/handlers/salt.log.handlers.sentry_mod.rst b/doc/ref/configuration/logging/handlers/salt.log.handlers.sentry_mod.rst index 6a92eadae9..c8bab3b99a 100644 --- a/doc/ref/configuration/logging/handlers/salt.log.handlers.sentry_mod.rst +++ b/doc/ref/configuration/logging/handlers/salt.log.handlers.sentry_mod.rst @@ -1 +1,5 @@ -.. automodule:: salt.log.handlers.sentry_mod \ No newline at end of file +============================ +salt.log.handlers.sentry_mod +============================ + +.. automodule:: salt.log.handlers.sentry_mod From 9ed85f3c598dce2a371fb33216a2f8453fc5885f Mon Sep 17 00:00:00 2001 From: zer0def Date: Tue, 4 Apr 2017 19:15:32 +0200 Subject: [PATCH 07/10] Remove directory content instead of directory itself when using `force_clone` in `git.latest` state. (refs #31363) --- salt/states/git.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/salt/states/git.py b/salt/states/git.py index 0d7f64541e..2d9ba7ecc7 100644 --- a/salt/states/git.py +++ b/salt/states/git.py @@ -1589,6 +1589,7 @@ def latest(name, return _uptodate(ret, target, _format_comments(comments)) else: if os.path.isdir(target): + target_contents = os.listdir(target) if force_clone: # Clone is required, and target directory exists, but the # ``force`` option is enabled, so we need to clear out its @@ -1607,22 +1608,26 @@ def latest(name, 'place (force_clone=True set in git.latest state)' .format(target, name) ) - try: - if os.path.islink(target): - os.unlink(target) - else: + removal_errors = {} + for target_object in target_contents: + target_path = os.path.join(target, target_object) + try: salt.utils.rm_rf(target) - except OSError as exc: + except OSError as exc: + removal_errors[target_path] = exc + if removal_errors: + err_strings = [ + ' {0}\n {1}'.format(k, v) for k, v in removal_errors.items() + ] return _fail( ret, - 'Unable to remove {0}: {1}'.format(target, exc), + 'Unable to remove\n{0}'.format('\n'.join(err_strings)), comments ) - else: - ret['changes']['forced clone'] = True + ret['changes']['forced clone'] = True # Clone is required, but target dir exists and is non-empty. We # can't proceed. - elif os.listdir(target): + elif target_contents: return _fail( ret, 'Target \'{0}\' exists, is non-empty and is not a git ' From 3c750c2b247ad83f1ea7a9b4e1950afd52461449 Mon Sep 17 00:00:00 2001 From: zer0def Date: Tue, 4 Apr 2017 19:23:26 +0200 Subject: [PATCH 08/10] Changed rm_rf's argument to actually remove intended file. (refs #31363) --- salt/states/git.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/states/git.py b/salt/states/git.py index 2d9ba7ecc7..ee0857b175 100644 --- a/salt/states/git.py +++ b/salt/states/git.py @@ -1612,7 +1612,7 @@ def latest(name, for target_object in target_contents: target_path = os.path.join(target, target_object) try: - salt.utils.rm_rf(target) + salt.utils.rm_rf(target_path) except OSError as exc: removal_errors[target_path] = exc if removal_errors: From ad88c58a09c5872a0560a9cbd5dd5d90a1b34886 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 4 Apr 2017 23:15:08 -0500 Subject: [PATCH 09/10] Check master's ssh_minion_opts for fileserver/pillar values and ignore them This prevents the ssh_minion_opts from overwriting values that should have been inherited or otherwise setup by the master for the SSH minion. Resolves #39892 --- salt/config/__init__.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/salt/config/__init__.py b/salt/config/__init__.py index 6ddd3cc5e1..ee996c635d 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -1562,6 +1562,29 @@ def _validate_opts(opts): return True +def _validate_ssh_minion_opts(opts): + ''' + Ensure we're not using any invalid ssh_minion_opts. We want to make sure + that the ssh_minion_opts does not override any pillar or fileserver options + inherited from the master config. To add other items, modify the if + statement in the for loop below. + ''' + ssh_minion_opts = opts.get('ssh_minion_opts', {}) + if not isinstance(ssh_minion_opts, dict): + log.error('Invalidly-formatted ssh_minion_opts') + opts.pop('ssh_minion_opts') + + for opt_name in list(ssh_minion_opts): + if re.match('^[a-z0-9]+fs_', opt_name, flags=re.IGNORECASE) \ + or 'pillar' in opt_name \ + or opt_name in ('fileserver_backend',): + log.warning( + '\'%s\' is not a valid ssh_minion_opts parameter, ignoring', + opt_name + ) + ssh_minion_opts.pop(opt_name) + + def _append_domain(opts): ''' Append a domain to the existing id if it doesn't already exist @@ -3110,6 +3133,7 @@ def master_config(path, env_var='SALT_MASTER_CONFIG', defaults=None, exit_on_con overrides.update(include_config(include, path, verbose=True), exit_on_config_errors=exit_on_config_errors) opts = apply_master_config(overrides, defaults) + _validate_ssh_minion_opts(opts) _validate_opts(opts) # If 'nodegroups:' is uncommented in the master config file, and there are # no nodegroups defined, opts['nodegroups'] will be None. Fix this by From 0918311330755a4a6571f2ae0ed64bdf8149cf1f Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 5 Apr 2017 21:23:15 -0500 Subject: [PATCH 10/10] Don't mark files that already were deleted as errors Also use six.iteritems() --- salt/states/git.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/salt/states/git.py b/salt/states/git.py index ee0857b175..f6edbdacbe 100644 --- a/salt/states/git.py +++ b/salt/states/git.py @@ -14,6 +14,7 @@ from __future__ import absolute_import # Import python libs import copy +import errno import logging import os import re @@ -1614,10 +1615,12 @@ def latest(name, try: salt.utils.rm_rf(target_path) except OSError as exc: - removal_errors[target_path] = exc + if exc.errno != errno.ENOENT: + removal_errors[target_path] = exc if removal_errors: err_strings = [ - ' {0}\n {1}'.format(k, v) for k, v in removal_errors.items() + ' {0}\n {1}'.format(k, v) + for k, v in six.iteritems(removal_errors) ] return _fail( ret,