PR#786: [rpmbuild] clean /var/cache/mock automatically - copr/copr

copr / copr

#786 [rpmbuild] clean /var/cache/mock automatically

Merged 4 years ago by praiskup. Opened 4 years ago by praiskup.

Unknown source clean-var-cache-mock into master

[rpmbuild] clean /var/cache/mock automatically

Pavel Raiskup • 4 years ago

03e8e84

rpmbuild/copr_rpmbuild/builders/mock.py

file modified

+18 -6

		`@@ -9,6 +9,7 @@`

		`log = logging.getLogger("__main__")`

		`+ MOCK_CALL = ['unbuffer', 'mock']`

		`class MockBuilder(object):`
		`def __init__(self, task, sourcedir, resultdir, config):`
		`@@ -34,10 +35,13 @@`

		`spec = locate_spec(self.sourcedir)`
		`shutil.copy(spec, self.resultdir)`
		`- self.produce_srpm(spec, self.sourcedir, configdir, self.resultdir)`
		`+ try:`
		`+ self.produce_srpm(spec, self.sourcedir, configdir, self.resultdir)`

		`- srpm = locate_srpm(self.resultdir)`
		`- self.produce_rpm(srpm, configdir, self.resultdir)`
		`+ srpm = locate_srpm(self.resultdir)`
		`+ self.produce_rpm(srpm, configdir, self.resultdir)`
		`+ finally:`
frostyx commented 4 years ago You are doing try-finally, so you probably expect, that the rpm producing code can fail. What do you think about adding an `except` block and at least logging the exception?
praiskup commented 4 years ago Wouldn't it be propagated up then, and than it would be up to the caller to take care of that? I don't have problem to add except, log something, and re-raise; but it seems to me that we track those exceptions in `main()`.
frostyx commented 4 years ago You are right, I didn't realize that. Scratch that idea. +1 to the PR as it is now
		`+ self.clean_cache(configdir)`

		`def prepare_configs(self, configdir):`
		`site_config_path = os.path.join(configdir, "site-defaults.cfg")`
		`@@ -65,8 +69,7 @@`
		`repos=self.repos, pkg_manager_conf=self.pkg_manager_conf)`

		`def produce_srpm(self, spec, sources, configdir, resultdir):`
		`- cmd = [`
		`- "unbuffer", "/usr/bin/mock",`
		`+ cmd = MOCK_CALL + [`
		`"--buildsrpm",`
		`"--spec", spec,`
		`"--sources", sources,`
		`@@ -94,9 +97,18 @@`
		`if process.returncode != 0:`
		`raise RuntimeError("Build failed")`

		`+ @classmethod`
		`+ def clean_cache(cls, configdir):`
		`+ """ Do best effort /var/mock/cache cleanup. """`
		`+ cmd = MOCK_CALL + [`
		`+ "--configdir", configdir,`
		`+ "-r", "child",`
		`+ "--scrub", "cache",`
		`+ ]`
		`+ subprocess.call(cmd) # ignore failure here, if any`

		`def produce_rpm(self, srpm, configdir, resultdir):`
		`- cmd = ["unbuffer", "/usr/bin/mock",`
		`+ cmd = MOCK_CALL + [`
		`"--rebuild", srpm,`
		`"--configdir", configdir,`
		`"--resultdir", resultdir,`

rpmbuild/tests/test_mock.py

file modified

+1 -1

		`@@ -87,7 +87,7 @@`
		`process = mock.MagicMock(returncode=0)`
		`popen_mock.return_value = process`
		`builder.produce_rpm("/path/to/pkg.src.rpm", "/path/to/configs", "/path/to/results")`
		`- assert_cmd = ['unbuffer', '/usr/bin/mock',`
		`+ assert_cmd = ['unbuffer', 'mock',`
		`'--rebuild', '/path/to/pkg.src.rpm',`
		`'--configdir', '/path/to/configs',`
		`'--resultdir', '/path/to/results',`

praiskup commented 4 years ago

This is to prevent "no space left on device" on / partition.
Since we recycle user-dedicated builder machines heavily, we need
to clean directories like

$ ls -1 /var/cache/mock
BUILD_ID-fedora-rawhide-x86_64
BUILD_ID-fedora-rawhide-x86_64-bootstrap

which contain yum/dnf caches there (soon to be enabled), and
mock's root_cache.

If we don't do this, after several (tens?) of builds on the same
builder, the / partition might be wasted. This is especially
important in Fedora Copr on aarch64 where we have small disks.

praiskup commented 4 years ago

@dmach, thanks for reporting this

rebased onto d745e6fa2137f60be09a2dc02bc3933b57e4892b

4 years ago

rebased onto d3c56360282f445997127d87484128fedc451e49

4 years ago

praiskup commented 4 years ago

[copr-build]

msuchy commented 4 years ago

frostyx commented on line 21 of rpmbuild/copr_rpmbuild/builders/mock.py 4 years ago

You are doing try-finally, so you probably expect, that the rpm producing code can fail. What do you think about adding an except block and at least logging the exception?

frostyx commented 4 years ago

Just a small note, otherwise +1

praiskup commented on line 21 of rpmbuild/copr_rpmbuild/builders/mock.py 4 years ago

Wouldn't it be propagated up then, and than it would be up to the caller to take care of that? I don't have problem to add except, log something, and re-raise; but it seems to me that we track those exceptions in main().

frostyx commented on line 21 of rpmbuild/copr_rpmbuild/builders/mock.py 4 years ago

You are right, I didn't realize that. Scratch that idea.
+1 to the PR as it is now

rebased onto 03e8e84

4 years ago

Pull-Request has been merged by praiskup

4 years ago