From e345b26471871fd95c963a3eeaa403e097b9bd81 Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Sat, 30 Apr 2016 19:09:14 -0400 Subject: [PATCH 1/5] Salt the Salt master Use Salt to install the Salt master. The Salt master will not get restarted automatically, neither on configuration changes nor on package changes. Instead, it will need to be manually restarted to avoid interrupting the in-progress highstate as well as allow for proper update ordering. (Salt masters must be updated before Salt minions.) Use Jinja to generate the config file YAML directly, preventing typos in the master config file and providing access to master configuration options in the SLS file (e.g. the file_roots locations). --- salt/map.jinja | 30 +++++++++++++++++++++++------- salt/master.sls | 26 ++++++++++++++++++++++++++ top.sls | 1 + 3 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 salt/master.sls diff --git a/salt/map.jinja b/salt/map.jinja index 6404f29e..9329c112 100644 --- a/salt/map.jinja +++ b/salt/map.jinja @@ -1,10 +1,26 @@ {% - set salt = salt.grains.filter_by({ - 'defaults': { - 'version': '2015.5.8' + set salt = { + 'version': '2015.5.8', + 'master': salt['grains.filter_by']({ + 'defaults': { + 'config': { + 'file_roots': { + 'base': [ '/srv/salt' ] + }, + 'pillar_roots': { + 'base': [ '/srv/pillar' ] + } + } + }, + 'Ubuntu': { + 'pkg': { + 'name': 'salt-master', + 'version': '2015.5.8+ds-1' + } + } }, - }, - base='defaults', - merge=salt.pillar.get('salt', {}) - ) + base='defaults', + grain='os', + ) + } %} diff --git a/salt/master.sls b/salt/master.sls new file mode 100644 index 00000000..6a6ba370 --- /dev/null +++ b/salt/master.sls @@ -0,0 +1,26 @@ +{% from tpldir ~ '/map.jinja' import salt %} + +include: + - .common + +salt-master: + pkg.installed: + - name: {{ salt.master.pkg.name }} + - version: {{ salt.master.pkg.version }} + - require: + - sls: salt.common + service.running: + - enable: True + - require: # Updates and upgrades must be handled manually + - file: /etc/salt/master + - pkg: salt-master + +/etc/salt/master: + file.managed: + - user: root + - group: root + - mode: 644 + - contents: | + {{ salt.master.config|yaml(False)|indent(8) }} + - require: + - pkg: salt-master diff --git a/top.sls b/top.sls index 1c84eb05..737167fd 100644 --- a/top.sls +++ b/top.sls @@ -34,3 +34,4 @@ base: - buildbot.master - homu - nginx + - salt.master From d577a7dcd9a840bb2bb31ff5ecddffa23225a60e Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Sun, 1 May 2016 00:57:06 -0400 Subject: [PATCH 2/5] Use explicit masterless args, not minion config Previously, the minion config was used to specify that a local set of file trees should be used. However, in order to test that the Salt master states function, and to more closely match the actual deployment of our minions, amend the minion config to point to a localhost master. Additionally, amend our Travis config and Vagrantfile to explicitly pass options to set the local file and pillar trees. Only sync them via Vagrant shared folders for the Salt master, which should have them available to allow debugging. Because using Salt is the only way to learn which minions (ids) would have a Salt master running on them, hardcode the salt-master\d+ regex into the Vagrantfile. In the future, this will allow adding a smoke test that check if the master can test.ping a local minion (on the same VM). However, this requires the minion to be started, which requires that the Salt minion also be salted, (which requires this PR to backport the launchctl.py execution module). --- .travis/dispatch.sh | 14 ++++++++++---- .travis/minion | 8 +------- .travis/setup_salt_roots.sh | 14 -------------- Vagrantfile | 32 ++++++++++++++++++++++++-------- 4 files changed, 35 insertions(+), 33 deletions(-) delete mode 100755 .travis/setup_salt_roots.sh diff --git a/.travis/dispatch.sh b/.travis/dispatch.sh index 7a6111a5..f5b98361 100755 --- a/.travis/dispatch.sh +++ b/.travis/dispatch.sh @@ -4,6 +4,13 @@ set -o errexit set -o nounset set -o pipefail +salt_call() { + sudo salt-call \ + --id="${SALT_NODE_ID}" \ + --local --file-root='./.' --pillar-root='./.travis/test_pillars' \ + "$@" +} + travis_fold_start () { printf "travis_fold:start:$1\n" printf "$2\n" @@ -16,19 +23,18 @@ travis_fold_end () { run_salt () { travis_fold_start "salt.install.$1" 'Installing and configuring Salt' .travis/install_salt.sh -F -c .travis -- "${TRAVIS_OS_NAME}" - .travis/setup_salt_roots.sh travis_fold_end "salt.install.$1" travis_fold_start "grains.items.$1" 'Printing Salt grains for debugging' - sudo salt-call --id="${SALT_NODE_ID}" grains.items + salt_call grains.items travis_fold_end "grains.items.$1" travis_fold_start "state.show_highstate.$1" 'Performing basic YAML and Jinja validation' - sudo salt-call --id="${SALT_NODE_ID}" --retcode-passthrough state.show_highstate + salt_call --retcode-passthrough state.show_highstate travis_fold_end "state.show_highstate.$1" printf 'Running the full Salt highstate\n' - sudo salt-call --id="${SALT_NODE_ID}" --retcode-passthrough --log-level=warning state.highstate + salt_call --retcode-passthrough --log-level=warning state.highstate } diff --git a/.travis/minion b/.travis/minion index 21a1b88e..43f2803c 100644 --- a/.travis/minion +++ b/.travis/minion @@ -1,9 +1,3 @@ +master: localhost # Used to test the salt-master state_output: terse state_tabular: True -file_client: local -file_roots: - base: - - /srv/salt/states -pillar_roots: - base: - - /srv/salt/pillars diff --git a/.travis/setup_salt_roots.sh b/.travis/setup_salt_roots.sh deleted file mode 100755 index dac0082b..00000000 --- a/.travis/setup_salt_roots.sh +++ /dev/null @@ -1,14 +0,0 @@ -#!/usr/bin/env bash - -set -o errexit -set -o nounset -set -o pipefail - -setup_salt_roots () { - sudo rm -rf /srv/salt - sudo mkdir -p /srv/salt - sudo cp -r . /srv/salt/states - sudo cp -r .travis/test_pillars /srv/salt/pillars -} - -setup_salt_roots "$@" diff --git a/Vagrantfile b/Vagrantfile index 007f5e28..ecf17eb7 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -7,6 +7,16 @@ def extract_id(env) id[0] end +def is_salt_master(id) + # The ideal way of doing this would be to ask Salt, which minions will + # execute the `salt/master.sls` state file during a highstate? However, + # it is not possible to do that from Vagrant outside the VM, and parsing + # the top.sls is also not a good idea because there are Salt intracacies + # like compound matching. Hence, simply hardcode this regex for now. + # This should be kept in sync with the top.sls file. + !id.match(/servo-master\d+/).nil? +end + Vagrant.require_version '>= 1.8.0' Vagrant.configure(2) do |config| @@ -16,10 +26,7 @@ Vagrant.configure(2) do |config| end dir = File.dirname(__FILE__) - minion_config_path = File.join(dir, '.travis', 'minion') - minion_config = YAML.load_file(minion_config_path) - state_root = minion_config['file_roots']['base'][0] - pillar_root = minion_config['pillar_roots']['base'][0] + test_pillars_path = File.join(dir, '.travis', 'test_pillars') YAML.load_file(File.join(dir, '.travis.yml'))['matrix']['include'].map do |node| node_config = case node['os'] @@ -46,15 +53,24 @@ Vagrant.configure(2) do |config| vbox.memory = 1024 vbox.linked_clone = true end - machine.vm.synced_folder dir, state_root - machine.vm.synced_folder File.join(dir, '.travis', 'test_pillars'), pillar_root + if is_salt_master(node[:id]) + # Salt master directories are hardcoded because we'd need to run Salt + # to resolve the configuration stored in the salt/map.jinja file. + # Make sure to keep these values in sync with that file. + machine.vm.synced_folder dir, '/srv/salt' + machine.vm.synced_folder test_pillars_path, '/srv/pillar' + end machine.vm.provision :salt do |salt| salt.bootstrap_script = File.join(dir, '.travis', 'install_salt.sh') salt.install_args = node[:os] # Pass OS type to bootstrap script salt.masterless = true - salt.minion_config = minion_config_path + salt.minion_config = File.join(dir, '.travis', 'minion') # hack to provide additional options to salt-call - salt.minion_id = node[:id] + ' --retcode-passthrough' + salt.minion_id = node[:id] + ' ' + ([ + '--file-root=/vagrant', + '--pillar-root=/vagrant/.travis/test_pillars', + '--retcode-passthrough' + ].join(' ')) salt.run_highstate = true salt.verbose = true salt.log_level = 'info' From 4603778203ce4b658948ad61f6add395ed4c367b Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Sun, 1 May 2016 01:18:57 -0400 Subject: [PATCH 3/5] Enable gitfs for Salt master Use gitfs backed by the saltfs repo (https://github.com/servo/saltfs/) as the main source for the Salt file tree. For local testing, additionally configure a local directory in /tmp as a Salt file tree root, and give it precedence over the gitfs tree. This allows temporarily cloning the saltfs repo into that directory and makes local changes to iterate on them. Add an ADMIN_README file with some ground rules about proper usage of the directory; this file should be set as immutable, but Salt does not yet have a module/state for this. --- Vagrantfile | 5 ++++- salt/files/master/ADMIN_README | 16 ++++++++++++++++ salt/map.jinja | 6 +++++- salt/master.sls | 29 ++++++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 salt/files/master/ADMIN_README diff --git a/Vagrantfile b/Vagrantfile index ecf17eb7..cadaebc7 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -57,7 +57,10 @@ Vagrant.configure(2) do |config| # Salt master directories are hardcoded because we'd need to run Salt # to resolve the configuration stored in the salt/map.jinja file. # Make sure to keep these values in sync with that file. - machine.vm.synced_folder dir, '/srv/salt' + # Note that because gitfs always reflects the master branch on GitHub, + # the states dir is synced to the override location instead to allow + # testing out changes locally. + machine.vm.synced_folder dir, '/tmp/salt-testing-root/saltfs' machine.vm.synced_folder test_pillars_path, '/srv/pillar' end machine.vm.provision :salt do |salt| diff --git a/salt/files/master/ADMIN_README b/salt/files/master/ADMIN_README new file mode 100644 index 00000000..db21c8a7 --- /dev/null +++ b/salt/files/master/ADMIN_README @@ -0,0 +1,16 @@ +Note: this directory is meant to be used to TEMPORARILY test out changes to our +Salt configurations on the live builders. To use it, clone the saltfs repo into +this folder, and Salt will read from it (/tmp/salt-testing-root/saltfs). +This folder takes precedence over gitfs (which reads file from the master +branch of the saltfs repository), so it is important to follow a few guidelines: + +- Avoid using this folder if possible: prefer to test changes locally using the +steps under "Making changes" at +https://github.com/servo/servo/wiki/Buildbot-administration + +- If you do need to use this folder, use it only as long as you need it, and +make sure to remove the cloned saltfs folder afterwards. Any files that are +left dangling will continue to mask changes in the real saltfs repository. + +- There is only one of these folders, so if it's already in use, wait until it +is empty again before testing your change. diff --git a/salt/map.jinja b/salt/map.jinja index 9329c112..b31d988c 100644 --- a/salt/map.jinja +++ b/salt/map.jinja @@ -1,12 +1,16 @@ +{% set salt_ = salt %} {% set salt = { 'version': '2015.5.8', 'master': salt['grains.filter_by']({ 'defaults': { 'config': { + 'fileserver_backend': [ 'roots', 'git' ], 'file_roots': { - 'base': [ '/srv/salt' ] + 'base': [ '/tmp/salt-testing-root/saltfs' ] }, + 'gitfs_remotes': [ 'https://github.com/servo/saltfs.git' ], + 'gitfs_env_whitelist': [ 'base' ], 'pillar_roots': { 'base': [ '/srv/pillar' ] } diff --git a/salt/master.sls b/salt/master.sls index 6a6ba370..4173b36b 100644 --- a/salt/master.sls +++ b/salt/master.sls @@ -1,14 +1,41 @@ -{% from tpldir ~ '/map.jinja' import salt %} +{% from tpldir ~ '/map.jinja' import salt, salt_ %} include: - .common +# Python modules for extra Salt master functionality +salt-master-dependencies: + pkg.installed: + - pkgs: + - python-git # GitPython for gitfs + +{% for base_rootfs_dir in salt.master.config.file_roots.base %} +{% set rootfs_parent_dir = salt_['file.dirname'](base_rootfs_dir) %} +{{ rootfs_parent_dir }}: + file.directory: + - user: root + - group: root + - mode: 755 + - require_in: + - service: salt-master + +{{ rootfs_parent_dir }}/ADMIN_README: + file.managed: + - user: root + - group: root + - mode: 644 + - source: salt://{{ tpldir }}/files/master/ADMIN_README + - require_in: + - service: salt-master +{% endfor %} + salt-master: pkg.installed: - name: {{ salt.master.pkg.name }} - version: {{ salt.master.pkg.version }} - require: - sls: salt.common + - pkg: salt-master-dependencies service.running: - enable: True - require: # Updates and upgrades must be handled manually From db1d45944bf9e1133e80646deecfab335daf4cdf Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Sun, 1 May 2016 23:43:38 -0400 Subject: [PATCH 4/5] Backport updated launchctl.py execution module Also update the buildbot slave state to use the now-working service state on OS X, and update the name of the service from 'buildslave' to 'buildbot-slave' for better distinguishability. Add a note to README.md about licensing of files taken and/or modified from the main Salt repo. --- README.md | 6 + _modules/launchctl.py | 357 ++++++++++++++++++ .../slave/files/net.buildbot.buildslave.plist | 2 +- buildbot/slave/init.sls | 18 +- 4 files changed, 370 insertions(+), 13 deletions(-) create mode 100644 _modules/launchctl.py diff --git a/README.md b/README.md index 07d3ac57..9b201143 100644 --- a/README.md +++ b/README.md @@ -23,3 +23,9 @@ This repository is distributed under the terms of both the MIT license and the Apache License (Version 2.0). See [LICENSE-APACHE](LICENSE-APACHE) and [LICENSE-MIT](LICENSE-MIT) for details. + +Note that some files in underscore-prefix directories (e.g. under `_modules`) +are copies (possibly with changes) of files from the +[main Salt repo](https://github.com/saltstack/salt); these files have headers +detailing the source of those files, any changes made, and the original license +notice associated with those files. diff --git a/_modules/launchctl.py b/_modules/launchctl.py new file mode 100644 index 00000000..7e5ae0d0 --- /dev/null +++ b/_modules/launchctl.py @@ -0,0 +1,357 @@ +# This module is a backported copy of the salt/states/pip_state.py module from +# Salt 2015.5.8 (at git revision ad4f204cb25c4856de59319a0c0692bfeb5243de), +# without any other changes applied. +# +# The original copyright and licensing notice for this module is reproduced +# below in the double-# comment block: +# +## Salt - Remote execution system +## +## Copyright 2014-2015 SaltStack Team +## +## Licensed under the Apache License, Version 2.0 (the "License"); +## you may not use this file except in compliance with the License. +## You may obtain a copy of the License at +## +## http://www.apache.org/licenses/LICENSE-2.0 +## +## Unless required by applicable law or agreed to in writing, software +## distributed under the License is distributed on an "AS IS" BASIS, +## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +## See the License for the specific language governing permissions and +## limitations under the License. + +# -*- coding: utf-8 -*- +''' +Module for the management of MacOS systems that use launchd/launchctl + +.. important:: + If you feel that Salt should be using this module to manage services on a + minion, and it is using a different module (or gives an error similar to + *'service.start' is not available*), see :ref:`here + `. + +:depends: - plistlib Python module +''' +from __future__ import absolute_import +from distutils.version import LooseVersion + +# Import python libs +import logging +import os +import plistlib +import re + +# Import salt libs +import salt.utils +import salt.utils.decorators as decorators +import salt.ext.six as six + +# Set up logging +log = logging.getLogger(__name__) + +# Define the module's virtual name +__virtualname__ = 'service' + +BEFORE_YOSEMITE = True + + +def __virtual__(): + ''' + Only work on MacOS + ''' + if not salt.utils.is_darwin(): + return (False, 'Failed to load the mac_service module:\n' + 'Only available on Mac OS X systems.') + + if not os.path.exists('/bin/launchctl'): + return (False, 'Failed to load the mac_service module:\n' + 'Required binary not found: "/bin/launchctl"') + + if LooseVersion(__grains__['osrelease']) >= LooseVersion('10.11'): + return (False, 'Failed to load the mac_service module:\n' + 'Not available on El Capitan, uses mac_service.py') + + if LooseVersion(__grains__['osrelease']) >= LooseVersion('10.10'): + global BEFORE_YOSEMITE + BEFORE_YOSEMITE = False + + return __virtualname__ + + +def _launchd_paths(): + ''' + Paths where launchd services can be found + ''' + return [ + '/Library/LaunchAgents', + '/Library/LaunchDaemons', + '/System/Library/LaunchAgents', + '/System/Library/LaunchDaemons', + ] + + +@decorators.memoize +def _available_services(): + ''' + Return a dictionary of all available services on the system + ''' + available_services = dict() + for launch_dir in _launchd_paths(): + for root, dirs, files in os.walk(launch_dir): + for filename in files: + file_path = os.path.join(root, filename) + # Follow symbolic links of files in _launchd_paths + true_path = os.path.realpath(file_path) + # ignore broken symlinks + if not os.path.exists(true_path): + continue + + try: + # This assumes most of the plist files + # will be already in XML format + with salt.utils.fopen(file_path): + plist = plistlib.readPlist(true_path) + + except Exception: + # If plistlib is unable to read the file we'll need to use + # the system provided plutil program to do the conversion + cmd = '/usr/bin/plutil -convert xml1 -o - -- "{0}"'.format( + true_path) + plist_xml = __salt__['cmd.run_all']( + cmd, python_shell=False)['stdout'] + if six.PY2: + plist = plistlib.readPlistFromString(plist_xml) + else: + plist = plistlib.readPlistFromBytes( + salt.utils.to_bytes(plist_xml)) + + available_services[plist.Label.lower()] = { + 'filename': filename, + 'file_path': true_path, + 'plist': plist, + } + + return available_services + + +def _service_by_name(name): + ''' + Return the service info for a service by label, filename or path + ''' + services = _available_services() + name = name.lower() + + if name in services: + # Match on label + return services[name] + + for service in six.itervalues(services): + if service['file_path'].lower() == name: + # Match on full path + return service + basename, ext = os.path.splitext(service['filename']) + if basename.lower() == name: + # Match on basename + return service + + return False + + +def get_all(): + ''' + Return all installed services + + CLI Example: + + .. code-block:: bash + + salt '*' service.get_all + ''' + cmd = 'launchctl list' + + service_lines = [ + line for line in __salt__['cmd.run'](cmd).splitlines() + if not line.startswith('PID') + ] + + service_labels_from_list = [ + line.split("\t")[2] for line in service_lines + ] + service_labels_from_services = list(_available_services().keys()) + + return sorted(set(service_labels_from_list + service_labels_from_services)) + + +def _get_launchctl_data(job_label, runas=None): + if BEFORE_YOSEMITE: + cmd = 'launchctl list -x {0}'.format(job_label) + else: + cmd = 'launchctl list {0}'.format(job_label) + + launchctl_data = __salt__['cmd.run_all'](cmd, + python_shell=False, + runas=runas) + + if launchctl_data['stderr']: + # The service is not loaded, further, it might not even exist + # in either case we didn't get XML to parse, so return an empty + # dict + return None + + return launchctl_data['stdout'] + + +def available(job_label): + ''' + Check that the given service is available. + + CLI Example: + + .. code-block:: bash + + salt '*' service.available com.openssh.sshd + ''' + return True if _service_by_name(job_label) else False + + +def missing(job_label): + ''' + The inverse of service.available + Check that the given service is not available. + + CLI Example: + + .. code-block:: bash + + salt '*' service.missing com.openssh.sshd + ''' + return False if _service_by_name(job_label) else True + + +def status(job_label, runas=None): + ''' + Return the status for a service, returns a bool whether the service is + running. + + CLI Example: + + .. code-block:: bash + + salt '*' service.status + ''' + service = _service_by_name(job_label) + + lookup_name = service['plist']['Label'] if service else job_label + launchctl_data = _get_launchctl_data(lookup_name, runas=runas) + + if launchctl_data: + if BEFORE_YOSEMITE: + if six.PY3: + return 'PID' in plistlib.loads(launchctl_data) + else: + return 'PID' in dict(plistlib.readPlistFromString(launchctl_data)) + else: + pattern = '"PID" = [0-9]+;' + return True if re.search(pattern, launchctl_data) else False + else: + return False + + +def stop(job_label, runas=None): + ''' + Stop the specified service + + CLI Example: + + .. code-block:: bash + + salt '*' service.stop + salt '*' service.stop org.ntp.ntpd + salt '*' service.stop /System/Library/LaunchDaemons/org.ntp.ntpd.plist + ''' + service = _service_by_name(job_label) + if service: + cmd = 'launchctl unload -w {0}'.format(service['file_path'], + runas=runas) + return not __salt__['cmd.retcode'](cmd, runas=runas, python_shell=False) + + return False + + +def start(job_label, runas=None): + ''' + Start the specified service + + CLI Example: + + .. code-block:: bash + + salt '*' service.start + salt '*' service.start org.ntp.ntpd + salt '*' service.start /System/Library/LaunchDaemons/org.ntp.ntpd.plist + ''' + service = _service_by_name(job_label) + if service: + cmd = 'launchctl load -w {0}'.format(service['file_path'], runas=runas) + return not __salt__['cmd.retcode'](cmd, runas=runas, python_shell=False) + + return False + + +def restart(job_label, runas=None): + ''' + Restart the named service + + CLI Example: + + .. code-block:: bash + + salt '*' service.restart + ''' + stop(job_label, runas=runas) + return start(job_label, runas=runas) + + +def enabled(job_label, runas=None): + ''' + Return True if the named service is enabled, false otherwise + + CLI Example: + + .. code-block:: bash + + salt '*' service.enabled + ''' + overrides_data = dict(plistlib.readPlist( + '/var/db/launchd.db/com.apple.launchd/overrides.plist' + )) + if overrides_data.get(job_label, False): + if overrides_data[job_label]['Disabled']: + return False + else: + return True + else: + return False + + +def disabled(job_label, runas=None): + ''' + Return True if the named service is disabled, false otherwise + + CLI Example: + + .. code-block:: bash + + salt '*' service.disabled + ''' + overrides_data = dict(plistlib.readPlist( + '/var/db/launchd.db/com.apple.launchd/overrides.plist' + )) + if overrides_data.get(job_label, False): + if overrides_data[job_label]['Disabled']: + return True + else: + return False + else: + return True diff --git a/buildbot/slave/files/net.buildbot.buildslave.plist b/buildbot/slave/files/net.buildbot.buildslave.plist index 56fd579a..234290ef 100644 --- a/buildbot/slave/files/net.buildbot.buildslave.plist +++ b/buildbot/slave/files/net.buildbot.buildslave.plist @@ -3,7 +3,7 @@ Label - buildslave + buildbot-slave EnvironmentVariables PATH diff --git a/buildbot/slave/init.sls b/buildbot/slave/init.sls index 03575f0c..6faf2e69 100644 --- a/buildbot/slave/init.sls +++ b/buildbot/slave/init.sls @@ -21,10 +21,6 @@ buildbot-slave-dependencies: - template: jinja - context: common: {{ common }} - {% if grains['kernel'] != 'Darwin' %} - - watch_in: - - service: buildbot-slave - {% endif %} {% if grains['kernel'] == 'Darwin' %} @@ -34,12 +30,8 @@ buildbot-slave-dependencies: - user: root - group: wheel - mode: 644 - -launchctl unload /Library/LaunchDaemons/net.buildbot.buildslave.plist: - cmd.run - -launchctl load -w /Library/LaunchDaemons/net.buildbot.buildslave.plist: - cmd.run + - watch_in: + - service: buildbot-slave {% else %} @@ -55,8 +47,10 @@ launchctl load -w /Library/LaunchDaemons/net.buildbot.buildslave.plist: - watch_in: - service: buildbot-slave +{% endif %} + buildbot-slave: service.running: - enable: True - -{% endif %} + - watch: + - file: {{ common.servo_home }}/buildbot/slave From a713928d133c7d3a6a1aae2176f3f6edbdc932b8 Mon Sep 17 00:00:00 2001 From: Aneesh Agrawal Date: Thu, 11 Aug 2016 12:25:55 -0400 Subject: [PATCH 5/5] Backport updated pip execution and state modules The backported versions support old and new pips more robustly without depending on pip internals. Recently (in version 8.1.2), pip changed their internals in a non-backwards compatible way, breaking the Salt pip modules. Salt used to depend on these internals, but has since been updated to no longer depend on them as they have no compatibility guarantees. We have been encountering issues where Salt is finding a new pip version, even though the installed pip is old (6.1.1). To work around this issue, backport updated modules that handle both old and new pips. --- _states/pip_state.py | 917 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 917 insertions(+) create mode 100644 _states/pip_state.py diff --git a/_states/pip_state.py b/_states/pip_state.py new file mode 100644 index 00000000..f2443f06 --- /dev/null +++ b/_states/pip_state.py @@ -0,0 +1,917 @@ +# This module is a copy of the salt/states/pip_state.py module from Salt +# version 2015.5.8 (git revision a26c10a811fef3c7deca0559f431713f69a83289), +# with the patches from the following two PRs backported: +# - https://github.com/saltstack/salt/pull/33180 +# - https://github.com/saltstack/salt/pull/33383 +# These patches allow the module to work with old and new pips after an +# change in the internals of pip in pip version 8.1.2. +# +# The original copyright and licensing notice for this module is reproduced +# below in the double-# comment block: +# +## Salt - Remote execution system +## +## Copyright 2014-2015 SaltStack Team +## +## Licensed under the Apache License, Version 2.0 (the "License"); +## you may not use this file except in compliance with the License. +## You may obtain a copy of the License at +## +## http://www.apache.org/licenses/LICENSE-2.0 +## +## Unless required by applicable law or agreed to in writing, software +## distributed under the License is distributed on an "AS IS" BASIS, +## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +## See the License for the specific language governing permissions and +## limitations under the License. + +# -*- coding: utf-8 -*- +''' +Installation of Python Packages Using pip +========================================= + +These states manage system installed python packages. Note that pip must be +installed for these states to be available, so pip states should include a +requisite to a pkg.installed state for the package which provides pip +(``python-pip`` in most cases). Example: + +.. code-block:: yaml + + python-pip: + pkg.installed + + virtualenvwrapper: + pip.installed: + - require: + - pkg: python-pip +''' + +# Import python libs +from __future__ import absolute_import +import re +import logging + +# Import salt libs +import salt.utils +from salt.version import SaltStackVersion as _SaltStackVersion +from salt.exceptions import CommandExecutionError, CommandNotFoundError + +# Import 3rd-party libs +import salt.ext.six as six +try: + import pip + HAS_PIP = True +except ImportError: + HAS_PIP = False + +if HAS_PIP is True: + try: + import pip.req + except ImportError: + HAS_PIP = False + # Remove references to the loaded pip module above so reloading works + import sys + del pip + if 'pip' in sys.modules: + del sys.modules['pip'] + +logger = logging.getLogger(__name__) + +# Define the module's virtual name +__virtualname__ = 'pip' + + +def __virtual__(): + ''' + Only load if the pip module is available in __salt__ + ''' + if 'pip.list' in __salt__: + return __virtualname__ + return False + + +def _find_key(prefix, pip_list): + ''' + Does a case-insensitive match in the pip_list for the desired package. + ''' + try: + match = next( + iter(x for x in pip_list if x.lower() == prefix.lower()) + ) + except StopIteration: + return None + else: + return match + + +def _fulfills_version_spec(version, version_spec): + ''' + Check version number against version specification info and return a + boolean value based on whether or not the version number meets the + specified version. + ''' + for oper, spec in version_spec: + if oper is None: + continue + if not salt.utils.compare_versions(ver1=version, oper=oper, ver2=spec): + return False + return True + + +def _check_pkg_version_format(pkg): + ''' + Takes a package name and version specification (if any) and checks it using + the pip library. + ''' + + ret = {'result': False, 'comment': None, + 'prefix': None, 'version_spec': None} + + if not HAS_PIP: + ret['comment'] = ( + 'An importable pip module is required but could not be found on ' + 'your system. This usually means that the system''s pip package ' + 'is not installed properly.' + ) + + return ret + + from_vcs = False + try: + # Get the requirement object from the pip library + try: + # With pip < 1.2, the __version__ attribute does not exist and + # vcs+URL urls are not properly parsed. + # The next line is meant to trigger an AttributeError and + # handle lower pip versions + logger.debug( + 'Installed pip version: {0}'.format(pip.__version__) + ) + install_req = pip.req.InstallRequirement.from_line(pkg) + except AttributeError: + logger.debug('Installed pip version is lower than 1.2') + supported_vcs = ('git', 'svn', 'hg', 'bzr') + if pkg.startswith(supported_vcs): + for vcs in supported_vcs: + if pkg.startswith(vcs): + from_vcs = True + install_req = pip.req.InstallRequirement.from_line( + pkg.split('{0}+'.format(vcs))[-1] + ) + break + else: + install_req = pip.req.InstallRequirement.from_line(pkg) + except ValueError as exc: + ret['result'] = False + if not from_vcs and '=' in pkg and '==' not in pkg: + ret['comment'] = ( + 'Invalid version specification in package {0}. \'=\' is ' + 'not supported, use \'==\' instead.'.format(pkg) + ) + return ret + ret['comment'] = ( + 'pip raised an exception while parsing {0!r}: {1}'.format( + pkg, exc + ) + ) + return ret + + if install_req.req is None: + # This is most likely an url and there's no way to know what will + # be installed before actually installing it. + ret['result'] = True + ret['prefix'] = '' + ret['version_spec'] = [] + else: + ret['result'] = True + try: + ret['prefix'] = install_req.req.project_name + ret['version_spec'] = install_req.req.specs + except Exception: + ret['prefix'] = re.sub('[^A-Za-z0-9.]+', '-', install_req.name) + if hasattr(install_req, "specifier"): + specifier = install_req.specifier + else: + specifier = install_req.req.specifier + ret['version_spec'] = [(spec.operator, spec.version) for spec in specifier] + + return ret + + +def _check_if_installed(prefix, state_pkg_name, version_spec, + ignore_installed, force_reinstall, + upgrade, user, cwd, bin_env): + + # result: None means the command failed to run + # result: True means the package is installed + # result: False means the package is not installed + ret = {'result': False, 'comment': None} + + # Check if the requested packated is already installed. + try: + pip_list = __salt__['pip.list'](prefix, bin_env=bin_env, + user=user, cwd=cwd) + prefix_realname = _find_key(prefix, pip_list) + except (CommandNotFoundError, CommandExecutionError) as err: + ret['result'] = None + ret['comment'] = 'Error installing {0!r}: {1}'.format(state_pkg_name, + err) + return ret + + # If the package was already installed, check + # the ignore_installed and force_reinstall flags + if ignore_installed is False and prefix_realname is not None: + if force_reinstall is False and not upgrade: + # Check desired version (if any) against currently-installed + if ( + any(version_spec) and + _fulfills_version_spec(pip_list[prefix_realname], + version_spec) + ) or (not any(version_spec)): + ret['result'] = True + ret['comment'] = ('Python package {0} was already ' + 'installed'.format(state_pkg_name)) + return ret + + return ret + + +def installed(name, + pkgs=None, + pip_bin=None, + requirements=None, + env=None, + bin_env=None, + use_wheel=False, + no_use_wheel=False, + log=None, + proxy=None, + timeout=None, + repo=None, + editable=None, + find_links=None, + index_url=None, + extra_index_url=None, + no_index=False, + mirrors=None, + build=None, + target=None, + download=None, + download_cache=None, + source=None, + upgrade=False, + force_reinstall=False, + ignore_installed=False, + exists_action=None, + no_deps=False, + no_install=False, + no_download=False, + install_options=None, + global_options=None, + user=None, + no_chown=False, + cwd=None, + activate=False, + pre_releases=False, + cert=None, + allow_all_external=False, + allow_external=None, + allow_unverified=None, + process_dependency_links=False, + env_vars=None, + use_vt=False): + ''' + Make sure the package is installed + + name + The name of the python package to install. You can also specify version + numbers here using the standard operators ``==, >=, <=``. If + ``requirements`` is given, this parameter will be ignored. + + Example: + + .. code-block:: yaml + + django: + pip.installed: + - name: django >= 1.6, <= 1.7 + - require: + - pkg: python-pip + + This will install the latest Django version greater than 1.6 but less + than 1.7. + + requirements + Path to a pip requirements file. If the path begins with salt:// + the file will be transferred from the master file server. + + user + The user under which to run pip + + use_wheel : False + Prefer wheel archives (requires pip>=1.4) + + no_use_wheel : False + Force to not use wheel archives (requires pip>=1.4) + + log + Log file where a complete (maximum verbosity) record will be kept + + proxy + Specify a proxy in the form + user:passwd@proxy.server:port. Note that the + user:password@ is optional and required only if you + are behind an authenticated proxy. If you provide + user@proxy.server:port then you will be prompted for a + password. + + timeout + Set the socket timeout (default 15 seconds) + + editable + install something editable (i.e. + git+https://github.com/worldcompany/djangoembed.git#egg=djangoembed) + + find_links + URL to look for packages at + + index_url + Base URL of Python Package Index + + extra_index_url + Extra URLs of package indexes to use in addition to ``index_url`` + + no_index + Ignore package index + + mirrors + Specific mirror URL(s) to query (automatically adds --use-mirrors) + + build + Unpack packages into ``build`` dir + + target + Install packages into ``target`` dir + + download + Download packages into ``download`` instead of installing them + + download_cache + Cache downloaded packages in ``download_cache`` dir + + source + Check out ``editable`` packages into ``source`` dir + + upgrade + Upgrade all packages to the newest available version + + force_reinstall + When upgrading, reinstall all packages even if they are already + up-to-date. + + ignore_installed + Ignore the installed packages (reinstalling instead) + + exists_action + Default action when a path already exists: (s)witch, (i)gnore, (w)ipe, + (b)ackup + + no_deps + Ignore package dependencies + + no_install + Download and unpack all packages, but don't actually install them + + no_chown + When user is given, do not attempt to copy and chown + a requirements file + + cwd + Current working directory to run pip from + + activate + Activates the virtual environment, if given via bin_env, + before running install. + + .. deprecated:: 2014.7.2 + If `bin_env` is given, pip will already be sourced from that + virualenv, making `activate` effectively a noop. + + pre_releases + Include pre-releases in the available versions + + cert + Provide a path to an alternate CA bundle + + allow_all_external + Allow the installation of all externally hosted files + + allow_external + Allow the installation of externally hosted files (comma separated list) + + allow_unverified + Allow the installation of insecure and unverifiable files (comma separated list) + + process_dependency_links + Enable the processing of dependency links + + bin_env : None + Absolute path to a virtual environment directory or absolute path to + a pip executable. The example below assumes a virtual environment + has been created at ``/foo/.virtualenvs/bar``. + + env_vars + Add or modify environment variables. Useful for tweaking build steps, + such as specifying INCLUDE or LIBRARY paths in Makefiles, build scripts or + compiler calls. This must be in the form of a dictionary or a mapping. + + Example: + + .. code-block:: yaml + + django: + pip.installed: + - name: django_app + - env_vars: + CUSTOM_PATH: /opt/django_app + VERBOSE: True + + use_vt + Use VT terminal emulation (see ouptut while installing) + + Example: + + .. code-block:: yaml + + django: + pip.installed: + - name: django >= 1.6, <= 1.7 + - bin_env: /foo/.virtualenvs/bar + - require: + - pkg: python-pip + + Or + + Example: + + .. code-block:: yaml + + django: + pip.installed: + - name: django >= 1.6, <= 1.7 + - bin_env: /foo/.virtualenvs/bar/bin/pip + - require: + - pkg: python-pip + + .. admonition:: Attention + + The following arguments are deprecated, do not use. + + pip_bin : None + Deprecated, use ``bin_env`` + + env : None + Deprecated, use ``bin_env`` + + .. versionchanged:: 0.17.0 + ``use_wheel`` option added. + + install_options + + Extra arguments to be supplied to the setup.py install command. + If you are using an option with a directory path, be sure to use + absolute path. + + Example: + + .. code-block:: yaml + + django: + pip.installed: + - name: django + - install_options: + - --prefix=/blah + - require: + - pkg: python-pip + + global_options + Extra global options to be supplied to the setup.py call before the + install command. + + .. versionadded:: 2014.1.3 + + .. admonition:: Attention + + As of Salt 0.17.0 the pip state **needs** an importable pip module. + This usually means having the system's pip package installed or running + Salt from an active `virtualenv`_. + + The reason for this requirement is because ``pip`` already does a + pretty good job parsing its own requirements. It makes no sense for + Salt to do ``pip`` requirements parsing and validation before passing + them to the ``pip`` library. It's functionality duplication and it's + more error prone. + + .. _`virtualenv`: http://www.virtualenv.org/en/latest/ + ''' + + if pip_bin and not bin_env: + bin_env = pip_bin + elif env and not bin_env: + bin_env = env + + # If pkgs is present, ignore name + if pkgs: + if not isinstance(pkgs, list): + return {'name': name, + 'result': False, + 'changes': {}, + 'comment': 'pkgs argument must be formatted as a list'} + else: + pkgs = [name] + + # Assumption: If `pkg` is not an `string`, it's a `collections.OrderedDict` + # prepro = lambda pkg: pkg if type(pkg) == str else \ + # ' '.join((pkg.items()[0][0], pkg.items()[0][1].replace(',', ';'))) + # pkgs = ','.join([prepro(pkg) for pkg in pkgs]) + prepro = lambda pkg: pkg if isinstance(pkg, str) else \ + ' '.join((six.iteritems(pkg)[0][0], six.iteritems(pkg)[0][1])) + pkgs = [prepro(pkg) for pkg in pkgs] + + ret = {'name': ';'.join(pkgs), 'result': None, + 'comment': '', 'changes': {}} + + # Check that the pip binary supports the 'use_wheel' option + if use_wheel: + min_version = '1.4' + cur_version = __salt__['pip.version'](bin_env) + if not salt.utils.compare_versions(ver1=cur_version, oper='>=', + ver2=min_version): + ret['result'] = False + ret['comment'] = ('The \'use_wheel\' option is only supported in ' + 'pip {0} and newer. The version of pip detected ' + 'was {1}.').format(min_version, cur_version) + return ret + + # Check that the pip binary supports the 'no_use_wheel' option + if no_use_wheel: + min_version = '1.4' + cur_version = __salt__['pip.version'](bin_env) + if not salt.utils.compare_versions(ver1=cur_version, oper='>=', + ver2=min_version): + ret['result'] = False + ret['comment'] = ('The \'no_use_wheel\' option is only supported in ' + 'pip {0} and newer. The version of pip detected ' + 'was {1}.').format(min_version, cur_version) + return ret + + # Deprecation warning for the repo option + if repo is not None: + msg = ('The \'repo\' argument to pip.installed is deprecated and will ' + 'be removed in Salt {version}. Please use \'name\' instead. ' + 'The current value for name, {0!r} will be replaced by the ' + 'value of repo, {1!r}'.format( + name, + repo, + version=_SaltStackVersion.from_name('Lithium').formatted_version + )) + salt.utils.warn_until('Lithium', msg) + ret.setdefault('warnings', []).append(msg) + name = repo + + # Get the packages parsed name and version from the pip library. + # This only is done when there is no requirements parameter. + pkgs_details = [] + if pkgs and not requirements: + comments = [] + for pkg in iter(pkgs): + out = _check_pkg_version_format(pkg) + if out['result'] is False: + ret['result'] = False + comments.append(out['comment']) + elif out['result'] is True: + pkgs_details.append((out['prefix'], pkg, out['version_spec'])) + + if ret['result'] is False: + ret['comment'] = '\n'.join(comments) + return ret + + # If a requirements file is specified, only install the contents of the + # requirements file. Similarly, using the --editable flag with pip should + # also ignore the "name" and "pkgs" parameters. + target_pkgs = [] + already_installed_comments = [] + if requirements or editable: + name = '' + comments = [] + # Append comments if this is a dry run. + if __opts__['test']: + ret['result'] = None + if requirements: + # TODO: Check requirements file against currently-installed + # packages to provide more accurate state output. + comments.append('Requirements file {0!r} will be ' + 'processed.'.format(requirements)) + if editable: + comments.append( + 'Package will be installed in editable mode (i.e. ' + 'setuptools "develop mode") from {0}.'.format(editable) + ) + ret['comment'] = ' '.join(comments) + return ret + + # No requirements case. + # Check pre-existence of the requested packages. + else: + for prefix, state_pkg_name, version_spec in pkgs_details: + + if prefix: + state_pkg_name = state_pkg_name + version_spec = version_spec + out = _check_if_installed(prefix, state_pkg_name, version_spec, + ignore_installed, force_reinstall, + upgrade, user, cwd, bin_env) + else: + out = {'result': False, 'comment': None} + + result = out['result'] + + # The package is not present. Add it to the pkgs to install. + if result is False: + # Replace commas (used for version ranges) with semicolons + # (which are not supported) in name so it does not treat + # them as multiple packages. + target_pkgs.append((prefix, state_pkg_name.replace(',', ';'))) + + # Append comments if this is a dry run. + if __opts__['test']: + msg = 'Python package {0} is set to be installed' + ret['result'] = None + ret['comment'] = msg.format(state_pkg_name) + return ret + + # The package is already present and will not be reinstalled. + elif result is True: + # Append comment stating its presence + already_installed_comments.append(out['comment']) + + # The command pip.list failed. Abort. + elif result is None: + ret['result'] = None + ret['comment'] = out['comment'] + return ret + + # Construct the string that will get passed to the install call + pkgs_str = ','.join([state_name for _, state_name in target_pkgs]) + + # Call to install the package. Actual installation takes place here + pip_install_call = __salt__['pip.install']( + pkgs='{0}'.format(pkgs_str) if pkgs_str else '', + requirements=requirements, + bin_env=bin_env, + use_wheel=use_wheel, + no_use_wheel=no_use_wheel, + log=log, + proxy=proxy, + timeout=timeout, + editable=editable, + find_links=find_links, + index_url=index_url, + extra_index_url=extra_index_url, + no_index=no_index, + mirrors=mirrors, + build=build, + target=target, + download=download, + download_cache=download_cache, + source=source, + upgrade=upgrade, + force_reinstall=force_reinstall, + ignore_installed=ignore_installed, + exists_action=exists_action, + no_deps=no_deps, + no_install=no_install, + no_download=no_download, + install_options=install_options, + global_options=global_options, + user=user, + no_chown=no_chown, + cwd=cwd, + activate=activate, + pre_releases=pre_releases, + cert=cert, + allow_all_external=allow_all_external, + allow_external=allow_external, + allow_unverified=allow_unverified, + process_dependency_links=process_dependency_links, + saltenv=__env__, + env_vars=env_vars, + use_vt=use_vt + ) + + # Check the retcode for success, but don't fail if using pip1 and the package is + # already present. Pip1 returns a retcode of 1 (instead of 0 for pip2) if you run + # "pip install" without any arguments. See issue #21845. + if pip_install_call and \ + (pip_install_call.get('retcode', 1) == 0 or pip_install_call.get('stdout', '').startswith( + 'You must give at least one requirement to install')): + ret['result'] = True + + if requirements or editable: + comments = [] + if requirements: + for line in pip_install_call.get('stdout', '').split('\n'): + if not line.startswith('Requirement already satisfied') \ + and line != 'Cleaning up...': + ret['changes']['requirements'] = True + if ret['changes'].get('requirements'): + comments.append('Successfully processed requirements file ' + '{0}.'.format(requirements)) + else: + comments.append('Requirements were already installed.') + + if editable: + comments.append('Package successfully installed from VCS ' + 'checkout {0}.'.format(editable)) + ret['changes']['editable'] = True + ret['comment'] = ' '.join(comments) + else: + + # Check that the packages set to be installed were installed. + # Create comments reporting success and failures + pkg_404_comms = [] + + for prefix, state_name in target_pkgs: + + # Case for packages that are not an URL + if prefix: + pipsearch = __salt__['pip.list'](prefix, bin_env, + user=user, cwd=cwd) + + # If we didnt find the package in the system after + # installing it report it + if not pipsearch: + msg = ( + 'There was no error installing package \'{0}\' ' + 'although it does not show when calling ' + '\'pip.freeze\'.'.format(pkg) + ) + pkg_404_comms.append(msg) + else: + pkg_name = _find_key(prefix, pipsearch) + ver = pipsearch[pkg_name] + ret['changes']['{0}=={1}'.format(pkg_name, + ver)] = 'Installed' + # Case for packages that are an URL + else: + ret['changes']['{0}==???'.format(state_name)] = 'Installed' + + # Set comments + aicomms = '\n'.join(already_installed_comments) + succ_comm = 'All packages were successfully installed'\ + if not pkg_404_comms else '\n'.join(pkg_404_comms) + ret['comment'] = aicomms + ('\n' if aicomms else '') + succ_comm + + return ret + + elif pip_install_call: + ret['result'] = False + if 'stdout' in pip_install_call: + error = 'Error: {0} {1}'.format(pip_install_call['stdout'], + pip_install_call['stderr']) + else: + error = 'Error: {0}'.format(pip_install_call['comment']) + + if requirements or editable: + comments = [] + if requirements: + comments.append('Unable to process requirements file ' + '{0}.'.format(requirements)) + if editable: + comments.append('Unable to install from VCS checkout' + '{0}.'.format(editable)) + comments.append(error) + ret['comment'] = ' '.join(comments) + else: + pkgs_str = ', '.join([state_name for _, state_name in target_pkgs]) + aicomms = '\n'.join(already_installed_comments) + error_comm = ('Failed to install packages: {0}. ' + '{1}'.format(pkgs_str, error)) + ret['comment'] = aicomms + ('\n' if aicomms else '') + error_comm + else: + ret['result'] = False + ret['comment'] = 'Could not install package' + + return ret + + +def removed(name, + requirements=None, + bin_env=None, + log=None, + proxy=None, + timeout=None, + user=None, + cwd=None, + use_vt=False): + ''' + Make sure that a package is not installed. + + name + The name of the package to uninstall + user + The user under which to run pip + bin_env : None + the pip executable or virtualenenv to use + use_vt + Use VT terminal emulation (see ouptut while installing) + ''' + ret = {'name': name, 'result': None, 'comment': '', 'changes': {}} + + try: + pip_list = __salt__['pip.list'](bin_env=bin_env, user=user, cwd=cwd) + except (CommandExecutionError, CommandNotFoundError) as err: + ret['result'] = False + ret['comment'] = 'Error uninstalling \'{0}\': {1}'.format(name, err) + return ret + + if name not in pip_list: + ret['result'] = True + ret['comment'] = 'Package is not installed.' + return ret + + if __opts__['test']: + ret['result'] = None + ret['comment'] = 'Package {0} is set to be removed'.format(name) + return ret + + if __salt__['pip.uninstall'](pkgs=name, + requirements=requirements, + bin_env=bin_env, + log=log, + proxy=proxy, + timeout=timeout, + user=user, + cwd=cwd, + use_vt=use_vt): + ret['result'] = True + ret['changes'][name] = 'Removed' + ret['comment'] = 'Package was successfully removed.' + else: + ret['result'] = False + ret['comment'] = 'Could not remove package.' + return ret + + +def uptodate(name, + bin_env=None, + user=None, + cwd=None, + use_vt=False): + ''' + .. versionadded:: 2015.5.0 + + Verify that the system is completely up to date. + + name + The name has no functional value and is only used as a tracking + reference + user + The user under which to run pip + bin_env + the pip executable or virtualenenv to use + use_vt + Use VT terminal emulation (see ouptut while installing) + ''' + ret = {'name': name, + 'changes': {}, + 'result': False, + 'comment': 'Failed to update.'} + + try: + packages = __salt__['pip.list_upgrades'](bin_env=bin_env, user=user, cwd=cwd) + except Exception as e: + ret['comment'] = str(e) + return ret + + if not packages: + ret['comment'] = 'System is already up-to-date.' + ret['result'] = True + return ret + elif __opts__['test']: + ret['comment'] = 'System update will be performed' + ret['result'] = None + return ret + + updated = __salt__['pip.upgrade'](bin_env=bin_env, user=user, cwd=cwd, use_vt=use_vt) + + if updated.get('result') is False: + ret.update(updated) + elif updated: + ret['changes'] = updated + ret['comment'] = 'Upgrade successful.' + ret['result'] = True + else: + ret['comment'] = 'Upgrade failed.' + + return ret