Repository :
http://git.fedorahosted.org/cgit/copr.git
On branch : master
---------------------------------------------------------------
commit d20dc04e32784d938d471705ab65b71d2127864e
Author: Valentin Gologuzov <vgologuz(a)redhat.com>
Date: Fri Jan 30 14:00:07 2015 +0100
[backend] [rhbz:#1171796] copr sometimes doesn't delete build from repository
---------------------------------------------------------------
backend/backend/actions.py | 61 ++++---
backend/tests/_resources/1171796.tar.gz | Bin 0 -> 587365 bytes
backend/tests/_resources/1171796_doubled.tar.gz | Bin 0 -> 1172212 bytes
backend/tests/test_action.py | 217 ++++++++++++++++++-----
frontend/run_tests.sh | 4 +-
5 files changed, 207 insertions(+), 75 deletions(-)
diff --git a/backend/backend/actions.py b/backend/backend/actions.py
index 4ae9c7c..f1d4557 100644
--- a/backend/backend/actions.py
+++ b/backend/backend/actions.py
@@ -39,6 +39,9 @@ class Action(object):
self.front_url = front_url
self.results_root_url = results_root_url
+ def __str__(self):
+ return "<Action: {}>".format(self.data)
+
def add_event(self, what):
self.events.put({"when": time.time(), "who":
"action", "what": what})
@@ -101,6 +104,7 @@ class Action(object):
ext_data = json.loads(self.data["data"])
username = ext_data["username"]
projectname = ext_data["projectname"]
+ chroots_requested = set(ext_data["chroots"])
packages = [os.path.basename(x).replace(".src.rpm", "")
for x in ext_data["pkgs"].split()]
@@ -111,34 +115,31 @@ class Action(object):
self.add_event("Copr path {0}".format(path))
try:
- chroot_list = os.listdir(path)
+ chroot_list = set(os.listdir(path))
except OSError:
# already deleted
- chroot_list = []
+ chroot_list = set()
+
+ chroots_to_do = chroot_list.intersection(chroots_requested)
+ if not chroots_to_do:
+ self.add_event("Nothing to delete for delete action: packages {},
{}"
+ .format(packages, ext_data))
+ return
- for chroot in chroot_list:
+ for chroot in chroots_to_do:
self.add_event("In chroot {0}".format(chroot))
altered = False
- # We need to delete the files only if they belong
- # to the build. For example if my build fails and I send
- # fixed pkg with the same version again, it succeeds and
- # than I delete the failed, it would delete the succeeded
- # files as well - that would be wrong.
for pkg in packages:
- if self.data["object_type"] == "build-succeeded" or
(
- self.data["object_type"] == "build-failed"
and
- os.path.exists(os.path.join(path, chroot, pkg,
"fail"))):
-
- pkg_path = os.path.join(path, chroot, pkg)
- if os.path.isdir(pkg_path):
- self.add_event("Removing build {0}".format(pkg_path))
- shutil.rmtree(pkg_path)
- altered = True
- else:
- self.add_event(
- "Package {0} dir not found in chroot {1}"
- .format(pkg, chroot))
+ pkg_path = os.path.join(path, chroot, pkg)
+ if os.path.isdir(pkg_path):
+ self.add_event("Removing build {0}".format(pkg_path))
+ shutil.rmtree(pkg_path)
+ altered = True
+ else:
+ self.add_event(
+ "Package {0} dir not found in chroot {1}"
+ .format(pkg, chroot))
if altered:
self.add_event("Running createrepo")
@@ -154,13 +155,14 @@ class Action(object):
self.add_event(
"Error making local repo: {0}".format(err))
- log_path = os.path.join(
- path, chroot,
- 'build-{0}.log'.format(self.data['object_id']))
-
- if os.path.isfile(log_path):
- self.add_event("Removing log {0}".format(log_path))
- os.unlink(log_path)
+ logs_to_remove = [
+ os.path.join(path, chroot,
template.format(self.data['object_id']))
+ for template in ['build-{}.log', 'build-{}.rsync.log']
+ ]
+ for log_path in logs_to_remove:
+ if os.path.isfile(log_path):
+ self.add_event("Removing log {0}".format(log_path))
+ os.remove(log_path)
def run(self):
""" Handle action (other then builds) - like rename or delete of
project """
@@ -172,8 +174,7 @@ class Action(object):
if action_type == ActionType.DELETE:
if self.data["object_type"] == "copr":
self.handle_delete_copr_project()
- elif self.data["object_type"] in \
- ["build-succeeded", "build-skipped",
"build-failed"]:
+ elif self.data["object_type"] == "build":
self.handle_delete_build()
result.result = ActionResult.SUCCESS
diff --git a/backend/tests/_resources/1171796.tar.gz
b/backend/tests/_resources/1171796.tar.gz
new file mode 100644
index 0000000..6d3b522
Binary files /dev/null and b/backend/tests/_resources/1171796.tar.gz differ
diff --git a/backend/tests/_resources/1171796_doubled.tar.gz
b/backend/tests/_resources/1171796_doubled.tar.gz
new file mode 100644
index 0000000..6cd04f3
Binary files /dev/null and b/backend/tests/_resources/1171796_doubled.tar.gz differ
diff --git a/backend/tests/test_action.py b/backend/tests/test_action.py
index 24e1367..3e7ce91 100644
--- a/backend/tests/test_action.py
+++ b/backend/tests/test_action.py
@@ -3,6 +3,7 @@ import json
import tempfile
import shutil
import time
+import tarfile
import pytest
import six
@@ -60,7 +61,8 @@ class TestAction(object):
self.ext_data_for_delete_build = json.dumps({
"pkgs": " ".join(self.pkgs),
"username": "foo",
- "projectname": "bar"
+ "projectname": "bar",
+ "chroots": ["fedora20", "epel7"]
})
def teardown_method(self, method):
@@ -83,6 +85,16 @@ class TestAction(object):
return self.tmp_dir_name
+ def unpack_resource(self, resource_name):
+ if self.tmp_dir_name is None:
+ self.make_temp_dir()
+
+ src_path = os.path.join(os.path.dirname(__file__),
+ "_resources", resource_name)
+
+ with tarfile.open(src_path, "r:gz") as tfile:
+ tfile.extractall(os.path.join(self.tmp_dir_name, "old_dir"))
+
def test_action_event(self, mc_time):
test_action = Action(
events=self.test_q,
@@ -308,40 +320,23 @@ class TestAction(object):
tmp_dir = self.make_temp_dir()
- for obj_type in ["build-succeeded", "build-skipped",
"build-failed"]:
- test_action = Action(
- action={
- "action_type": ActionType.DELETE,
- "object_type": obj_type,
- "id": 7,
- "old_value": "not-existing-project",
- "data": self.ext_data_for_delete_build,
- },
- events=self.test_q, lock=None,
- frontend_callback=mc_front_cb,
- destdir=tmp_dir,
- front_url=None,
- results_root_url=RESULTS_ROOT_URL
- )
- with mock.patch("backend.actions.shutil") as mc_shutil:
- test_action.run()
- assert not mc_shutil.rmtree.called
-
- ev_1 = self.test_q.get_nowait()
- assert "Action delete build" == ev_1["what"]
- assert ev_1["who"] == "action"
-
- ev_2 = self.test_q.get_nowait()
- assert "Packages to delete" in ev_2["what"]
- assert " ".join(self.pkgs_stripped) in ev_2["what"]
- assert ev_2["who"] == "action"
-
- ev_3 = self.test_q.get_nowait()
- assert "Copr path" in ev_3["what"]
- assert ev_3["who"] == "action"
-
- with pytest.raises(EmptyQueue):
- self.test_q.get_nowait()
+ test_action = Action(
+ action={
+ "action_type": ActionType.DELETE,
+ "object_type": "build",
+ "id": 7,
+ "old_value": "not-existing-project",
+ "data": self.ext_data_for_delete_build,
+ },
+ events=self.test_q, lock=None,
+ frontend_callback=mc_front_cb,
+ destdir=tmp_dir,
+ front_url=None,
+ results_root_url=RESULTS_ROOT_URL
+ )
+ with mock.patch("backend.actions.shutil") as mc_shutil:
+ test_action.run()
+ assert not mc_shutil.rmtree.called
@mock.patch("backend.actions.createrepo")
def test_delete_build_succeeded(self, mc_createrepo, mc_time):
@@ -370,7 +365,7 @@ class TestAction(object):
test_action = Action(
action={
"action_type": ActionType.DELETE,
- "object_type": "build-succeeded",
+ "object_type": "build",
"id": 7,
"old_value": "old_dir",
"data": self.ext_data_for_delete_build,
@@ -458,11 +453,11 @@ class TestAction(object):
test_action = Action(
action={
"action_type": ActionType.DELETE,
- "object_type": "build-succeeded",
+ "object_type": "build",
"id": 7,
"old_value": "old_dir",
"data": self.ext_data_for_delete_build,
- "object_id": 42
+ "object_id": 42,
},
events=self.test_q, lock=None,
frontend_callback=mc_front_cb,
@@ -481,11 +476,147 @@ class TestAction(object):
assert error_event_recorded
+ @mock.patch("backend.actions.createrepo")
+ def test_delete_two_chroots(self, mc_createrepo_unsafe, mc_time):
+ """
+ Regression test,
https://bugzilla.redhat.com/show_bug.cgi?id=1171796
+
+ """
+ mc_createrepo_unsafe.return_value = 0, STDOUT, ""
+
+ resource_name = "1171796.tar.gz"
+ self.unpack_resource(resource_name)
+
+ chroot_20_path = os.path.join(self.tmp_dir_name, "old_dir",
"fedora-20-x86_64")
+ chroot_21_path = os.path.join(self.tmp_dir_name, "old_dir",
"fedora-21-x86_64")
+
+ assert os.path.exists(os.path.join(chroot_20_path, "build-15.log"))
+ assert os.path.exists(os.path.join(chroot_21_path, "build-15.log"))
+
+ assert os.path.exists(os.path.join(chroot_20_path,
"build-15.rsync.log"))
+ assert os.path.exists(os.path.join(chroot_21_path,
"build-15.rsync.log"))
+
+ assert os.path.isdir(os.path.join(chroot_20_path,
"rubygem-log4r-1.1.10-2.fc21"))
+ assert os.path.isdir(os.path.join(chroot_21_path,
"rubygem-log4r-1.1.10-2.fc21"))
+
+ mc_time.time.return_value = self.test_time
+ mc_front_cb = MagicMock()
+
+ test_action = Action(
+ action={
+ "action_type": ActionType.DELETE,
+ "object_type": "build",
+ "object_id": 15,
+ "id": 15,
+ "old_value": "old_dir",
+ "data": json.dumps({
+ "pkgs": "rubygem-log4r-1.1.10-2.fc21.src.rpm",
+ "username": "foo",
+ "projectname": "bar",
+ "chroots": ["fedora-20-x86_64",
"fedora-21-x86_64"]
+ }),
+ },
+ events=self.test_q, lock=None,
+ frontend_callback=mc_front_cb,
+ destdir=self.tmp_dir_name,
+ front_url=None,
+ results_root_url=RESULTS_ROOT_URL
+ )
+ test_action.run()
+
+ assert not os.path.exists(os.path.join(chroot_20_path,
"build-15.log"))
+ assert not os.path.exists(os.path.join(chroot_21_path,
"build-15.log"))
+
+ assert not os.path.exists(os.path.join(chroot_20_path,
"build-15.rsync.log"))
+ assert not os.path.exists(os.path.join(chroot_21_path,
"build-15.rsync.log"))
+
+ assert not os.path.isdir(os.path.join(chroot_20_path,
"rubygem-log4r-1.1.10-2.fc21"))
+ assert not os.path.isdir(os.path.join(chroot_21_path,
"rubygem-log4r-1.1.10-2.fc21"))
+
+ @mock.patch("backend.actions.createrepo")
+ def test_delete_two_chroots_two_remains(self, mc_createrepo_unsafe, mc_time):
+ """
+ Regression test,
https://bugzilla.redhat.com/show_bug.cgi?id=1171796
+ extended: we also put two more chroots, which should be unaffected
+ """
+ mc_createrepo_unsafe.return_value = 0, STDOUT, ""
+
+ resource_name = "1171796_doubled.tar.gz"
+ self.unpack_resource(resource_name)
+
+ chroot_20_path = os.path.join(self.tmp_dir_name, "old_dir",
"fedora-20-x86_64")
+ chroot_21_path = os.path.join(self.tmp_dir_name, "old_dir",
"fedora-21-x86_64")
+ chroot_20_i386_path = os.path.join(self.tmp_dir_name, "old_dir",
"fedora-20-i386")
+ chroot_21_i386_path = os.path.join(self.tmp_dir_name, "old_dir",
"fedora-21-i386")
+
+ assert os.path.exists(os.path.join(chroot_20_path, "build-15.log"))
+ assert os.path.exists(os.path.join(chroot_21_path, "build-15.log"))
+ assert os.path.exists(os.path.join(chroot_20_i386_path,
"build-15.log"))
+ assert os.path.exists(os.path.join(chroot_21_i386_path,
"build-15.log"))
+
+ assert os.path.exists(os.path.join(chroot_20_path,
"build-15.rsync.log"))
+ assert os.path.exists(os.path.join(chroot_21_path,
"build-15.rsync.log"))
+ assert os.path.exists(os.path.join(chroot_20_i386_path,
"build-15.rsync.log"))
+ assert os.path.exists(os.path.join(chroot_21_i386_path,
"build-15.rsync.log"))
+
+ assert os.path.isdir(os.path.join(chroot_20_path,
"rubygem-log4r-1.1.10-2.fc21"))
+ assert os.path.isdir(os.path.join(chroot_21_path,
"rubygem-log4r-1.1.10-2.fc21"))
+ assert os.path.isdir(os.path.join(chroot_20_i386_path,
"rubygem-log4r-1.1.10-2.fc21"))
+ assert os.path.isdir(os.path.join(chroot_21_i386_path,
"rubygem-log4r-1.1.10-2.fc21"))
+
+ mc_time.time.return_value = self.test_time
+ mc_front_cb = MagicMock()
+
+ test_action = Action(
+ action={
+ "action_type": ActionType.DELETE,
+ "object_type": "build",
+ "object_id": 15,
+ "id": 15,
+ "old_value": "old_dir",
+ "data": json.dumps({
+ "pkgs": "rubygem-log4r-1.1.10-2.fc21.src.rpm",
+ "username": "foo",
+ "projectname": "bar",
+ "chroots": ["fedora-20-x86_64",
"fedora-21-x86_64"]
+ }),
+ },
+ events=self.test_q, lock=None,
+ frontend_callback=mc_front_cb,
+ destdir=self.tmp_dir_name,
+ front_url=None,
+ results_root_url=RESULTS_ROOT_URL
+ )
+ test_action.run()
+
+ assert not os.path.exists(os.path.join(chroot_20_path,
"build-15.log"))
+ assert not os.path.exists(os.path.join(chroot_21_path,
"build-15.log"))
+ assert os.path.exists(os.path.join(chroot_20_i386_path,
"build-15.log"))
+ assert os.path.exists(os.path.join(chroot_21_i386_path,
"build-15.log"))
+
+ assert not os.path.exists(os.path.join(chroot_20_path,
"build-15.rsync.log"))
+ assert not os.path.exists(os.path.join(chroot_21_path,
"build-15.rsync.log"))
+ assert os.path.exists(os.path.join(chroot_20_i386_path,
"build-15.rsync.log"))
+ assert os.path.exists(os.path.join(chroot_21_i386_path,
"build-15.rsync.log"))
+
+ assert not os.path.isdir(os.path.join(chroot_20_path,
"rubygem-log4r-1.1.10-2.fc21"))
+ assert not os.path.isdir(os.path.join(chroot_21_path,
"rubygem-log4r-1.1.10-2.fc21"))
+ assert os.path.isdir(os.path.join(chroot_20_i386_path,
"rubygem-log4r-1.1.10-2.fc21"))
+ assert os.path.isdir(os.path.join(chroot_21_i386_path,
"rubygem-log4r-1.1.10-2.fc21"))
+
+ @mock.patch("backend.actions.createrepo_unsafe")
+ def test_delete_two_chroots_two_builds_stay_untouched(self, mc_createrepo_unsafe,
mc_time):
+ # TODO: prepare archive
+ """
+ Before: 2 builds of the same package-version, all using different chroot
+ """
+ pass
+
@mock.patch("backend.actions.createrepo_unsafe")
- def test_handle_createrepo_ok(self, mc_createrepo_unsfe, mc_time):
+ def test_handle_createrepo_ok(self, mc_createrepo_unsafe, mc_time):
mc_front_cb = MagicMock()
tmp_dir = self.make_temp_dir()
- mc_createrepo_unsfe.return_value = 0, STDOUT, ""
+ mc_createrepo_unsafe.return_value = 0, STDOUT, ""
action_data = json.dumps({
"chroots": ["epel-6-i386",
"fedora-20-x86_64"],
@@ -513,9 +644,9 @@ class TestAction(object):
exp_call_1 = mock.call(lock=None, path=tmp_dir +
u'/foo/bar/epel-6-i386')
exp_call_2 = mock.call(lock=None, path=tmp_dir +
u'/foo/bar/fedora-20-x86_64')
- assert exp_call_1 in mc_createrepo_unsfe.call_args_list
- assert exp_call_2 in mc_createrepo_unsfe.call_args_list
- assert len(mc_createrepo_unsfe.call_args_list) == 2
+ assert exp_call_1 in mc_createrepo_unsafe.call_args_list
+ assert exp_call_2 in mc_createrepo_unsafe.call_args_list
+ assert len(mc_createrepo_unsafe.call_args_list) == 2
@mock.patch("backend.actions.createrepo_unsafe")
diff --git a/frontend/run_tests.sh b/frontend/run_tests.sh
index 41e390e..2ecd9b9 100755
--- a/frontend/run_tests.sh
+++ b/frontend/run_tests.sh
@@ -1,6 +1,6 @@
#!/bin/sh
cd coprs_frontend
-COPR_CONFIG="$(pwd)/config/copr_unit_test.conf" python -m pytest tests \
- --cov-report term-missing --cov coprs $@
+COPR_CONFIG="$(pwd)/config/copr_unit_test.conf" python -m pytest tests -s \
+ --cov-report term-missing --cov coprs $@