From 07f863101f56f54ca77a3060bfa2d1c6f9eef70a Mon Sep 17 00:00:00 2001 From: zer0def Date: Fri, 8 Jun 2018 13:17:20 +0200 Subject: [PATCH 01/38] Fixed partition names with spaces effectively containing only the first word. --- salt/modules/parted.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/parted.py b/salt/modules/parted.py index f568c3d0011..e34d48ca1fb 100644 --- a/salt/modules/parted.py +++ b/salt/modules/parted.py @@ -528,7 +528,7 @@ def name(device, partition, name): 'Invalid characters passed to partition.name' ) - cmd = 'parted -m -s {0} name {1} {2}'.format(device, partition, name) + cmd = '''parted -m -s {0} name {1} "'{2}'"'''.format(device, partition, name) out = __salt__['cmd.run'](cmd).splitlines() return out From 566a4ea66aff40dae8d1c450500b1eb252df0088 Mon Sep 17 00:00:00 2001 From: Daniel A Wozniak Date: Mon, 13 Aug 2018 23:21:28 +0000 Subject: [PATCH 02/38] Install the launcher so we can execute py files --- pkg/windows/build_env_3.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/windows/build_env_3.ps1 b/pkg/windows/build_env_3.ps1 index b9b78cf9e15..623ee0f6c6c 100644 --- a/pkg/windows/build_env_3.ps1 +++ b/pkg/windows/build_env_3.ps1 @@ -175,7 +175,7 @@ If (Test-Path "$($ini['Settings']['Python3Dir'])\python.exe") { DownloadFileWithProgress $url $file Write-Output " - $script_name :: Installing $($ini[$bitPrograms]['Python3']) . . ." - $p = Start-Process $file -ArgumentList "/Quiet InstallAllUsers=1 TargetDir=`"$($ini['Settings']['Python3Dir'])`" Include_doc=0 Include_tcltk=0 Include_test=0 Include_launcher=0 PrependPath=1 Shortcuts=0" -Wait -NoNewWindow -PassThru + $p = Start-Process $file -ArgumentList "/Quiet InstallAllUsers=1 TargetDir=`"$($ini['Settings']['Python3Dir'])`" Include_doc=0 Include_tcltk=0 Include_test=0 Include_launcher=1 PrependPath=1 Shortcuts=0" -Wait -NoNewWindow -PassThru } #------------------------------------------------------------------------------ From c6b781eea5c671e26ef607088d111963a8a7de96 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Tue, 14 Aug 2018 18:37:53 +0000 Subject: [PATCH 03/38] Multiple fixes for integration.states.test_file - Fix wart in PR #49087 file://c:\foo\bar vs c:\foo\bar local paths - Finalize fix for test_issue_8343_accumulated_require_in - Fix wart in PR #49088 Ignore proper directory test --- salt/modules/file.py | 8 ++++++-- salt/states/file.py | 4 +++- tests/integration/states/test_file.py | 8 ++++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/salt/modules/file.py b/salt/modules/file.py index 71fd98ac824..acbfbd4aca7 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -3893,8 +3893,12 @@ def get_managed( parsed_path = os.path.join( urlparsed_source.netloc, urlparsed_source.path).rstrip(os.sep) unix_local_source = parsed_scheme in ('file', '') - - if unix_local_source: + if parsed_scheme == '': + parsed_path = sfn = source + if not os.path.exists(sfn): + msg = 'Local file source {0} does not exist'.format(sfn) + return '', {}, msg + elif parsed_scheme == 'file': sfn = parsed_path if not os.path.exists(sfn): msg = 'Local file source {0} does not exist'.format(sfn) diff --git a/salt/states/file.py b/salt/states/file.py index 70d808f6c05..2f4f16d1ea8 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -2377,7 +2377,9 @@ def managed(name, 'contents_grains is not a string or list of strings, and ' 'is not binary data. SLS is likely malformed.' ) - contents = os.linesep.join(validated_contents) + contents = os.linesep.join( + [line.rstrip('\n').rstrip('\r') for line in validated_contents] + ) if contents_newline and not contents.endswith(os.linesep): contents += os.linesep if template: diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index 3cbae4e0142..60acaeea2d6 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -844,13 +844,9 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): finally: shutil.rmtree(name, ignore_errors=True) - @skipIf(salt.utils.is_windows(), 'Skip on windows') def test_directory_clean_exclude(self): ''' file.directory with clean=True and exclude_pat set - - Skipped on windows because clean and exclude_pat not supported by - salt.sates.file._check_directory_win ''' name = os.path.join(TMP, 'directory_clean_dir') if not os.path.isdir(name): @@ -889,9 +885,13 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): finally: shutil.rmtree(name, ignore_errors=True) + @skipIf(salt.utils.is_windows(), 'Skip on windows') def test_test_directory_clean_exclude(self): ''' file.directory with test=True, clean=True and exclude_pat set + + Skipped on windows because clean and exclude_pat not supported by + salt.sates.file._check_directory_win ''' name = os.path.join(TMP, 'directory_clean_dir') if not os.path.isdir(name): From de40dfb14268feca203e7a307a052aab43127cd4 Mon Sep 17 00:00:00 2001 From: Ch3LL Date: Wed, 15 Aug 2018 15:30:20 -0400 Subject: [PATCH 04/38] [2017.7] Update bootstrap script to latest release (2018.08.15) --- salt/cloud/deploy/bootstrap-salt.sh | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/salt/cloud/deploy/bootstrap-salt.sh b/salt/cloud/deploy/bootstrap-salt.sh index 9f7cf21c9be..c3af7920713 100755 --- a/salt/cloud/deploy/bootstrap-salt.sh +++ b/salt/cloud/deploy/bootstrap-salt.sh @@ -18,7 +18,7 @@ #====================================================================================================================== set -o nounset # Treat unset variables as an error -__ScriptVersion="2018.08.13" +__ScriptVersion="2018.08.15" __ScriptName="bootstrap-salt.sh" __ScriptFullName="$0" @@ -3140,7 +3140,7 @@ install_debian_8_git_deps() { __PACKAGES="libzmq3 libzmq3-dev lsb-release python-apt python-crypto python-jinja2" __PACKAGES="${__PACKAGES} python-m2crypto python-msgpack python-requests python-systemd" - __PACKAGES="${__PACKAGES} python-yaml python-zmq" + __PACKAGES="${__PACKAGES} python-yaml python-zmq python-concurrent.futures" if [ "$_INSTALL_CLOUD" -eq $BS_TRUE ]; then # Install python-libcloud if asked to @@ -3211,7 +3211,7 @@ install_debian_9_git_deps() { PY_PKG_VER="" # These packages are PY2-ONLY - __PACKAGES="${__PACKAGES} python-backports-abc python-m2crypto" + __PACKAGES="${__PACKAGES} python-backports-abc python-m2crypto python-concurrent.futures" fi __PACKAGES="${__PACKAGES} python${PY_PKG_VER}-apt python${PY_PKG_VER}-crypto python${PY_PKG_VER}-jinja2" @@ -3429,7 +3429,7 @@ install_fedora_deps() { fi fi - __PACKAGES="${__PACKAGES} dnf-utils libyaml python${PY_PKG_VER}-crypto python${PY_PKG_VER}-jinja2" + __PACKAGES="${__PACKAGES} procps-ng dnf-utils libyaml python${PY_PKG_VER}-crypto python${PY_PKG_VER}-jinja2" __PACKAGES="${__PACKAGES} python${PY_PKG_VER}-msgpack python${PY_PKG_VER}-requests python${PY_PKG_VER}-zmq" # shellcheck disable=SC2086 @@ -3800,7 +3800,7 @@ install_centos_git_deps() { fi fi - __PACKAGES="python${PY_PKG_VER}-crypto python${PY_PKG_VER}-jinja2" + __PACKAGES="${__PACKAGES} python${PY_PKG_VER}-crypto python${PY_PKG_VER}-jinja2" __PACKAGES="${__PACKAGES} python${PY_PKG_VER}-msgpack python${PY_PKG_VER}-requests" __PACKAGES="${__PACKAGES} python${PY_PKG_VER}-tornado python${PY_PKG_VER}-zmq" @@ -3815,7 +3815,7 @@ install_centos_git_deps() { if [ "${_PY_EXE}" != "" ] && [ "$_PIP_ALLOWED" -eq "$BS_TRUE" ]; then # If "-x" is defined, install dependencies with pip based on the Python version given. - _PIP_PACKAGES="m2crypto jinja2 msgpack-python pycrypto PyYAML tornado<5.0 zmq" + _PIP_PACKAGES="m2crypto jinja2 msgpack-python pycrypto PyYAML tornado<5.0 zmq futures>=2.0" # install swig and openssl on cent6 if [ "$DISTRO_MAJOR_VERSION" -eq 6 ]; then @@ -4608,6 +4608,7 @@ _eof # which is already installed __PACKAGES="m2crypto ${pkg_append}-crypto ${pkg_append}-jinja2 ${pkg_append}-PyYAML" __PACKAGES="${__PACKAGES} ${pkg_append}-msgpack ${pkg_append}-requests ${pkg_append}-zmq" + __PACKAGES="${__PACKAGES} ${pkg_append}-futures" # shellcheck disable=SC2086 __yum_install_noinput ${__PACKAGES} || return 1 @@ -4627,6 +4628,9 @@ install_amazon_linux_ami_git_deps() { PIP_EXE='pip' if __check_command_exists python2.7; then if ! __check_command_exists pip2.7; then + if ! __check_command_exists easy_install-2.7; then + __yum_install_noinput python27-setuptools + fi /usr/bin/easy_install-2.7 pip || return 1 fi PIP_EXE='/usr/local/bin/pip2.7' @@ -4646,7 +4650,7 @@ install_amazon_linux_ami_git_deps() { if [ "$_INSTALL_CLOUD" -eq $BS_TRUE ]; then __check_pip_allowed "You need to allow pip based installations (-P) in order to install apache-libcloud" - __PACKAGES="${__PACKAGES} python-pip" + __PACKAGES="${__PACKAGES} python27-pip" __PIP_PACKAGES="${__PIP_PACKAGES} apache-libcloud>=$_LIBCLOUD_MIN_VERSION" fi @@ -4795,7 +4799,7 @@ install_arch_linux_stable() { pacman -S --noconfirm --needed bash || return 1 pacman -Su --noconfirm || return 1 # We can now resume regular salt update - pacman -Syu --noconfirm salt || return 1 + pacman -Syu --noconfirm salt python2-futures || return 1 return 0 } @@ -5649,7 +5653,7 @@ install_opensuse_git_deps() { __git_clone_and_checkout || return 1 - __PACKAGES="libzmq5 python-Jinja2 python-m2crypto python-msgpack-python python-pycrypto python-pyzmq python-xml" + __PACKAGES="libzmq5 python-Jinja2 python-m2crypto python-msgpack-python python-pycrypto python-pyzmq python-xml python-futures" if [ -f "${_SALT_GIT_CHECKOUT_DIR}/requirements/base.txt" ]; then # We're on the develop branch, install whichever tornado is on the requirements file From d2e73cc988241dedcc52a694866a7dd4d1a1790d Mon Sep 17 00:00:00 2001 From: Ch3LL Date: Wed, 15 Aug 2018 17:27:02 -0400 Subject: [PATCH 05/38] Remove -Z script_arg for cloud tests --- tests/integration/files/conf/cloud.profiles.d/azure.conf | 2 +- .../integration/files/conf/cloud.profiles.d/digital_ocean.conf | 2 +- tests/integration/files/conf/cloud.profiles.d/ec2.conf | 2 +- tests/integration/files/conf/cloud.profiles.d/gogrid.conf | 2 +- tests/integration/files/conf/cloud.profiles.d/joyent.conf | 2 +- tests/integration/files/conf/cloud.profiles.d/linode.conf | 2 +- tests/integration/files/conf/cloud.profiles.d/rackspace.conf | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/integration/files/conf/cloud.profiles.d/azure.conf b/tests/integration/files/conf/cloud.profiles.d/azure.conf index d139bfe7a77..fe97e00b085 100644 --- a/tests/integration/files/conf/cloud.profiles.d/azure.conf +++ b/tests/integration/files/conf/cloud.profiles.d/azure.conf @@ -7,4 +7,4 @@ azure-test: ssh_username: '' ssh_password: '' media_link: '' - script_args: '-P -Z' + script_args: '-P' diff --git a/tests/integration/files/conf/cloud.profiles.d/digital_ocean.conf b/tests/integration/files/conf/cloud.profiles.d/digital_ocean.conf index 486097b8c6a..72315f52694 100644 --- a/tests/integration/files/conf/cloud.profiles.d/digital_ocean.conf +++ b/tests/integration/files/conf/cloud.profiles.d/digital_ocean.conf @@ -2,4 +2,4 @@ digitalocean-test: provider: digitalocean-config image: 14.04.5 x64 size: 2GB - script_args: '-P -Z' + script_args: '-P' diff --git a/tests/integration/files/conf/cloud.profiles.d/ec2.conf b/tests/integration/files/conf/cloud.profiles.d/ec2.conf index eeec3219a96..511e433b179 100644 --- a/tests/integration/files/conf/cloud.profiles.d/ec2.conf +++ b/tests/integration/files/conf/cloud.profiles.d/ec2.conf @@ -3,7 +3,7 @@ ec2-test: image: ami-98aa1cf0 size: m1.large sh_username: ec2-user - script_args: '-P -Z' + script_args: '-P' ec2-win2012r2-test: provider: ec2-config size: m1.large diff --git a/tests/integration/files/conf/cloud.profiles.d/gogrid.conf b/tests/integration/files/conf/cloud.profiles.d/gogrid.conf index fc596e48d11..84d43a008ce 100644 --- a/tests/integration/files/conf/cloud.profiles.d/gogrid.conf +++ b/tests/integration/files/conf/cloud.profiles.d/gogrid.conf @@ -2,4 +2,4 @@ gogrid-test: provider: gogrid-config size: 512MB image: Ubuntu 14.04 LTS Server (64-bit) w/ None - script_args: '-P -Z' + script_args: '-P' diff --git a/tests/integration/files/conf/cloud.profiles.d/joyent.conf b/tests/integration/files/conf/cloud.profiles.d/joyent.conf index b628d923dae..bfe17b2928c 100644 --- a/tests/integration/files/conf/cloud.profiles.d/joyent.conf +++ b/tests/integration/files/conf/cloud.profiles.d/joyent.conf @@ -3,4 +3,4 @@ joyent-test: size: k4-highcpu-kvm-250M image: ubuntu-16.04 location: us-east-1 - script_args: '-P -Z' + script_args: '-P' diff --git a/tests/integration/files/conf/cloud.profiles.d/linode.conf b/tests/integration/files/conf/cloud.profiles.d/linode.conf index f419e8a9423..26a3e53a244 100644 --- a/tests/integration/files/conf/cloud.profiles.d/linode.conf +++ b/tests/integration/files/conf/cloud.profiles.d/linode.conf @@ -2,4 +2,4 @@ linode-test: provider: linode-config size: Linode 2GB image: Ubuntu 14.04 LTS - script_args: '-P -Z' + script_args: '-P' diff --git a/tests/integration/files/conf/cloud.profiles.d/rackspace.conf b/tests/integration/files/conf/cloud.profiles.d/rackspace.conf index 727a250f07f..3bf8e11355f 100644 --- a/tests/integration/files/conf/cloud.profiles.d/rackspace.conf +++ b/tests/integration/files/conf/cloud.profiles.d/rackspace.conf @@ -2,4 +2,4 @@ rackspace-test: provider: rackspace-config size: 2 GB Performance image: Ubuntu 14.04 LTS (Trusty Tahr) (PVHVM) - script_args: '-P -Z' + script_args: '-P' From d579b3e2ef7adca254ae7a8b554c3daa1db3ba88 Mon Sep 17 00:00:00 2001 From: twangboy Date: Wed, 15 Aug 2018 16:18:10 -0600 Subject: [PATCH 06/38] Add timeout parameter Add a timeout parameter to start, stop, restart, and delete --- salt/modules/win_service.py | 206 ++++++++++++++++++++++-------------- 1 file changed, 126 insertions(+), 80 deletions(-) diff --git a/salt/modules/win_service.py b/salt/modules/win_service.py index 49d6fb1dbfc..338cd79701d 100644 --- a/salt/modules/win_service.py +++ b/salt/modules/win_service.py @@ -84,8 +84,6 @@ SERVICE_ERROR_CONTROL = {0: 'Ignore', 'severe': 2, 'critical': 3} -RETRY_ATTEMPTS = 90 - def __virtual__(): ''' @@ -374,7 +372,7 @@ def info(name): return ret -def start(name): +def start(name, timeout=90): ''' Start the specified service. @@ -385,8 +383,12 @@ def start(name): Args: name (str): The name of the service to start + timeout (int): + The time in seconds to wait for the service to start before + returning. Default is 90 seconds + Returns: - bool: True if successful, False otherwise + bool: ``True`` if successful, otherwise ``False`` CLI Example: @@ -409,22 +411,26 @@ def start(name): attempts = 0 while info(name)['Status'] in ['Start Pending', 'Stopped'] \ - and attempts <= RETRY_ATTEMPTS: + and attempts <= timeout: time.sleep(1) attempts += 1 return status(name) -def stop(name): +def stop(name, timeout=90): ''' Stop the specified service Args: name (str): The name of the service to stop + timeout (int): + The time in seconds to wait for the service to stop before + returning. Default is 90 seconds + Returns: - bool: True if successful, False otherwise + bool: ``True`` if successful, otherwise ``False`` CLI Example: @@ -450,26 +456,35 @@ def stop(name): attempts = 0 while info(name)['Status'] in ['Running', 'Stop Pending'] \ - and attempts <= RETRY_ATTEMPTS: + and attempts <= timeout: time.sleep(1) attempts += 1 return not status(name) -def restart(name): +def restart(name, timeout=90): ''' Restart the named service. This issues a stop command followed by a start. Args: name: The name of the service to restart. - .. note:: - If the name passed is ``salt-minion`` a scheduled task is created and - executed to restart the salt-minion service. + .. note:: + If the name passed is ``salt-minion`` a scheduled task is + created and executed to restart the salt-minion service. + + timeout (int): + The time in seconds to wait for the service to stop and start before + returning. Default is 90 seconds + + .. note:: + The timeout is cumulative meaning it is applied to the stop and + then to the start command. A timeout of 90 could take up to 180 + seconds if the service is long in stopping and starting Returns: - bool: ``True`` if successful, ``False`` otherwise + bool: ``True`` if successful, otherwise ``False`` CLI Example: @@ -481,7 +496,8 @@ def restart(name): create_win_salt_restart_task() return execute_salt_restart_task() - return stop(name) and start(name) + return stop(name=name, timeout=timeout) and \ + start(name=name, timeout=timeout) def create_win_salt_restart_task(): @@ -527,13 +543,12 @@ def execute_salt_restart_task(): return __salt__['task.run'](name='restart-salt-minion') -def status(name, sig=None): +def status(name, **kwargs): ''' Return the status for a service Args: name (str): The name of the service to check - sig (str): Not supported on Windows Returns: bool: True if running, False otherwise @@ -590,20 +605,26 @@ def modify(name, .. versionadded:: 2016.11.0 Args: - name (str): The name of the service. Can be found using the + name (str): + The name of the service. Can be found using the ``service.get_service_name`` function - bin_path (str): The path to the service executable. Backslashes must be - escaped, eg: C:\\path\\to\\binary.exe + bin_path (str): + The path to the service executable. Backslashes must be escaped, eg: + ``C:\\path\\to\\binary.exe`` - exe_args (str): Any arguments required by the service executable + exe_args (str): + Any arguments required by the service executable - display_name (str): The name to display in the service manager + display_name (str): + The name to display in the service manager - description (str): The description to display for the service + description (str): + The description to display for the service - service_type (str): Specifies the service type. Default is ``own``. - Valid options are as follows: + service_type (str): + Specifies the service type. Default is ``own``. Valid options are as + follows: - kernel: Driver service - filesystem: File system driver service @@ -612,8 +633,8 @@ def modify(name, - own (default): Service runs in its own process - share: Service shares a process with one or more other services - start_type (str): Specifies the service start type. Valid options are as - follows: + start_type (str): + Specifies the service start type. Valid options are as follows: - boot: Device driver that is loaded by the boot loader - system: Device driver that is started during kernel initialization @@ -621,13 +642,14 @@ def modify(name, - manual: Service must be started manually - disabled: Service cannot be started - start_delayed (bool): Set the service to Auto(Delayed Start). Only valid - if the start_type is set to ``Auto``. If service_type is not passed, - but the service is already set to ``Auto``, then the flag will be - set. + start_delayed (bool): + Set the service to Auto(Delayed Start). Only valid if the start_type + is set to ``Auto``. If service_type is not passed, but the service + is already set to ``Auto``, then the flag will be set. - error_control (str): The severity of the error, and action taken, if - this service fails to start. Valid options are as follows: + error_control (str): + The severity of the error, and action taken, if this service fails + to start. Valid options are as follows: - normal: Error is logged and a message box is displayed - severe: Error is logged and computer attempts a restart with the @@ -637,29 +659,33 @@ def modify(name, - ignore: Error is logged and startup continues, no notification is given to the user - load_order_group: The name of the load order group to which this service - belongs + load_order_group (str): + The name of the load order group to which this service belongs - dependencies (list): A list of services or load ordering groups that - must start before this service + dependencies (list): + A list of services or load ordering groups that must start before + this service - account_name (str): The name of the account under which the service - should run. For ``own`` type services this should be in the - ``domain\username`` format. The following are examples of valid - built-in service accounts: + account_name (str): + The name of the account under which the service should run. For + ``own`` type services this should be in the ``domain\username`` + format. The following are examples of valid built-in service + accounts: - NT Authority\\LocalService - NT Authority\\NetworkService - NT Authority\\LocalSystem - .\LocalSystem - account_password (str): The password for the account name specified in - ``account_name``. For the above built-in accounts, this can be None. - Otherwise a password must be specified. + account_password (str): + The password for the account name specified in ``account_name``. For + the above built-in accounts, this can be None. Otherwise a password + must be specified. - run_interactive (bool): If this setting is True, the service will be - allowed to interact with the user. Not recommended for services that - run with elevated privileges. + run_interactive (bool): + If this setting is True, the service will be allowed to interact + with the user. Not recommended for services that run with elevated + privileges. Returns: dict: a dictionary of changes made @@ -895,20 +921,26 @@ def create(name, Args: - name (str): Specifies the service name. This is not the display_name + name (str): + Specifies the service name. This is not the display_name - bin_path (str): Specifies the path to the service binary file. - Backslashes must be escaped, eg: C:\\path\\to\\binary.exe + bin_path (str): + Specifies the path to the service binary file. Backslashes must be + escaped, eg: ``C:\\path\\to\\binary.exe`` - exe_args (str): Any additional arguments required by the service binary. + exe_args (str): + Any additional arguments required by the service binary. - display_name (str): the name to be displayed in the service manager. If - not passed, the ``name`` will be used + display_name (str): + The name to be displayed in the service manager. If not passed, the + ``name`` will be used - description (str): A description of the service + description (str): + A description of the service - service_type (str): Specifies the service type. Default is ``own``. - Valid options are as follows: + service_type (str): + Specifies the service type. Default is ``own``. Valid options are as + follows: - kernel: Driver service - filesystem: File system driver service @@ -917,8 +949,8 @@ def create(name, - own (default): Service runs in its own process - share: Service shares a process with one or more other services - start_type (str): Specifies the service start type. Valid options are as - follows: + start_type (str): + Specifies the service start type. Valid options are as follows: - boot: Device driver that is loaded by the boot loader - system: Device driver that is started during kernel initialization @@ -926,13 +958,15 @@ def create(name, - manual (default): Service must be started manually - disabled: Service cannot be started - start_delayed (bool): Set the service to Auto(Delayed Start). Only valid - if the start_type is set to ``Auto``. If service_type is not passed, - but the service is already set to ``Auto``, then the flag will be - set. Default is ``False`` + start_delayed (bool): + Set the service to Auto(Delayed Start). Only valid if the start_type + is set to ``Auto``. If service_type is not passed, but the service + is already set to ``Auto``, then the flag will be set. Default is + ``False`` - error_control (str): The severity of the error, and action taken, if - this service fails to start. Valid options are as follows: + error_control (str): + The severity of the error, and action taken, if this service fails + to start. Valid options are as follows: - normal (normal): Error is logged and a message box is displayed - severe: Error is logged and computer attempts a restart with the @@ -942,29 +976,33 @@ def create(name, - ignore: Error is logged and startup continues, no notification is given to the user - load_order_group: The name of the load order group to which this service - belongs + load_order_group (str): + The name of the load order group to which this service belongs - dependencies (list): A list of services or load ordering groups that - must start before this service + dependencies (list): + A list of services or load ordering groups that must start before + this service - account_name (str): The name of the account under which the service - should run. For ``own`` type services this should be in the - ``domain\username`` format. The following are examples of valid - built-in service accounts: + account_name (str): + The name of the account under which the service should run. For + ``own`` type services this should be in the ``domain\username`` + format. The following are examples of valid built-in service + accounts: - NT Authority\\LocalService - NT Authority\\NetworkService - NT Authority\\LocalSystem - .\\LocalSystem - account_password (str): The password for the account name specified in - ``account_name``. For the above built-in accounts, this can be None. - Otherwise a password must be specified. + account_password (str): + The password for the account name specified in ``account_name``. For + the above built-in accounts, this can be None. Otherwise a password + must be specified. - run_interactive (bool): If this setting is True, the service will be - allowed to interact with the user. Not recommended for services that - run with elevated privileges. + run_interactive (bool): + If this setting is True, the service will be allowed to interact + with the user. Not recommended for services that run with elevated + privileges. Returns: dict: A dictionary containing information about the new service @@ -1250,15 +1288,23 @@ def config(name, account_password=password) -def delete(name): +def delete(name, timeout=90): ''' Delete the named service Args: + name (str): The name of the service to delete + timeout (int): + The time in seconds to wait for the service to be deleted before + returning. This is necessary because a service must be stopped + before it can be deleted. Default is 90 seconds + + .. versionadded:: 2017.7.9, 2018.3.4 + Returns: - bool: True if successful, False otherwise + bool: ``True`` if successful, otherwise ``False`` CLI Example: @@ -1282,7 +1328,7 @@ def delete(name): win32service.CloseServiceHandle(handle_svc) attempts = 0 - while name in get_all() and attempts <= RETRY_ATTEMPTS: + while name in get_all() and attempts <= timeout: time.sleep(1) attempts += 1 From d44eaeed6cd550268631d81f0014c41aee3cd8f7 Mon Sep 17 00:00:00 2001 From: twangboy Date: Wed, 15 Aug 2018 19:20:46 -0600 Subject: [PATCH 07/38] Add timeout support to the state Remove redundent net stop command from the stop function Remove extra _enable call on Windows that was causing incorrect state return comments --- salt/modules/win_service.py | 11 +---------- salt/states/service.py | 10 ++++++---- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/salt/modules/win_service.py b/salt/modules/win_service.py index 338cd79701d..054e0b6e18b 100644 --- a/salt/modules/win_service.py +++ b/salt/modules/win_service.py @@ -438,15 +438,6 @@ def stop(name, timeout=90): salt '*' service.stop ''' - # net stop issues a stop command and waits briefly (~30s), but will give - # up if the service takes too long to stop with a misleading - # "service could not be stopped" message and RC 0. - - cmd = ['net', 'stop', '/y', name] - res = __salt__['cmd.run'](cmd, python_shell=False) - if 'service was stopped' in res: - return True - try: win32serviceutil.StopService(name) except pywintypes.error as exc: @@ -543,7 +534,7 @@ def execute_salt_restart_task(): return __salt__['task.run'](name='restart-salt-minion') -def status(name, **kwargs): +def status(name, *args, **kwargs): ''' Return the status for a service diff --git a/salt/states/service.py b/salt/states/service.py index 253bea11fa6..b11abbf5ab4 100644 --- a/salt/states/service.py +++ b/salt/states/service.py @@ -434,16 +434,15 @@ def running(name, ret['comment'] = 'Service {0} is set to start'.format(name) return ret - if salt.utils.is_windows(): - if enable is True: - ret.update(_enable(name, False, result=False, **kwargs)) - # Conditionally add systemd-specific args to call to service.start start_kwargs, warnings = \ _get_systemd_only(__salt__['service.start'], locals()) if warnings: ret.setdefault('warnings', []).extend(warnings) + if salt.utils.is_windows() and kwargs.get('timeout', False): + start_kwargs.update({'timeout': kwargs.get('timeout')}) + try: func_ret = __salt__['service.start'](name, **start_kwargs) except CommandExecutionError as exc: @@ -583,6 +582,9 @@ def dead(name, if warnings: ret.setdefault('warnings', []).extend(warnings) + if salt.utils.is_windows() and kwargs.get('timeout', False): + stop_kwargs.update({'timeout': kwargs.get('timeout')}) + func_ret = __salt__['service.stop'](name, **stop_kwargs) if not func_ret: ret['result'] = False From 57c4b9ff550af6c6cbcff0058a2934d5bbd4acbd Mon Sep 17 00:00:00 2001 From: Damon Atkins Date: Thu, 16 Aug 2018 14:58:35 +1000 Subject: [PATCH 08/38] 2017_win_service_damon Merge this if you like the suggestion. --- salt/modules/win_service.py | 53 +++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/salt/modules/win_service.py b/salt/modules/win_service.py index 054e0b6e18b..74cd75126d6 100644 --- a/salt/modules/win_service.py +++ b/salt/modules/win_service.py @@ -97,6 +97,39 @@ def __virtual__(): return __virtualname__ +def _wait_while_status_is(service_name, end_time, *service_states): + ''' + Internal use only in this module, wait for status of the service to + match the provided status before a end time expires. + + Args: + service_name (str): name of the service + + end_time (float): a future time. e.g. time.time() + 10 + + service_states (list): Status to wait for as return by info() + + Returns: + dict: A dictionary containing information about the service. + + :codeauthor: Damon Atkins + ''' + + info_results = info(service_name) + while info_results['Status'] in service_states and time.time() < end_time: + # From MS. Do not wait longer than the wait hint. A good interval is one-tenth of the + # wait hint but not less than 1 second and not more than 10 seconds. + wait_time = info_results['Status_WaitHint']/1000 + if wait_time < 1000: + wait_time = 1 + elif wait_time > 10000: + wait_time = 10 + else: + wait_time = wait_time / 1000 # convert ms to seconds + time.sleep(wait_time) + info_results = info(service_name) + + return info_results def get_enabled(): ''' @@ -403,19 +436,15 @@ def start(name, timeout=90): if disabled(name): modify(name, start_type='Manual') + end_t = time.time() + int(timeout) try: win32serviceutil.StartService(name) except pywintypes.error as exc: raise CommandExecutionError( 'Failed To Start {0}: {1}'.format(name, exc[2])) - attempts = 0 - while info(name)['Status'] in ['Start Pending', 'Stopped'] \ - and attempts <= timeout: - time.sleep(1) - attempts += 1 - - return status(name) + srv_status = _wait_while_status_is(name, end_t, 'Start Pending', 'Stopped') + return srv_status['status'] == 'Running' def stop(name, timeout=90): @@ -438,6 +467,7 @@ def stop(name, timeout=90): salt '*' service.stop ''' + end_t = time.time() + int(timeout) try: win32serviceutil.StopService(name) except pywintypes.error as exc: @@ -445,13 +475,8 @@ def stop(name, timeout=90): raise CommandExecutionError( 'Failed To Stop {0}: {1}'.format(name, exc[2])) - attempts = 0 - while info(name)['Status'] in ['Running', 'Stop Pending'] \ - and attempts <= timeout: - time.sleep(1) - attempts += 1 - - return not status(name) + srv_status = _wait_while_status_is(name, end_t, 'Running', 'Stop Pending') + return srv_status['status'] == 'Stopped' def restart(name, timeout=90): From 20f134edea85e62856add0cf95fb06403f0dd45f Mon Sep 17 00:00:00 2001 From: Erwin Dondorp Date: Thu, 16 Aug 2018 19:16:40 +0200 Subject: [PATCH 09/38] Fixed unknown 'exceptions' under Python3 (#49152) --- salt/utils/error.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/salt/utils/error.py b/salt/utils/error.py index 23f4b47fb68..e6964518712 100644 --- a/salt/utils/error.py +++ b/salt/utils/error.py @@ -5,11 +5,15 @@ Utilities to enable exception reraising across the master commands ''' from __future__ import absolute_import -# Import python libs -try: +import sys +# True if we are running on Python 3. +PY3 = sys.version_info[0] == 3 + +if PY3: + import builtins + exceptions = builtins +else: import exceptions -except ImportError: - pass # Import salt libs import salt.exceptions From 9da79dd0b58f385b0cb80af6691b2032441c86d2 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 16 Aug 2018 10:48:19 -0700 Subject: [PATCH 10/38] Allow test suite to finish if tmp dir removal fails --- tests/integration/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py index 72e136d3c72..245d8f91d92 100644 --- a/tests/integration/__init__.py +++ b/tests/integration/__init__.py @@ -1115,7 +1115,10 @@ class TestDaemon(object): for dirname in (TMP, RUNTIME_VARS.TMP_STATE_TREE, RUNTIME_VARS.TMP_PILLAR_TREE, RUNTIME_VARS.TMP_PRODENV_STATE_TREE): if os.path.isdir(dirname): - shutil.rmtree(dirname, onerror=remove_readonly) + try: + shutil.rmtree(dirname, onerror=remove_readonly) + except: + log.exception('Failed to remove directory: %s', dirname) def wait_for_jid(self, targets, jid, timeout=120): time.sleep(1) # Allow some time for minions to accept jobs From 3384864e8f39e95184e0da9519408f510c83cbf3 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 16 Aug 2018 11:57:28 -0700 Subject: [PATCH 11/38] add Exception type --- tests/integration/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py index 245d8f91d92..b7214aa60b0 100644 --- a/tests/integration/__init__.py +++ b/tests/integration/__init__.py @@ -1117,7 +1117,7 @@ class TestDaemon(object): if os.path.isdir(dirname): try: shutil.rmtree(dirname, onerror=remove_readonly) - except: + except Exception: log.exception('Failed to remove directory: %s', dirname) def wait_for_jid(self, targets, jid, timeout=120): From 82638c6dd220cb25a42efdb7d1f5f84de8bb911c Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Thu, 16 Aug 2018 14:01:15 -0500 Subject: [PATCH 12/38] Fix bug in keep_source for non-templated salt:// file sources Unless the file is one of A) a remote file source (http/https/ftp/etc.) or B) being templated, then `file.get_managed` does not cache the file. Thus, the variable we rely on to tell us the cached path is an empty string, and we don't remove it even though the file would eventually later be cached when we run `file.manage_file`. This adds some additional logic to ensure that we do check if the file has been cached if `sfn` is an empty string. --- salt/states/file.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/salt/states/file.py b/salt/states/file.py index 2f4f16d1ea8..7115097a96c 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -2596,8 +2596,15 @@ def managed(name, ret['changes'] = {} log.debug(traceback.format_exc()) salt.utils.files.remove(tmp_filename) - if not keep_source and sfn: - salt.utils.files.remove(sfn) + if not keep_source: + if not sfn \ + and source \ + and _urlparse(source).scheme == 'salt': + # The file would not have been cached until manage_file was + # run, so check again here for a cached copy. + sfn = __salt__['cp.is_cached'](source, __env__) + if sfn: + salt.utils.files.remove(sfn) return _error(ret, 'Unable to check_cmd file: {0}'.format(exc)) # file being updated to verify using check_cmd @@ -2667,8 +2674,15 @@ def managed(name, finally: if tmp_filename: salt.utils.files.remove(tmp_filename) - if not keep_source and sfn: - salt.utils.files.remove(sfn) + if not keep_source: + if not sfn \ + and source \ + and _urlparse(source).scheme == 'salt': + # The file would not have been cached until manage_file was + # run, so check again here for a cached copy. + sfn = __salt__['cp.is_cached'](source, __env__) + if sfn: + salt.utils.files.remove(sfn) _RECURSE_TYPES = ['user', 'group', 'mode', 'ignore_files', 'ignore_dirs'] From b4544d7f89c12e7c31dd61f6a14aafcbfc86f25c Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Thu, 16 Aug 2018 14:53:32 -0500 Subject: [PATCH 13/38] Add keep_source integration tests --- tests/integration/states/test_file.py | 50 +++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index 60acaeea2d6..ceeb45873a2 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -723,6 +723,29 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): if os.path.exists(name): os.remove(name) + @with_tempfile + def test_managed_keep_source_false_salt(self, name): + ''' + This test ensures that we properly clean the cached file if keep_source + is set to False, for source files using a salt:// URL + ''' + source = 'salt://grail/scene33' + saltenv = 'base' + + # Run the state + ret = self.run_state( + 'file.managed', + name=name, + source=source, + saltenv=saltenv, + keep_source=False) + ret = ret[next(iter(ret))] + assert ret['result'] is True + + # Now make sure that the file is not cached + result = self.run_function('cp.is_cached', [source, saltenv]) + assert result == '', 'File is still cached at {0}'.format(result) + def test_directory(self): ''' file.directory @@ -3860,6 +3883,11 @@ class RemoteFileTest(ModuleCase, SaltReturnAssertsMixin): if exc.errno != errno.ENOENT: raise exc + def run_state(self, *args, **kwargs): + ret = super(RemoteFileTest, self).run_state(*args, **kwargs) + log.debug('ret = %s', ret) + return ret + def test_file_managed_http_source_no_hash(self): ''' Test a remote file with no hash @@ -3868,7 +3896,6 @@ class RemoteFileTest(ModuleCase, SaltReturnAssertsMixin): name=self.name, source=self.source, skip_verify=False) - log.debug('ret = %s', ret) # This should fail because no hash was provided self.assertSaltFalseReturn(ret) @@ -3881,7 +3908,6 @@ class RemoteFileTest(ModuleCase, SaltReturnAssertsMixin): source=self.source, source_hash=self.source_hash, skip_verify=False) - log.debug('ret = %s', ret) self.assertSaltTrueReturn(ret) def test_file_managed_http_source_skip_verify(self): @@ -3892,9 +3918,27 @@ class RemoteFileTest(ModuleCase, SaltReturnAssertsMixin): name=self.name, source=self.source, skip_verify=True) - log.debug('ret = %s', ret) self.assertSaltTrueReturn(ret) + def test_file_managed_keep_source_false_http(self): + ''' + This test ensures that we properly clean the cached file if keep_source + is set to False, for source files using an http:// URL + ''' + # Run the state + ret = self.run_state('file.managed', + name=self.name, + source=self.source, + source_hash=self.source_hash, + keep_source=False) + ret = ret[next(iter(ret))] + assert ret['result'] is True + + # Now make sure that the file is not cached + result = self.run_function('cp.is_cached', [self.source]) + assert result == '', 'File is still cached at {0}'.format(result) + + WIN_TEST_FILE = 'c:/testfile' From 7dd795322ef63c6fae4c6d317c7eca22c8de567e Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 16 Aug 2018 14:00:38 -0600 Subject: [PATCH 14/38] Fix start/stop functions Fix typo in the dict key 'Status' Standardize docs Fix some lint issues Rename the new helper function to _status_wait Add urls to code comments Add version added statements --- salt/modules/win_service.py | 61 ++++++++++++++++++++++++++----------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/salt/modules/win_service.py b/salt/modules/win_service.py index 74cd75126d6..85578bb75b2 100644 --- a/salt/modules/win_service.py +++ b/salt/modules/win_service.py @@ -97,40 +97,51 @@ def __virtual__(): return __virtualname__ -def _wait_while_status_is(service_name, end_time, *service_states): + +def _status_wait(service_name, end_time, service_states): ''' - Internal use only in this module, wait for status of the service to - match the provided status before a end time expires. + Helper function that will wait for the status of the service to match the + provided status before an end time expires. Used for service stop and start + + .. versionadded:: 2017.7.9, 2018.3.4 Args: - service_name (str): name of the service + service_name (str): + The name of the service - end_time (float): a future time. e.g. time.time() + 10 + end_time (float): + A future time. e.g. time.time() + 10 - service_states (list): Status to wait for as return by info() + service_states (list): + Services statuses to wait for as returned by info() Returns: dict: A dictionary containing information about the service. :codeauthor: Damon Atkins ''' + info_results = info(service_name) - info_results = info(service_name) while info_results['Status'] in service_states and time.time() < end_time: - # From MS. Do not wait longer than the wait hint. A good interval is one-tenth of the - # wait hint but not less than 1 second and not more than 10 seconds. - wait_time = info_results['Status_WaitHint']/1000 + # From Microsoft: Do not wait longer than the wait hint. A good interval + # is one-tenth of the wait hint but not less than 1 second and not more + # than 10 seconds. + # https://docs.microsoft.com/en-us/windows/desktop/services/starting-a-service + # https://docs.microsoft.com/en-us/windows/desktop/services/stopping-a-service + wait_time = info_results['Status_WaitHint'] / 1000 if wait_time < 1000: wait_time = 1 elif wait_time > 10000: wait_time = 10 else: - wait_time = wait_time / 1000 # convert ms to seconds + wait_time = wait_time / 1000 # convert ms to seconds + time.sleep(wait_time) - info_results = info(service_name) + info_results = info(service_name) return info_results + def get_enabled(): ''' Return a list of enabled services. Enabled is defined as a service that is @@ -420,6 +431,8 @@ def start(name, timeout=90): The time in seconds to wait for the service to start before returning. Default is 90 seconds + .. versionadded:: 2017.7.9, 2018.3.4 + Returns: bool: ``True`` if successful, otherwise ``False`` @@ -436,15 +449,17 @@ def start(name, timeout=90): if disabled(name): modify(name, start_type='Manual') - end_t = time.time() + int(timeout) try: win32serviceutil.StartService(name) except pywintypes.error as exc: raise CommandExecutionError( 'Failed To Start {0}: {1}'.format(name, exc[2])) - srv_status = _wait_while_status_is(name, end_t, 'Start Pending', 'Stopped') - return srv_status['status'] == 'Running' + srv_status = _status_wait(service_name=name, + end_time=time.time() + int(timeout), + service_states=['Start Pending', 'Stopped']) + + return srv_status['Status'] == 'Running' def stop(name, timeout=90): @@ -458,6 +473,8 @@ def stop(name, timeout=90): The time in seconds to wait for the service to stop before returning. Default is 90 seconds + .. versionadded:: 2017.7.9, 2018.3.4 + Returns: bool: ``True`` if successful, otherwise ``False`` @@ -467,7 +484,6 @@ def stop(name, timeout=90): salt '*' service.stop ''' - end_t = time.time() + int(timeout) try: win32serviceutil.StopService(name) except pywintypes.error as exc: @@ -475,8 +491,15 @@ def stop(name, timeout=90): raise CommandExecutionError( 'Failed To Stop {0}: {1}'.format(name, exc[2])) - srv_status = _wait_while_status_is(name, end_t, 'Running', 'Stop Pending') - return srv_status['status'] == 'Stopped' + srv_status = _status_wait(service_name=name, + end_time=time.time() + int(timeout), + service_states=['Running', 'Stop Pending']) + + log.debug('TEST ' * 20) + log.debug(srv_status) + log.debug('TEST ' * 20) + + return srv_status['Status'] == 'Stopped' def restart(name, timeout=90): @@ -499,6 +522,8 @@ def restart(name, timeout=90): then to the start command. A timeout of 90 could take up to 180 seconds if the service is long in stopping and starting + .. versionadded:: 2017.7.9, 2018.3.4 + Returns: bool: ``True`` if successful, otherwise ``False`` From a6ecb7552de731ec77850c4907e8ef26d0e1fe47 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 16 Aug 2018 14:03:16 -0600 Subject: [PATCH 15/38] Remove testing debug stuff --- salt/modules/win_service.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/salt/modules/win_service.py b/salt/modules/win_service.py index 85578bb75b2..b2906514c5f 100644 --- a/salt/modules/win_service.py +++ b/salt/modules/win_service.py @@ -495,10 +495,6 @@ def stop(name, timeout=90): end_time=time.time() + int(timeout), service_states=['Running', 'Stop Pending']) - log.debug('TEST ' * 20) - log.debug(srv_status) - log.debug('TEST ' * 20) - return srv_status['Status'] == 'Stopped' From 4f9973db20c505fd7301c4cedf7a0cb4acdfea25 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 16 Aug 2018 15:01:18 -0600 Subject: [PATCH 16/38] Improve timeout in delete Improved error handling --- salt/modules/win_service.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/salt/modules/win_service.py b/salt/modules/win_service.py index b2906514c5f..b4577be63ae 100644 --- a/salt/modules/win_service.py +++ b/salt/modules/win_service.py @@ -1357,16 +1357,20 @@ def delete(name, timeout=90): handle_scm, name, win32service.SERVICE_ALL_ACCESS) except pywintypes.error as exc: raise CommandExecutionError( - 'Failed To Open {0}: {1}'.format(name, exc[2])) + 'Failed to open {0}. {1}'.format(name, exc.strerror)) - win32service.DeleteService(handle_svc) + try: + win32service.DeleteService(handle_svc) + except pywintypes.error as exc: + raise CommandExecutionError( + 'Failed to delete {0}. {1}'.format(name, exc.strerror)) + finally: + log.debug('Cleaning up') + win32service.CloseServiceHandle(handle_scm) + win32service.CloseServiceHandle(handle_svc) - win32service.CloseServiceHandle(handle_scm) - win32service.CloseServiceHandle(handle_svc) - - attempts = 0 - while name in get_all() and attempts <= timeout: + end_time = time.time() + int(timeout) + while name in get_all() and time.time() < end_time: time.sleep(1) - attempts += 1 return name not in get_all() From 7264008fedc73dba1f54f87d2dd7e26ca1db9f2b Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 16 Aug 2018 15:04:07 -0600 Subject: [PATCH 17/38] Fix some lint (remove whitespace)` --- salt/modules/win_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/win_service.py b/salt/modules/win_service.py index b4577be63ae..b6d0cc04e44 100644 --- a/salt/modules/win_service.py +++ b/salt/modules/win_service.py @@ -108,10 +108,10 @@ def _status_wait(service_name, end_time, service_states): Args: service_name (str): The name of the service - + end_time (float): A future time. e.g. time.time() + 10 - + service_states (list): Services statuses to wait for as returned by info() From 684425131565ffcf52afd8f2b797cb4c7e4640a2 Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 17 Aug 2018 02:56:17 +0000 Subject: [PATCH 18/38] Fix remaining file state integration tests (py3) --- salt/states/file.py | 4 ++-- tests/integration/states/test_file.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/salt/states/file.py b/salt/states/file.py index 2f4f16d1ea8..057e32309ce 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -2919,7 +2919,7 @@ def directory(name, perms: full_control - win_inheritance: False ''' - name = os.path.expanduser(name) + name = os.path.normcase(os.path.expanduser(name)) ret = {'name': name, 'changes': {}, 'pchanges': {}, @@ -3405,7 +3405,7 @@ def recurse(name, ) kwargs.pop('env') - name = os.path.expanduser(sdecode(name)) + name = os.path.normcase(os.path.expanduser(sdecode(name))) user = _test_owner(kwargs, user=user) if salt.utils.is_windows(): diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index 60acaeea2d6..505b6050c99 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -267,7 +267,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): if IS_WINDOWS: expected = 'The \'mode\' option is not supported on Windows' - self.assertEqual(ret[ret.keys()[0]]['comment'], expected) + self.assertEqual(ret[list(ret.keys())[0]]['comment'], expected) self.assertSaltFalseReturn(ret) return @@ -303,7 +303,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): if IS_WINDOWS: expected = 'The \'mode\' option is not supported on Windows' - self.assertEqual(ret[ret.keys()[0]]['comment'], expected) + self.assertEqual(ret[list(ret.keys())[0]]['comment'], expected) self.assertSaltFalseReturn(ret) return @@ -335,7 +335,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): if IS_WINDOWS: expected = 'The \'mode\' option is not supported on Windows' - self.assertEqual(ret[ret.keys()[0]]['comment'], expected) + self.assertEqual(ret[list(ret.keys())[0]]['comment'], expected) self.assertSaltFalseReturn(ret) return @@ -418,7 +418,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): ) if IS_WINDOWS: expected = 'The \'mode\' option is not supported on Windows' - self.assertEqual(ret[ret.keys()[0]]['comment'], expected) + self.assertEqual(ret[list(ret.keys())[0]]['comment'], expected) self.assertSaltFalseReturn(ret) return From b5ba073d6593aa7376d4e7bc7d88492bffe4d39f Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Thu, 16 Aug 2018 21:17:08 -0700 Subject: [PATCH 19/38] Simplify dict keys lookup --- tests/integration/states/test_file.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index 505b6050c99..153e774d718 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -267,7 +267,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): if IS_WINDOWS: expected = 'The \'mode\' option is not supported on Windows' - self.assertEqual(ret[list(ret.keys())[0]]['comment'], expected) + self.assertEqual(ret[list(ret)[0]]['comment'], expected) self.assertSaltFalseReturn(ret) return @@ -303,7 +303,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): if IS_WINDOWS: expected = 'The \'mode\' option is not supported on Windows' - self.assertEqual(ret[list(ret.keys())[0]]['comment'], expected) + self.assertEqual(ret[list(ret)[0]]['comment'], expected) self.assertSaltFalseReturn(ret) return @@ -335,7 +335,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): if IS_WINDOWS: expected = 'The \'mode\' option is not supported on Windows' - self.assertEqual(ret[list(ret.keys())[0]]['comment'], expected) + self.assertEqual(ret[list(ret)[0]]['comment'], expected) self.assertSaltFalseReturn(ret) return @@ -418,7 +418,7 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): ) if IS_WINDOWS: expected = 'The \'mode\' option is not supported on Windows' - self.assertEqual(ret[list(ret.keys())[0]]['comment'], expected) + self.assertEqual(ret[list(ret)[0]]['comment'], expected) self.assertSaltFalseReturn(ret) return From ec1f01357352e33afdf45f20c24f705cb1520790 Mon Sep 17 00:00:00 2001 From: Erwin Dondorp Date: Fri, 17 Aug 2018 11:14:40 +0200 Subject: [PATCH 20/38] Improved solution as per @dwoz's suggestion --- salt/utils/error.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/salt/utils/error.py b/salt/utils/error.py index e6964518712..e1b7db8b5ea 100644 --- a/salt/utils/error.py +++ b/salt/utils/error.py @@ -5,20 +5,12 @@ Utilities to enable exception reraising across the master commands ''' from __future__ import absolute_import -import sys -# True if we are running on Python 3. -PY3 = sys.version_info[0] == 3 - -if PY3: - import builtins - exceptions = builtins -else: - import exceptions - # Import salt libs import salt.exceptions import salt.utils.event +# Import 3rd-party libs +from salt.ext.six.moves import builtins as exceptions def raise_error(name=None, args=None, message=''): ''' From 4335c5cdc497c7c02212aa1680b77d1c7ec857e4 Mon Sep 17 00:00:00 2001 From: Erwin Dondorp Date: Fri, 17 Aug 2018 12:29:22 +0200 Subject: [PATCH 21/38] Must have 2 lines between imports and code --- salt/utils/error.py | 1 + 1 file changed, 1 insertion(+) diff --git a/salt/utils/error.py b/salt/utils/error.py index e1b7db8b5ea..149518ac84e 100644 --- a/salt/utils/error.py +++ b/salt/utils/error.py @@ -12,6 +12,7 @@ import salt.utils.event # Import 3rd-party libs from salt.ext.six.moves import builtins as exceptions + def raise_error(name=None, args=None, message=''): ''' Raise an exception with __name__ from name, args from args From 3363238ff647b0e2f28775f463b7ba67e05f1c4d Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 17 Aug 2018 13:17:08 -0700 Subject: [PATCH 22/38] Account for normalized dirs in unit tests --- tests/unit/states/test_file.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/unit/states/test_file.py b/tests/unit/states/test_file.py index db52b122f2b..e7bdb6b6b14 100644 --- a/tests/unit/states/test_file.py +++ b/tests/unit/states/test_file.py @@ -769,6 +769,8 @@ class TestFileState(TestCase, LoaderModuleMockMixin): name = '/etc/testdir' user = 'salt' group = 'saltstack' + if salt.utils.is_windows(): + name = name.replace('/', '\\') ret = {'name': name, 'result': False, @@ -917,6 +919,8 @@ class TestFileState(TestCase, LoaderModuleMockMixin): source = 'salt://code/flask' user = 'salt' group = 'saltstack' + if salt.utils.is_windows(): + name = name.replace('/', '\\') ret = {'name': name, 'result': False, From 913ea5ebde9f296d4209345d623c66ea612f997f Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Fri, 17 Aug 2018 14:07:19 -0700 Subject: [PATCH 23/38] Fix directory unit test --- tests/unit/states/test_file.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/states/test_file.py b/tests/unit/states/test_file.py index e7bdb6b6b14..2dbcf821f83 100644 --- a/tests/unit/states/test_file.py +++ b/tests/unit/states/test_file.py @@ -867,12 +867,12 @@ class TestFileState(TestCase, LoaderModuleMockMixin): else: comt = ('The following files will be changed:\n{0}:' ' directory - new\n'.format(name)) - p_chg = {'/etc/testdir': {'directory': 'new'}} + p_chg = {name: {'directory': 'new'}} ret.update({ 'comment': comt, 'result': None, 'pchanges': p_chg, - 'changes': {'/etc/testdir': {'directory': 'new'}} + 'changes': {name: {'directory': 'new'}} }) self.assertDictEqual(filestate.directory(name, user=user, From 2e29b292bffd51c8536a65ecb2a1db8a336f779c Mon Sep 17 00:00:00 2001 From: "Samuel D. Leslie" Date: Wed, 15 Aug 2018 12:22:16 +1000 Subject: [PATCH 24/38] Remove obsolete documentation on external_nodes setting This setting is no longer used anywhere in the codebase having been superseded by the master_tops setting many releases ago. --- conf/master | 6 ------ conf/suse/master | 6 ------ doc/man/salt.7 | 25 ------------------------- doc/ref/configuration/master.rst | 17 ----------------- 4 files changed, 54 deletions(-) diff --git a/conf/master b/conf/master index abfc1fa8088..e88aa578c7c 100644 --- a/conf/master +++ b/conf/master @@ -529,12 +529,6 @@ # #master_tops: {} -# The external_nodes option allows Salt to gather data that would normally be -# placed in a top file. The external_nodes option is the executable that will -# return the ENC data. Remember that Salt will look for external nodes AND top -# files and combine the results if both are enabled! -#external_nodes: None - # The renderer to use on the minions to render the state data #renderer: yaml_jinja diff --git a/conf/suse/master b/conf/suse/master index aeaa1d88591..e0a51c000fb 100644 --- a/conf/suse/master +++ b/conf/suse/master @@ -528,12 +528,6 @@ syndic_user: salt # #master_tops: {} -# The external_nodes option allows Salt to gather data that would normally be -# placed in a top file. The external_nodes option is the executable that will -# return the ENC data. Remember that Salt will look for external nodes AND top -# files and combine the results if both are enabled! -#external_nodes: None - # The renderer to use on the minions to render the state data #renderer: yaml_jinja diff --git a/doc/man/salt.7 b/doc/man/salt.7 index aeb69051f38..3dedd13dae2 100644 --- a/doc/man/salt.7 +++ b/doc/man/salt.7 @@ -6885,25 +6885,6 @@ master_tops: .fi .UNINDENT .UNINDENT -.SS \fBexternal_nodes\fP -.sp -Default: None -.sp -The external_nodes option allows Salt to gather data that would normally be -placed in a top file from and external node controller. The external_nodes -option is the executable that will return the ENC data. Remember that Salt -will look for external nodes AND top files and combine the results if both -are enabled and available! -.INDENT 0.0 -.INDENT 3.5 -.sp -.nf -.ft C -external_nodes: cobbler\-ext\-nodes -.ft P -.fi -.UNINDENT -.UNINDENT .SS \fBrenderer\fP .sp Default: \fByaml_jinja\fP @@ -14770,12 +14751,6 @@ and \fBmine_functions\fP\&. # #master_tops: {} -# The external_nodes option allows Salt to gather data that would normally be -# placed in a top file. The external_nodes option is the executable that will -# return the ENC data. Remember that Salt will look for external nodes AND top -# files and combine the results if both are enabled! -#external_nodes: None - # The renderer to use on the minions to render the state data #renderer: yaml_jinja diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index f243ad887bd..5105f954ecc 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -1918,23 +1918,6 @@ following configuration: master_tops: ext_nodes: -.. conf_master:: external_nodes - -``external_nodes`` ------------------- - -Default: None - -The external_nodes option allows Salt to gather data that would normally be -placed in a top file from and external node controller. The external_nodes -option is the executable that will return the ENC data. Remember that Salt -will look for external nodes AND top files and combine the results if both -are enabled and available! - -.. code-block:: yaml - - external_nodes: cobbler-ext-nodes - .. conf_master:: renderer ``renderer`` From 2fe675cfdd86ba370f3eec70dd50d149a70d04e5 Mon Sep 17 00:00:00 2001 From: "Samuel D. Leslie" Date: Sat, 18 Aug 2018 13:44:24 +1000 Subject: [PATCH 25/38] Update documentation to correctly state enable_gpu_grains default While the relevant code in grains/core.py does enable GPU grains by default, the default in config/__init__.py for the DEFAULT_MASTER_OPTS dictionary is disabled. This takes precedence over the default in grains/core.py which would only take effect if the enable_gpu_grains setting was not specified at all. --- conf/master | 2 +- conf/suse/master | 2 +- doc/man/salt.7 | 4 ++-- doc/ref/configuration/master.rst | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/conf/master b/conf/master index abfc1fa8088..b0ce65423a1 100644 --- a/conf/master +++ b/conf/master @@ -125,7 +125,7 @@ # The master can take a while to start up when lspci and/or dmidecode is used # to populate the grains for the master. Enable if you want to see GPU hardware # data for your master. -# enable_gpu_grains: False +# enable_gpu_grains: True # The master maintains a job cache. While this is a great addition, it can be # a burden on the master for larger deployments (over 5000 minions). diff --git a/conf/suse/master b/conf/suse/master index aeaa1d88591..b738ba571c3 100644 --- a/conf/suse/master +++ b/conf/suse/master @@ -127,7 +127,7 @@ syndic_user: salt # The master can take a while to start up when lspci and/or dmidecode is used # to populate the grains for the master. Enable if you want to see GPU hardware # data for your master. -# enable_gpu_grains: False +# enable_gpu_grains: True # The master maintains a job cache. While this is a great addition, it can be # a burden on the master for larger deployments (over 5000 minions). diff --git a/doc/man/salt.7 b/doc/man/salt.7 index aeb69051f38..5d34e8ed11f 100644 --- a/doc/man/salt.7 +++ b/doc/man/salt.7 @@ -5261,7 +5261,7 @@ sock_dir: /var/run/salt/master .UNINDENT .SS \fBenable_gpu_grains\fP .sp -Default: \fBTrue\fP +Default: \fBFalse\fP .sp Enable GPU hardware data for your master. Be aware that the master can take a while to start up when lspci and/or dmidecode is used to populate the @@ -14366,7 +14366,7 @@ and \fBmine_functions\fP\&. # The master can take a while to start up when lspci and/or dmidecode is used # to populate the grains for the master. Enable if you want to see GPU hardware # data for your master. -# enable_gpu_grains: False +# enable_gpu_grains: True # The master maintains a job cache. While this is a great addition, it can be # a burden on the master for larger deployments (over 5000 minions). diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index f243ad887bd..6fa7b0e92bf 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -440,7 +440,7 @@ communication. ``enable_gpu_grains`` --------------------- -Default: ``True`` +Default: ``False`` Enable GPU hardware data for your master. Be aware that the master can take a while to start up when lspci and/or dmidecode is used to populate the From 2149e228ac2e5bfee192f5b9b3e5d86f8c273e17 Mon Sep 17 00:00:00 2001 From: Erwin Dondorp Date: Sat, 18 Aug 2018 13:22:05 +0200 Subject: [PATCH 26/38] Fix for #45620: "Salt CLI is rounding floats to 2 decimal places" (actually: Salt CLI is using only 12 digits for precision) use repr() to get the full precision also for older python versions as until about python32 it was limited to 12 digits only by default --- salt/output/nested.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/output/nested.py b/salt/output/nested.py index 7d21dba49f9..c4c2e2ed7d7 100644 --- a/salt/output/nested.py +++ b/salt/output/nested.py @@ -81,12 +81,14 @@ class NestDisplay(object): ) # Number includes all python numbers types # (float, int, long, complex, ...) + # use repr() to get the full precision also for older python versions + # as until about python32 it was limited to 12 digits only by default elif isinstance(ret, Number): out.append( self.ustring( indent, self.LIGHT_YELLOW, - ret, + repr(ret), prefix=prefix ) ) From 12261a543e9fdc94ae2743ca500b0cbb73bab99e Mon Sep 17 00:00:00 2001 From: Erwin Dondorp Date: Sat, 18 Aug 2018 13:55:22 +0200 Subject: [PATCH 27/38] trailing whitespace removal --- salt/output/nested.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/output/nested.py b/salt/output/nested.py index c4c2e2ed7d7..21983c73b1b 100644 --- a/salt/output/nested.py +++ b/salt/output/nested.py @@ -81,7 +81,7 @@ class NestDisplay(object): ) # Number includes all python numbers types # (float, int, long, complex, ...) - # use repr() to get the full precision also for older python versions + # use repr() to get the full precision also for older python versions # as until about python32 it was limited to 12 digits only by default elif isinstance(ret, Number): out.append( From ff5ec86252e9f52ff8f9eddd648068c4892e1c89 Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 20 Aug 2018 17:36:55 -0600 Subject: [PATCH 28/38] Work with seconds --- salt/modules/win_service.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/salt/modules/win_service.py b/salt/modules/win_service.py index b6d0cc04e44..53ebfa88579 100644 --- a/salt/modules/win_service.py +++ b/salt/modules/win_service.py @@ -128,13 +128,14 @@ def _status_wait(service_name, end_time, service_states): # than 10 seconds. # https://docs.microsoft.com/en-us/windows/desktop/services/starting-a-service # https://docs.microsoft.com/en-us/windows/desktop/services/stopping-a-service - wait_time = info_results['Status_WaitHint'] / 1000 - if wait_time < 1000: + # Wait hint is in ms + wait_time = info_results['Status_WaitHint'] + # Convert to seconds or 0 + wait_time = wait_time / 1000 if wait_time else 0 + if wait_time < 1: wait_time = 1 - elif wait_time > 10000: + elif wait_time > 10: wait_time = 10 - else: - wait_time = wait_time / 1000 # convert ms to seconds time.sleep(wait_time) info_results = info(service_name) From ec2a091376151a56da99ffa9b052caf52a57eb58 Mon Sep 17 00:00:00 2001 From: Frantisek Holop Date: Tue, 21 Aug 2018 16:08:10 +0200 Subject: [PATCH 29/38] check for mandatory parameters to avoid false positives --- salt/states/mount.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/salt/states/mount.py b/salt/states/mount.py index f046c794a1a..0b49e66ac03 100644 --- a/salt/states/mount.py +++ b/salt/states/mount.py @@ -186,6 +186,21 @@ def mounted(name, 'result': True, 'comment': ''} + if not name: + ret['result'] = False + ret['comment'] = 'Must provide name to mount.mounted' + return ret + + if not device: + ret['result'] = False + ret['comment'] = 'Must provide device to mount.mounted' + return ret + + if not fstype: + ret['result'] = False + ret['comment'] = 'Must provide fstype to mount.mounted' + return ret + if device_name_regex is None: device_name_regex = [] @@ -720,6 +735,11 @@ def unmounted(name, 'result': True, 'comment': ''} + if not name: + ret['result'] = False + ret['comment'] = 'Must provide name to mount.unmounted' + return ret + # Get the active data active = __salt__['mount.active'](extended=True) if name not in active: From eb5cab35cacec2802ef7292bba3cbfa5298cb835 Mon Sep 17 00:00:00 2001 From: Frantisek Holop Date: Tue, 21 Aug 2018 16:09:29 +0200 Subject: [PATCH 30/38] fix some underhanging indent while here... --- salt/states/mount.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/salt/states/mount.py b/salt/states/mount.py index 0b49e66ac03..4d9088b624e 100644 --- a/salt/states/mount.py +++ b/salt/states/mount.py @@ -534,11 +534,11 @@ def mounted(name, if __opts__['test']: if __grains__['os'] in ['MacOS', 'Darwin']: out = __salt__['mount.set_automaster'](name, - device, - fstype, - opts, - config, - test=True) + device, + fstype, + opts, + config, + test=True) else: out = __salt__['mount.set_fstab'](name, device, @@ -583,10 +583,10 @@ def mounted(name, else: if __grains__['os'] in ['MacOS', 'Darwin']: out = __salt__['mount.set_automaster'](name, - device, - fstype, - opts, - config) + device, + fstype, + opts, + config) else: out = __salt__['mount.set_fstab'](name, device, From 233bbae62fdf5fb7c0ba5809d694c1486ae048d0 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 21 Aug 2018 14:54:39 -0500 Subject: [PATCH 31/38] Allow compound matching in eauth config expressions When we try to match the configured expression, we look for a match type (e.g. `I@`, `G@`, etc.) at the beginning, and when we don't find one we were falling back to a glob. This changes the fallback to compound so that we can support compound matches. Note that the compound matching engine will act just like the glob match engine when the host passed to it is a single minion ID glob, so using compound as the fallback gets you glob matching for free. --- salt/utils/minions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/utils/minions.py b/salt/utils/minions.py index 6ef6b269374..c24154954ab 100644 --- a/salt/utils/minions.py +++ b/salt/utils/minions.py @@ -670,7 +670,7 @@ class CkMinions(object): 'S': 'ipcidr', 'E': 'pcre', 'N': 'node', - None: 'glob'} + None: 'compound'} target_info = parse_target(auth_entry) if not target_info: From d3358423aee69f37a89073bf719621552be131e0 Mon Sep 17 00:00:00 2001 From: twangboy Date: Tue, 21 Aug 2018 12:37:17 -0600 Subject: [PATCH 32/38] Use os.linesep.join instead of textwrap.dedent --- tests/integration/states/test_file.py | 104 +++++++++++++------------- 1 file changed, 52 insertions(+), 52 deletions(-) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index 36e18df4512..af633301534 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -5,7 +5,7 @@ Tests for the file state ''' # Import python libs -from __future__ import absolute_import +from __future__ import absolute_import, unicode_literals import errno import glob import logging @@ -2647,57 +2647,57 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): marker_start = '# start' marker_end = '# end' - content = textwrap.dedent('''\ - Line 1 of block - Line 2 of block - ''') - without_block = textwrap.dedent('''\ - Hello world! - - # comment here - ''') - with_non_matching_block = textwrap.dedent('''\ - Hello world! - - # start - No match here - # end - # comment here - ''') - with_non_matching_block_and_marker_end_not_after_newline = textwrap.dedent('''\ - Hello world! - - # start - No match here# end - # comment here - ''') - with_matching_block = textwrap.dedent('''\ - Hello world! - - # start - Line 1 of block - Line 2 of block - # end - # comment here - ''') - with_matching_block_and_extra_newline = textwrap.dedent('''\ - Hello world! - - # start - Line 1 of block - Line 2 of block - - # end - # comment here - ''') - with_matching_block_and_marker_end_not_after_newline = textwrap.dedent('''\ - Hello world! - - # start - Line 1 of block - Line 2 of block# end - # comment here - ''') + content = os.linesep.join([ + 'Line 1 of block', + 'Line 2 of block', + '']) + without_block = os.linesep.join([ + 'Hello world!', + '', + '# comment here', + '']) + with_non_matching_block = os.linesep.join([ + 'Hello world!', + '', + '# start', + 'No match here', + '# end', + '# comment here', + '']) + with_non_matching_block_and_marker_end_not_after_newline = os.linesep.join([ + 'Hello world!', + '', + '# start', + 'No match here# end', + '# comment here', + '']) + with_matching_block = os.linesep.join([ + 'Hello world!', + '', + '# start', + 'Line 1 of block', + 'Line 2 of block', + '# end', + '# comment here', + '']) + with_matching_block_and_extra_newline = os.linesep.join([ + 'Hello world!', + '', + '# start', + 'Line 1 of block', + 'Line 2 of block', + '', + '# end', + '# comment here', + '']) + with_matching_block_and_marker_end_not_after_newline = os.linesep.join([ + 'Hello world!', + '', + '# start', + 'Line 1 of block', + 'Line 2 of block# end', + '# comment here', + '']) content_explicit_posix_newlines = ('Line 1 of block\n' 'Line 2 of block\n') content_explicit_windows_newlines = ('Line 1 of block\r\n' From dd4fcd3445b956d53d3ea383a8c9be72e528a27f Mon Sep 17 00:00:00 2001 From: Daniel A Wozniak Date: Tue, 21 Aug 2018 22:29:13 +0000 Subject: [PATCH 33/38] Revert "Multiple block replace test fixes" This reverts commit 94ac2b4fc70d6b2b971c6aafa1f82ea11c983e4e. --- tests/integration/states/test_file.py | 40 +++++++++++++-------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index af633301534..7a22692830f 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -2747,8 +2747,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): Test blockreplace when prepend_if_not_found=True and block doesn't exist in file. ''' - expected = self.marker_start + '\n' + self.content + \ - self.marker_end + '\n' + self.without_block + expected = self.marker_start + os.linesep + self.content + \ + self.marker_end + os.linesep + self.without_block # Pass 1: content ends in newline self._write(name, self.without_block) @@ -2801,8 +2801,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): exist in file. Test with append_newline explicitly set to True. ''' # Pass 1: content ends in newline - expected = self.marker_start + '\n' + self.content + \ - '\n' + self.marker_end + '\n' + self.without_block + expected = self.marker_start + os.linesep + self.content + \ + os.linesep + self.marker_end + os.linesep + self.without_block self._write(name, self.without_block) ret = self.run_state('file.blockreplace', name=name, @@ -2827,8 +2827,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): self.assertEqual(self._read(name), expected) # Pass 2: content does not end in newline - expected = self.marker_start + '\n' + self.content + \ - self.marker_end + '\n' + self.without_block + expected = self.marker_start + os.linesep + self.content + \ + self.marker_end + os.linesep + self.without_block self._write(name, self.without_block) ret = self.run_state('file.blockreplace', name=name, @@ -2859,8 +2859,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): exist in file. Test with append_newline explicitly set to False. ''' # Pass 1: content ends in newline - expected = self.marker_start + '\n' + self.content + \ - self.marker_end + '\n' + self.without_block + expected = self.marker_start + os.linesep + self.content + \ + self.marker_end + os.linesep + self.without_block self._write(name, self.without_block) ret = self.run_state('file.blockreplace', name=name, @@ -2885,8 +2885,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): self.assertEqual(self._read(name), expected) # Pass 2: content does not end in newline - expected = self.marker_start + '\n' + \ - self.content.rstrip('\r\n') + self.marker_end + '\n' + \ + expected = self.marker_start + os.linesep + \ + self.content.rstrip('\r\n') + self.marker_end + os.linesep + \ self.without_block self._write(name, self.without_block) ret = self.run_state('file.blockreplace', @@ -2917,8 +2917,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): Test blockreplace when append_if_not_found=True and block doesn't exist in file. ''' - expected = self.without_block + self.marker_start + '\n' + \ - self.content + self.marker_end + '\n' + expected = self.without_block + self.marker_start + os.linesep + \ + self.content + self.marker_end + os.linesep # Pass 1: content ends in newline self._write(name, self.without_block) @@ -2971,8 +2971,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): exist in file. Test with append_newline explicitly set to True. ''' # Pass 1: content ends in newline - expected = self.without_block + self.marker_start + '\n' + \ - self.content + '\n' + self.marker_end + '\n' + expected = self.without_block + self.marker_start + os.linesep + \ + self.content + os.linesep + self.marker_end + os.linesep self._write(name, self.without_block) ret = self.run_state('file.blockreplace', name=name, @@ -2997,8 +2997,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): self.assertEqual(self._read(name), expected) # Pass 2: content does not end in newline - expected = self.without_block + self.marker_start + '\n' + \ - self.content + self.marker_end + '\n' + expected = self.without_block + self.marker_start + os.linesep + \ + self.content + self.marker_end + os.linesep self._write(name, self.without_block) ret = self.run_state('file.blockreplace', name=name, @@ -3029,8 +3029,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): exist in file. Test with append_newline explicitly set to False. ''' # Pass 1: content ends in newline - expected = self.without_block + self.marker_start + '\n' + \ - self.content + self.marker_end + '\n' + expected = self.without_block + self.marker_start + os.linesep + \ + self.content + self.marker_end + os.linesep self._write(name, self.without_block) ret = self.run_state('file.blockreplace', name=name, @@ -3055,8 +3055,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): self.assertEqual(self._read(name), expected) # Pass 2: content does not end in newline - expected = self.without_block + self.marker_start + '\n' + \ - self.content.rstrip('\r\n') + self.marker_end + '\n' + expected = self.without_block + self.marker_start + os.linesep + \ + self.content.rstrip('\r\n') + self.marker_end + os.linesep self._write(name, self.without_block) ret = self.run_state('file.blockreplace', name=name, From 1bf0b18224b21f4f30c63bbe8ff44bbcd53ca298 Mon Sep 17 00:00:00 2001 From: Daniel A Wozniak Date: Tue, 21 Aug 2018 23:09:00 +0000 Subject: [PATCH 34/38] Better blockfix replace --- tests/integration/states/test_file.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index 7a22692830f..a6bd7c32892 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -5,7 +5,7 @@ Tests for the file state ''' # Import python libs -from __future__ import absolute_import, unicode_literals +from __future__ import absolute_import import errno import glob import logging @@ -2650,12 +2650,12 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): content = os.linesep.join([ 'Line 1 of block', 'Line 2 of block', - '']) + '']).decode('utf-8') without_block = os.linesep.join([ 'Hello world!', '', '# comment here', - '']) + '']).decode('utf-8') with_non_matching_block = os.linesep.join([ 'Hello world!', '', @@ -2663,14 +2663,14 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): 'No match here', '# end', '# comment here', - '']) + '']).decode('utf-8') with_non_matching_block_and_marker_end_not_after_newline = os.linesep.join([ 'Hello world!', '', '# start', 'No match here# end', '# comment here', - '']) + '']).decode('utf-8') with_matching_block = os.linesep.join([ 'Hello world!', '', @@ -2679,7 +2679,7 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): 'Line 2 of block', '# end', '# comment here', - '']) + '']).decode('utf-8') with_matching_block_and_extra_newline = os.linesep.join([ 'Hello world!', '', @@ -2689,7 +2689,7 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): '', '# end', '# comment here', - '']) + '']).decode('utf-8') with_matching_block_and_marker_end_not_after_newline = os.linesep.join([ 'Hello world!', '', @@ -2697,7 +2697,7 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): 'Line 1 of block', 'Line 2 of block# end', '# comment here', - '']) + '']).decode('utf-8') content_explicit_posix_newlines = ('Line 1 of block\n' 'Line 2 of block\n') content_explicit_windows_newlines = ('Line 1 of block\r\n' From 72c37271a526c024645f1fe9818fa475dd1ea94c Mon Sep 17 00:00:00 2001 From: Daniel A Wozniak Date: Wed, 22 Aug 2018 08:04:02 +0000 Subject: [PATCH 35/38] Use six to make sure content is unicode --- tests/integration/states/test_file.py | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/integration/states/test_file.py b/tests/integration/states/test_file.py index a6bd7c32892..b96c34de834 100644 --- a/tests/integration/states/test_file.py +++ b/tests/integration/states/test_file.py @@ -2647,31 +2647,31 @@ class FileTest(ModuleCase, SaltReturnAssertsMixin): class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): marker_start = '# start' marker_end = '# end' - content = os.linesep.join([ + content = six.text_type(os.linesep.join([ 'Line 1 of block', 'Line 2 of block', - '']).decode('utf-8') - without_block = os.linesep.join([ + ''])) + without_block = six.text_type(os.linesep.join([ 'Hello world!', '', '# comment here', - '']).decode('utf-8') - with_non_matching_block = os.linesep.join([ + ''])) + with_non_matching_block = six.text_type(os.linesep.join([ 'Hello world!', '', '# start', 'No match here', '# end', '# comment here', - '']).decode('utf-8') - with_non_matching_block_and_marker_end_not_after_newline = os.linesep.join([ + ''])) + with_non_matching_block_and_marker_end_not_after_newline = six.text_type(os.linesep.join([ 'Hello world!', '', '# start', 'No match here# end', '# comment here', - '']).decode('utf-8') - with_matching_block = os.linesep.join([ + ''])) + with_matching_block = six.text_type(os.linesep.join([ 'Hello world!', '', '# start', @@ -2679,8 +2679,8 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): 'Line 2 of block', '# end', '# comment here', - '']).decode('utf-8') - with_matching_block_and_extra_newline = os.linesep.join([ + ''])) + with_matching_block_and_extra_newline = six.text_type(os.linesep.join([ 'Hello world!', '', '# start', @@ -2689,15 +2689,15 @@ class BlockreplaceTest(ModuleCase, SaltReturnAssertsMixin): '', '# end', '# comment here', - '']).decode('utf-8') - with_matching_block_and_marker_end_not_after_newline = os.linesep.join([ + ''])) + with_matching_block_and_marker_end_not_after_newline = six.text_type(os.linesep.join([ 'Hello world!', '', '# start', 'Line 1 of block', 'Line 2 of block# end', '# comment here', - '']).decode('utf-8') + ''])) content_explicit_posix_newlines = ('Line 1 of block\n' 'Line 2 of block\n') content_explicit_windows_newlines = ('Line 1 of block\r\n' From f518bd3efc7ee13ac482224a0d72b3bef6b43b85 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Wed, 22 Aug 2018 11:06:22 -0500 Subject: [PATCH 36/38] mark orchestration state tests as flaky --- tests/integration/runners/test_state.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/runners/test_state.py b/tests/integration/runners/test_state.py index b8fe24cd850..a66ba542769 100644 --- a/tests/integration/runners/test_state.py +++ b/tests/integration/runners/test_state.py @@ -30,6 +30,7 @@ import salt.utils.event log = logging.getLogger(__name__) +@flaky class StateRunnerTest(ShellCase): ''' Test the state runner. @@ -84,7 +85,6 @@ class StateRunnerTest(ShellCase): assert os.path.exists('/tmp/ewu-2016-12-13') is False assert code != 0 - @flaky def test_orchestrate_target_exists(self): ''' test orchestration when target exists @@ -110,7 +110,6 @@ class StateRunnerTest(ShellCase): for item in out: assert item in ret - @flaky def test_orchestrate_target_doesnt_exist(self): ''' test orchestration when target doesn't exist @@ -190,6 +189,7 @@ class StateRunnerTest(ShellCase): @skipIf(salt.utils.is_windows(), '*NIX-only test') +@flaky class OrchEventTest(ShellCase): ''' Tests for orchestration events From 93a576ef6353ccd68f6daf57cca2574ae1eac6f7 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Wed, 22 Aug 2018 11:07:02 -0500 Subject: [PATCH 37/38] flaky tests are flaky yo --- tests/integration/shell/test_call.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/shell/test_call.py b/tests/integration/shell/test_call.py index 55a23535a31..f13fd40a67f 100644 --- a/tests/integration/shell/test_call.py +++ b/tests/integration/shell/test_call.py @@ -164,6 +164,7 @@ class CallTest(ShellCase, testprogram.TestProgramCase, ShellCaseCommonTestsMixin self.assertTrue(True in ['returnTOmaster' in a for a in master_out]) @skipIf(sys.platform.startswith('win'), 'This test does not apply on Win') + @flaky def test_issue_2731_masterless(self): root_dir = os.path.join(TMP, 'issue-2731') config_dir = os.path.join(root_dir, 'conf') @@ -380,6 +381,7 @@ class CallTest(ShellCase, testprogram.TestProgramCase, ShellCaseCommonTestsMixin if os.path.isdir(config_dir): shutil.rmtree(config_dir) + @flaky def test_issue_15074_output_file_append(self): output_file_append = os.path.join(TMP, 'issue-15074') try: From a3594db10f684e894ea2764f32e129b791bf0699 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Wed, 22 Aug 2018 12:54:00 -0400 Subject: [PATCH 38/38] extend #488588 to cover SyncAuth class the original fix was only covered asyncauth, but the syncauth method overrode the fix. This patch duplicates it so behavior should be consistent in both codepaths. --- salt/crypt.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/salt/crypt.py b/salt/crypt.py index b96d76ff54c..3e0f91ebb6e 100644 --- a/salt/crypt.py +++ b/salt/crypt.py @@ -1097,9 +1097,17 @@ class SAuth(AsyncAuth): creds = self.sign_in(channel=channel) if creds == 'retry': if self.opts.get('caller'): - print('Minion failed to authenticate with the master, ' - 'has the minion key been accepted?') - sys.exit(2) + # We have a list of masters, so we should break + # and try the next one in the list. + if self.opts.get('local_masters', None): + error = SaltClientError('Minion failed to authenticate' + ' with the master, has the ' + 'minion key been accepted?') + break + else: + print('Minion failed to authenticate with the master, ' + 'has the minion key been accepted?') + sys.exit(2) if acceptance_wait_time: log.info('Waiting {0} seconds before retry.'.format(acceptance_wait_time)) time.sleep(acceptance_wait_time)