diff --git a/.ci/lint b/.ci/lint index 8adbbece69..25efc251d0 100644 --- a/.ci/lint +++ b/.ci/lint @@ -3,7 +3,7 @@ pipeline { options { timestamps() ansiColor('xterm') - timeout(time: 1, unit: 'HOURS') + timeout(time: 3, unit: 'HOURS') } environment { PYENV_ROOT = "/usr/local/pyenv" @@ -14,7 +14,7 @@ pipeline { stage('github-pending') { steps { githubNotify credentialsId: 'test-jenkins-credentials', - description: 'Testing lint...', + description: 'Python lint on changes...', status: 'PENDING', context: "jenkins/pr/lint" } @@ -24,12 +24,14 @@ pipeline { sh ''' # Need -M to detect renames otherwise they are reported as Delete and Add, need -C to detect copies, -C includes -M # -M is on by default in git 2.9+ - git diff --name-status -l99999 -C "origin/$CHANGE_TARGET" "origin/$BRANCH_NAME" > file-list-status.log + git diff --name-status -l99999 -C "origin/$CHANGE_TARGET" > file-list-status.log # the -l increase the search limit, lets use awk so we do not need to repeat the search above. gawk 'BEGIN {FS="\\t"} {if ($1 != "D") {print $NF}}' file-list-status.log > file-list-changed.log gawk 'BEGIN {FS="\\t"} {if ($1 == "D") {print $NF}}' file-list-status.log > file-list-deleted.log - (git diff --name-status -l99999 -C "origin/$CHANGE_TARGET";echo "---";git diff --name-status -l99999 -C "origin/$BRANCH_NAME";printenv|grep -E '=[0-9a-z]{40,}+$|COMMIT=|BRANCH') > file-list-experiment.log + (git diff --name-status -l99999 -C "origin/$CHANGE_TARGET" "origin/$BRANCH_NAME";echo "---";git diff --name-status -l99999 -C "origin/$BRANCH_NAME";printenv|grep -E '=[0-9a-z]{40,}+$|COMMIT=|BRANCH') > file-list-experiment.log touch pylint-report-salt.log pylint-report-tests.log + echo 254 > pylint-salt-chg.exit # assume failure + echo 254 > pylint-tests-chg.exit # assume failure eval "$(pyenv init -)" pyenv --version pyenv install --skip-existing 2.7.14 @@ -41,52 +43,117 @@ pipeline { archiveArtifacts artifacts: 'file-list-status.log,file-list-changed.log,file-list-deleted.log,file-list-experiment.log' } } - stage('linting') { - failFast false + stage('linting chg') { parallel { - stage('salt linting') { + stage('lint salt') { when { expression { return readFile('file-list-changed.log') =~ /(?i)(^|\n)(salt\/.*\.py|setup\.py)\n/ } } steps { sh ''' eval "$(pyenv init - --no-rehash)" - grep -Ei '^salt/.*\\.py$|^setup\\.py$' file-list-changed.log | xargs -r '--delimiter=\\n' tox -e pylint-salt | tee pylint-report-salt.log + # tee makes the exit/return code always 0 + grep -Ei '^salt/.*\\.py$|^setup\\.py$' file-list-changed.log | (xargs -r '--delimiter=\\n' tox -e pylint-salt;echo "$?" > pylint-salt-chg.exit) | tee pylint-report-salt.log # remove color escape coding sed -ri 's/\\x1B\\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g' pylint-report-salt.log + read rc_exit < pylint-salt-chg.exit + exit "$rc_exit" ''' archiveArtifacts artifacts: 'pylint-report-salt.log' } } - stage('test linting') { + stage('lint test') { when { expression { return readFile('file-list-changed.log') =~ /(?i)(^|\n)tests\/.*\.py\n/ } } steps { sh ''' eval "$(pyenv init - --no-rehash)" - grep -Ei '^tests/.*\\.py$' file-list-changed.log | xargs -r '--delimiter=\\n' tox -e pylint-tests | tee pylint-report-tests.log + # tee makes the exit/return code always 0 + grep -Ei '^tests/.*\\.py$' file-list-changed.log | (xargs -r '--delimiter=\\n' tox -e pylint-tests;echo "$?" > pylint-tests-chg.exit) | tee pylint-report-tests.log # remove color escape coding sed -ri 's/\\x1B\\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g' pylint-report-tests.log + read rc_exit < pylint-tests-chg.exit + exit "$rc_exit" ''' archiveArtifacts artifacts: 'pylint-report-tests.log' } } } + post { + always { + step([$class: 'WarningsPublisher', + parserConfigurations: [[ + parserName: 'PyLint', + pattern: 'pylint-report*chg.log' + ]], + failedTotalAll: '0', + useDeltaValues: false, + canRunOnFailed: true, + usePreviousBuildAsReference: true + ]) + } + } + } + stage('lint all') { + // perform a full linit if this is a merge forward and the change only lint passed. + when { + expression { return params.BRANCH_NAME =~ /(?i)^merge-/ && readFile('file-list-changed.log') =~ /^0/ } + } + parallel { + stage('begin') { + steps { + githubNotify credentialsId: 'test-jenkins-credentials', + description: 'Python lint on everything...', + status: 'PENDING', + context: "jenkins/pr/lint" + } + } + stage('lint salt') { + steps { + sh ''' + eval "$(pyenv init - --no-rehash)" + (tox -e pylint-salt ; echo "$?" > pylint-salt-full.exit) | tee pylint-report-salt-full.log + # remove color escape coding + sed -ri 's/\\x1B\\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g' pylint-report-salt-full.log + read rc_exit < pylint-salt-full.exit + exit "$rc_exit" + ''' + archiveArtifacts artifacts: 'pylint-report-salt-full.log' + } + } + stage('lint test') { + steps { + sh ''' + eval "$(pyenv init - --no-rehash)" + (tox -e pylint-tests ; echo "$?" > pylint-tests-full.exit) | tee pylint-report-tests-full.log + # remove color escape coding + sed -ri 's/\\x1B\\[([0-9]{1,2}(;[0-9]{1,2})?)?[m|K]//g' pylint-report-tests-full.log + read rc_exit < pylint-tests-full.exit + exit "$rc_exit" + ''' + archiveArtifacts artifacts: 'pylint-report-tests-full.log' + } + } + } + post { + always { + step([$class: 'WarningsPublisher', + parserConfigurations: [[ + parserName: 'PyLint', + pattern: 'pylint-report*full.log' + ]], + failedTotalAll: '0', + useDeltaValues: false, + canRunOnFailed: true, + usePreviousBuildAsReference: true + ]) + } + } } } post { always { - step([$class: 'WarningsPublisher', - parserConfigurations: [[ - parserName: 'PyLint', - pattern: 'pylint-report*.log' - ]], - failedTotalAll: '0', - useDeltaValues: false, - canRunOnFailed: true, - usePreviousBuildAsReference: true - ]) cleanWs() } success { @@ -97,7 +164,7 @@ pipeline { } failure { githubNotify credentialsId: 'test-jenkins-credentials', - description: 'The lint job has failed', + description: 'The lint test has failed', status: 'FAILURE', context: "jenkins/pr/lint" slackSend channel: "#jenkins-prod-pr", diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 0a8f989d83..b8f19f3b21 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -13,6 +13,7 @@ salt/*/*boto* @saltstack/team-boto # Team Core requirements/* @saltstack/team-core +rfcs/* @saltstack/team-core salt/auth/* @saltstack/team-core salt/cache/* @saltstack/team-core salt/cli/* @saltstack/team-core @@ -73,3 +74,6 @@ salt/modules/reg.py @saltstack/team-windows salt/states/reg.py @saltstack/team-windows tests/*/*win* @saltstack/team-windows tests/*/test_reg.py @saltstack/team-windows + +# Jenkins Integration +.ci/* @saltstack/saltstack-release-engineering @saltstack/team-core @saltstack/team-windows diff --git a/salt/modules/file.py b/salt/modules/file.py index 91d362ffef..1a88e1ed21 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -2277,6 +2277,8 @@ def replace(path, # Just search; bail as early as a match is found if re.search(cpattern, r_data): return True # `with` block handles file closure + else: + return False else: result, nrepl = re.subn(cpattern, repl.replace('\\', '\\\\') if backslash_literal else repl, diff --git a/salt/states/gem.py b/salt/states/gem.py index 5a715c8b63..c8a45b8d79 100644 --- a/salt/states/gem.py +++ b/salt/states/gem.py @@ -16,6 +16,9 @@ you can specify what ruby version and gemset to target. ''' from __future__ import absolute_import, unicode_literals, print_function +import salt.utils + +import re import logging log = logging.getLogger(__name__) @@ -84,10 +87,29 @@ def installed(name, # pylint: disable=C0103 'Use of argument ruby found, but neither rvm or rbenv is installed' ) gems = __salt__['gem.list'](name, ruby, gem_bin=gem_bin, runas=user) - if name in gems and version is not None and str(version) in gems[name]: - ret['result'] = True - ret['comment'] = 'Gem is already installed.' - return ret + if name in gems and version is not None: + match = re.match(r'(>=|>|<|<=)', version) + if match: + # Grab the comparison + cmpr = match.group() + + # Clear out 'default:' and any whitespace + installed_version = re.sub('default: ', '', gems[name][0]).strip() + + # Clear out comparison from version and whitespace + desired_version = re.sub(cmpr, '', version).strip() + + if salt.utils.compare_versions(installed_version, + cmpr, + desired_version): + ret['result'] = True + ret['comment'] = 'Installed Gem meets version requirements.' + return ret + else: + if str(version) in gems[name]: + ret['result'] = True + ret['comment'] = 'Gem is already installed.' + return ret elif name in gems and version is None: ret['result'] = True ret['comment'] = 'Gem is already installed.' diff --git a/salt/states/supervisord.py b/salt/states/supervisord.py index 4dcd7beb76..2bc8a31cca 100644 --- a/salt/states/supervisord.py +++ b/salt/states/supervisord.py @@ -338,6 +338,7 @@ def dead(name, else: # process name doesn't exist ret['comment'] = "Service {0} doesn't exist".format(name) + return ret if is_stopped is True: ret['comment'] = "Service {0} is not running".format(name) diff --git a/tests/unit/modules/test_file.py b/tests/unit/modules/test_file.py index 8cc355e3ff..e1df401956 100644 --- a/tests/unit/modules/test_file.py +++ b/tests/unit/modules/test_file.py @@ -226,6 +226,22 @@ class FileReplaceTestCase(TestCase, LoaderModuleMockMixin): ''' filemod.replace(self.tfile.name, r'Etiam', 123) + def test_search_only_return_true(self): + ret = filemod.replace(self.tfile.name, + r'Etiam', 'Salticus', + search_only=True) + + self.assertIsInstance(ret, bool) + self.assertEqual(ret, True) + + def test_search_only_return_false(self): + ret = filemod.replace(self.tfile.name, + r'Etian', 'Salticus', + search_only=True) + + self.assertIsInstance(ret, bool) + self.assertEqual(ret, False) + class FileBlockReplaceTestCase(TestCase, LoaderModuleMockMixin): def setup_loader_modules(self): diff --git a/tests/unit/states/test_gem.py b/tests/unit/states/test_gem.py index 83d4e6a861..5ae4214665 100644 --- a/tests/unit/states/test_gem.py +++ b/tests/unit/states/test_gem.py @@ -47,6 +47,19 @@ class TestGemState(TestCase, LoaderModuleMockMixin): ri=False, gem_bin=None ) + def test_installed_version(self): + gems = {'foo': ['1.0'], 'bar': ['2.0']} + gem_list = MagicMock(return_value=gems) + gem_install_succeeds = MagicMock(return_value=True) + + with patch.dict(gem.__salt__, {'gem.list': gem_list}): + with patch.dict(gem.__salt__, + {'gem.install': gem_install_succeeds}): + ret = gem.installed('foo', version='>= 1.0') + self.assertEqual(True, ret['result']) + self.assertEqual('Installed Gem meets version requirements.', + ret['comment']) + def test_removed(self): gems = ['foo', 'bar'] gem_list = MagicMock(return_value=gems) diff --git a/tests/unit/transport/test_zeromq.py b/tests/unit/transport/test_zeromq.py index a118b7cd82..6b55416dce 100644 --- a/tests/unit/transport/test_zeromq.py +++ b/tests/unit/transport/test_zeromq.py @@ -31,6 +31,7 @@ import salt.config import salt.log.setup from salt.ext import six import salt.utils.process +import salt.utils.platform import salt.transport.server import salt.transport.client import salt.exceptions @@ -434,7 +435,7 @@ class PubServerChannel(TestCase, AdaptedConfigurationTestCaseMixin): results.append(payload['jid']) return results - @skipIf(salt.utils.is_windows(), 'Skip on Windows OS') + @skipIf(salt.utils.platform.is_windows(), 'Skip on Windows OS') def test_publish_to_pubserv_ipc(self): ''' Test sending 10K messags to ZeroMQPubServerChannel using IPC transport @@ -534,7 +535,7 @@ class PubServerChannel(TestCase, AdaptedConfigurationTestCaseMixin): server_channel.pub_close() assert len(results) == send_num, (len(results), set(expect).difference(results)) - @skipIf(salt.utils.is_windows(), 'Skip on Windows OS') + @skipIf(salt.utils.platform.is_windows(), 'Skip on Windows OS') def test_issue_36469_udp(self): ''' Test sending both large and small messags to publisher using UDP