From bb6898929cd3b9cb4089d901db4b7c7cdf04b497 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Mon, 17 Jun 2019 08:31:38 +0200 Subject: [PATCH] Check tags in track and team repos With this commit Rally does not only check for branch matches but also for matching tags in track and team repos. This allows to "freeze" support for older versions of Elasticsearch, i.e. once we drop support for Elasticsearch 1.x, we can tag the branch `1` in rally-teams and rally-tracks as `v1` (using a `v` prefix as convention for tags). Rally can then still pick up configurations for these older versions that our out of maintenance. --- esrally/utils/git.py | 27 +++++++++++++++++--------- esrally/utils/repo.py | 23 ++++++++++++++++++++-- tests/utils/git_test.py | 42 ++++++++++++++++++++++++++++++---------- tests/utils/repo_test.py | 42 +++++++++++++++++++++++++++++++++++++--- 4 files changed, 110 insertions(+), 24 deletions(-) diff --git a/esrally/utils/git.py b/esrally/utils/git.py index c038e8a1e..7b89492fc 100644 --- a/esrally/utils/git.py +++ b/esrally/utils/git.py @@ -57,7 +57,7 @@ def clone(src, remote): def fetch(src, remote="origin"): # Don't swallow output but silence git at least a bit... (--quiet) if process.run_subprocess( - "git -C {0} fetch --prune --quiet {1}".format(src, remote)): + "git -C {0} fetch --prune --prune-tags --tags --quiet {1}".format(src, remote)): raise exceptions.SupplyError("Could not fetch source tree from [%s]" % remote) @@ -65,7 +65,7 @@ def fetch(src, remote="origin"): def checkout(src_dir, branch="master"): if process.run_subprocess( "git -C {0} checkout --quiet {1}".format(src_dir, branch)): - raise exceptions.SupplyError("Could not checkout branch [%s]. Do you have uncommitted changes?" % branch) + raise exceptions.SupplyError("Could not checkout [%s]. Do you have uncommitted changes?" % branch) @probed @@ -83,17 +83,17 @@ def pull(src_dir, remote="origin", branch="master"): @probed def pull_ts(src_dir, ts): - if process.run_subprocess( - "git -C {0} fetch --prune --quiet origin && git -C {0} checkout --quiet `git -C {0} rev-list -n 1 --before=\"{1}\" " - "--date=iso8601 origin/master`".format(src_dir, ts)): - raise exceptions.SupplyError("Could not fetch source tree for timestamped revision [%s]" % ts) + fetch(src_dir) + if process.run_subprocess("git -C {0} checkout --quiet `git -C {0} rev-list -n 1 --before=\"{1}\" " + "--date=iso8601 origin/master`".format(src_dir, ts)): + raise exceptions.SupplyError("Could not checkout source tree for timestamped revision [%s]" % ts) @probed def pull_revision(src_dir, revision): - if process.run_subprocess( - "git -C {0} fetch --prune --quiet origin && git -C {0} checkout --quiet {1}".format(src_dir, revision)): - raise exceptions.SupplyError("Could not fetch source tree for revision [%s]" % revision) + fetch(src_dir) + if process.run_subprocess("git -C {0} checkout --quiet {1}".format(src_dir, revision)): + raise exceptions.SupplyError("Could not checkout source tree for revision [%s]" % revision) @probed @@ -118,9 +118,18 @@ def branches(src_dir, remote=True): "git -C {src} for-each-ref refs/heads/ --format='%(refname:short)'".format(src=src_dir))) +@probed +def tags(src_dir): + return _cleanup_tag_names(process.run_subprocess_with_output("git -C {0} tag".format(src_dir))) + + def _cleanup_remote_branch_names(branch_names): return [(b[b.index("/") + 1:]).strip() for b in branch_names if not b.endswith("/HEAD")] def _cleanup_local_branch_names(branch_names): return [b.strip() for b in branch_names if not b.endswith("HEAD")] + + +def _cleanup_tag_names(tag_names): + return [t.strip() for t in tag_names] diff --git a/esrally/utils/repo.py b/esrally/utils/repo.py index 4ad0d5557..4b0fde5cb 100644 --- a/esrally/utils/repo.py +++ b/esrally/utils/repo.py @@ -79,14 +79,33 @@ def update(self, distribution_version): branch = versions.best_match(git.branches(self.repo_dir, remote=False), distribution_version) if branch: if git.current_branch(self.repo_dir) != branch: - self.logger.info("Checking out [%s] in [%s] for distribution version [%s].", branch, self.repo_dir, distribution_version) + self.logger.info("Checking out [%s] in [%s] for distribution version [%s].", + branch, self.repo_dir, distribution_version) git.checkout(self.repo_dir, branch=branch) else: - raise exceptions.SystemSetupError("Cannot find %s for distribution version %s" % (self.resource_name, distribution_version)) + self.logger.info("No local branch found for distribution version [%s] in [%s]. Checking tags.", + distribution_version, self.repo_dir) + tag = self._find_matching_tag(distribution_version) + if tag: + self.logger.info("Checking out tag [%s] in [%s] for distribution version [%s].", + tag, self.repo_dir, distribution_version) + git.checkout(self.repo_dir, branch=tag) + else: + raise exceptions.SystemSetupError("Cannot find %s for distribution version %s" + % (self.resource_name, distribution_version)) except exceptions.SupplyError as e: tb = sys.exc_info()[2] raise exceptions.DataError("Cannot update %s in [%s] (%s)." % (self.resource_name, self.repo_dir, e.message)).with_traceback(tb) + def _find_matching_tag(self, distribution_version): + tags = git.tags(self.repo_dir) + for version in versions.versions(distribution_version): + # tags have a "v" prefix by convention. + tag_candidate = "v{}".format(version) + if tag_candidate in tags: + return tag_candidate + return None + def checkout(self, revision): self.logger.info("Checking out revision [%s] in [%s].", revision, self.repo_dir) git.checkout(self.repo_dir, revision) diff --git a/tests/utils/git_test.py b/tests/utils/git_test.py index 2a46421fd..8bf820053 100644 --- a/tests/utils/git_test.py +++ b/tests/utils/git_test.py @@ -75,7 +75,7 @@ def test_fetch_successful(self, run_subprocess_with_logging, run_subprocess): run_subprocess_with_logging.return_value = 0 run_subprocess.return_value = False git.fetch("/src", remote="my-origin") - run_subprocess.assert_called_with("git -C /src fetch --prune --quiet my-origin") + run_subprocess.assert_called_with("git -C /src fetch --prune --prune-tags --tags --quiet my-origin") @mock.patch("esrally.utils.process.run_subprocess") @mock.patch("esrally.utils.process.run_subprocess_with_logging") @@ -85,7 +85,7 @@ def test_fetch_with_error(self, run_subprocess_with_logging, run_subprocess): with self.assertRaises(exceptions.SupplyError) as ctx: git.fetch("/src", remote="my-origin") self.assertEqual("Could not fetch source tree from [my-origin]", ctx.exception.args[0]) - run_subprocess.assert_called_with("git -C /src fetch --prune --quiet my-origin") + run_subprocess.assert_called_with("git -C /src fetch --prune --prune-tags --tags --quiet my-origin") @mock.patch("esrally.utils.process.run_subprocess") @mock.patch("esrally.utils.process.run_subprocess_with_logging") @@ -102,7 +102,7 @@ def test_checkout_with_error(self, run_subprocess_with_logging, run_subprocess): run_subprocess.return_value = True with self.assertRaises(exceptions.SupplyError) as ctx: git.checkout("/src", "feature-branch") - self.assertEqual("Could not checkout branch [feature-branch]. Do you have uncommitted changes?", ctx.exception.args[0]) + self.assertEqual("Could not checkout [feature-branch]. Do you have uncommitted changes?", ctx.exception.args[0]) run_subprocess.assert_called_with("git -C /src checkout --quiet feature-branch") @mock.patch("esrally.utils.process.run_subprocess") @@ -124,7 +124,7 @@ def test_pull(self, run_subprocess_with_logging, run_subprocess): run_subprocess.return_value = False git.pull("/src", remote="my-origin", branch="feature-branch") calls = [ - mock.call("git -C /src fetch --prune --quiet my-origin"), + mock.call("git -C /src fetch --prune --prune-tags --tags --quiet my-origin"), mock.call("git -C /src checkout --quiet feature-branch"), mock.call("git -C /src rebase --quiet my-origin/feature-branch") ] @@ -134,19 +134,24 @@ def test_pull(self, run_subprocess_with_logging, run_subprocess): @mock.patch("esrally.utils.process.run_subprocess_with_logging") def test_pull_ts(self, run_subprocess_with_logging, run_subprocess): run_subprocess_with_logging.return_value = 0 - run_subprocess.return_value = False + run_subprocess.side_effect = [False, False] git.pull_ts("/src", "20160101T110000Z") - run_subprocess.assert_called_with( - "git -C /src fetch --prune --quiet origin && git -C /src checkout " - "--quiet `git -C /src rev-list -n 1 --before=\"20160101T110000Z\" --date=iso8601 origin/master`") + run_subprocess.has_calls([ + mock.call("git -C /src fetch --prune --prune-tags --tags --quiet origin"), + mock.call("git -C /src checkout --quiet `git -C /src rev-list -n 1 --before=\"20160101T110000Z\" " + "--date=iso8601 origin/master`"), + ]) @mock.patch("esrally.utils.process.run_subprocess") @mock.patch("esrally.utils.process.run_subprocess_with_logging") def test_pull_revision(self, run_subprocess_with_logging, run_subprocess): run_subprocess_with_logging.return_value = 0 - run_subprocess.return_value = False + run_subprocess.side_effect = [False, False] git.pull_revision("/src", "3694a07") - run_subprocess.assert_called_with("git -C /src fetch --prune --quiet origin && git -C /src checkout --quiet 3694a07") + run_subprocess.has_calls([ + mock.call("git -C /src fetch --prune --prune-tags --tags --quiet origin"), + mock.call("git -C /src checkout --quiet 3694a07"), + ]) @mock.patch("esrally.utils.process.run_subprocess_with_output") @mock.patch("esrally.utils.process.run_subprocess_with_logging") @@ -177,3 +182,20 @@ def test_list_local_branches(self, run_subprocess_with_logging, run_subprocess): " 5"] self.assertEqual(["master", "5.0.0-alpha1", "5"], git.branches("/src", remote=False)) run_subprocess.assert_called_with("git -C /src for-each-ref refs/heads/ --format='%(refname:short)'") + + @mock.patch("esrally.utils.process.run_subprocess_with_output") + @mock.patch("esrally.utils.process.run_subprocess_with_logging") + def test_list_tags_with_tags_present(self, run_subprocess_with_logging, run_subprocess): + run_subprocess_with_logging.return_value = 0 + run_subprocess.return_value = [" v1", + " v2"] + self.assertEqual(["v1", "v2"], git.tags("/src")) + run_subprocess.assert_called_with("git -C /src tag") + + @mock.patch("esrally.utils.process.run_subprocess_with_output") + @mock.patch("esrally.utils.process.run_subprocess_with_logging") + def test_list_tags_no_tags_available(self, run_subprocess_with_logging, run_subprocess): + run_subprocess_with_logging.return_value = 0 + run_subprocess.return_value = "" + self.assertEqual([], git.tags("/src")) + run_subprocess.assert_called_with("git -C /src tag") diff --git a/tests/utils/repo_test.py b/tests/utils/repo_test.py index a4409f932..958927475 100644 --- a/tests/utils/repo_test.py +++ b/tests/utils/repo_test.py @@ -183,11 +183,40 @@ def test_updates_locally(self, curr_branch, rebase, checkout, branches, fetch, i @mock.patch("esrally.utils.git.is_working_copy", autospec=True) @mock.patch("esrally.utils.git.fetch", autospec=True) + @mock.patch("esrally.utils.git.tags", autospec=True) + @mock.patch("esrally.utils.git.branches", autospec=True) + @mock.patch("esrally.utils.git.checkout", autospec=True) + @mock.patch("esrally.utils.git.rebase") + @mock.patch("esrally.utils.git.current_branch") + def test_fallback_to_tags(self, curr_branch, rebase, checkout, branches, tags, fetch, is_working_copy): + curr_branch.return_value = "master" + branches.return_value = ["5", "master"] + tags.return_value = ["v1", "v1.7", "v2"] + is_working_copy.return_value = True + + r = repo.RallyRepository( + remote_url=None, + root_dir="/rally-resources", + repo_name="unit-test", + resource_name="unittest-resources", + offline=False) + + r.update(distribution_version="1.7.4") + + branches.assert_called_with("/rally-resources/unit-test", remote=False) + self.assertEqual(0, rebase.call_count) + tags.assert_called_with("/rally-resources/unit-test") + checkout.assert_called_with("/rally-resources/unit-test", branch="v1.7") + + @mock.patch("esrally.utils.git.is_working_copy", autospec=True) + @mock.patch("esrally.utils.git.fetch", autospec=True) + @mock.patch("esrally.utils.git.tags", autospec=True) @mock.patch("esrally.utils.git.branches", autospec=True) @mock.patch("esrally.utils.git.checkout") @mock.patch("esrally.utils.git.rebase") - def test_does_not_update_unknown_branch_remotely(self, rebase, checkout, branches, fetch, is_working_copy): + def test_does_not_update_unknown_branch_remotely(self, rebase, checkout, branches, tags, fetch, is_working_copy): branches.return_value = ["1", "2", "5", "master"] + tags.return_value = [] is_working_copy.return_value = True r = repo.RallyRepository( @@ -212,19 +241,23 @@ def test_does_not_update_unknown_branch_remotely(self, rebase, checkout, branche ] branches.assert_has_calls(calls) + tags.assert_called_with("/rally-resources/unit-test") self.assertEqual(0, checkout.call_count) self.assertEqual(0, rebase.call_count) @mock.patch("esrally.utils.git.is_working_copy", autospec=True) @mock.patch("esrally.utils.git.fetch", autospec=True) + @mock.patch("esrally.utils.git.tags", autospec=True) @mock.patch("esrally.utils.git.branches", autospec=True) @mock.patch("esrally.utils.git.checkout", autospec=True) @mock.patch("esrally.utils.git.rebase") @mock.patch("esrally.utils.git.current_branch") - def test_does_not_update_unknown_branch_remotely_local_fallback(self, curr_branch, rebase, checkout, branches, fetch, is_working_copy): + def test_does_not_update_unknown_branch_remotely_local_fallback(self, curr_branch, rebase, checkout, branches, tags, + fetch, is_working_copy): curr_branch.return_value = "master" # we have only "master" remotely but a few more branches locally branches.side_effect = ["5", ["1", "2", "5", "master"]] + tags.return_value = [] is_working_copy.return_value = True r = repo.RallyRepository( @@ -244,16 +277,19 @@ def test_does_not_update_unknown_branch_remotely_local_fallback(self, curr_branc ] branches.assert_has_calls(calls) + self.assertEqual(0, tags.call_count) checkout.assert_called_with("/rally-resources/unit-test", branch="1") self.assertEqual(0, rebase.call_count) @mock.patch("esrally.utils.git.is_working_copy", autospec=True) @mock.patch("esrally.utils.git.fetch", autospec=True) + @mock.patch("esrally.utils.git.tags", autospec=True) @mock.patch("esrally.utils.git.branches", autospec=True) @mock.patch("esrally.utils.git.checkout") @mock.patch("esrally.utils.git.rebase") - def test_does_not_update_unknown_branch_locally(self, rebase, checkout, branches, fetch, is_working_copy): + def test_does_not_update_unknown_branch_locally(self, rebase, checkout, branches, tags, fetch, is_working_copy): branches.return_value = ["1", "2", "5", "master"] + tags.return_value = [] is_working_copy.return_value = True r = repo.RallyRepository(