cbuildbot_launch: Remove legacy cleanup from cache
With the move to an overlayfs approach, and no named cache, we can
remove the cleanup that is a bit too aggressive and results in emptying
the directory completely, which in turn requires a full sync on every
bot.
BUG=b:186752777
TEST=`run_tests`
Cq-Depend: chromium:2911033
Change-Id: Iede42c22f681fbc2205893f86a942496ac08ea1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2911118
Reviewed-by: LaMont Jones <lamontjones@chromium.org>
Tested-by: Mike Nichols <mikenichols@chromium.org>
Commit-Queue: Mike Nichols <mikenichols@chromium.org>
diff --git a/scripts/cbuildbot_launch_unittest.py b/scripts/cbuildbot_launch_unittest.py
index 8a9a6ff..24e6f31 100644
--- a/scripts/cbuildbot_launch_unittest.py
+++ b/scripts/cbuildbot_launch_unittest.py
@@ -9,7 +9,6 @@
import os
import sys
-import time
from chromite.cbuildbot import commands
from chromite.cbuildbot import repository
@@ -79,8 +78,7 @@
cbuildbot_launch.InitialCheckout(mock_repo, options)
self.assertEqual(mock_repo.mock_calls, [
- mock.call.PreLoad('/preload/chromeos'),
- mock.call.Sync(detach=True, downgrade_repo=False),
+ mock.call.Sync(jobs=20, detach=True, downgrade_repo=False),
])
def testConfigureGlobalEnvironment(self):
@@ -150,12 +148,10 @@
constants.REEXEC_API_MINOR))
mock_repo = mock.MagicMock()
mock_repo.branch = 'main'
- mock_repo.directory = '/root/repository'
+ mock_repo.directory = '/root'
mock_repo_create = self.PatchObject(repository, 'RepoRepository',
autospec=True, return_value=mock_repo)
- mock_clean = self.PatchObject(cbuildbot_launch, 'CleanBuildRoot',
- autospec=True)
mock_checkout = self.PatchObject(cbuildbot_launch, 'InitialCheckout',
autospec=True)
mock_cleanup_chroot = self.PatchObject(cbuildbot_launch, 'CleanupChroot',
@@ -167,20 +163,16 @@
build_number=0, master_build_id=0, status=mock.ANY,
buildroot_layout=2, branch='main')
- argv = ['-r', '/root', 'config']
+ argv = ['-r', '/root', 'config',
+ '--workspace', '/root/workspace']
options = cbuildbot_launch.PreParseArguments(argv)
cbuildbot_launch._main(options, argv)
# Did we create the repo instance correctly?
self.assertEqual(mock_repo_create.mock_calls,
- [mock.call(EXPECTED_MANIFEST_URL, '/root/repository',
+ [mock.call(EXPECTED_MANIFEST_URL, '/root',
git_cache_dir=None, branch='main')])
- # Ensure we clean, as expected.
- self.assertEqual(mock_clean.mock_calls, [
- mock.call('/root', mock_repo, '/root/repository/.cache',
- expected_build_state)])
-
# Ensure we checkout, as expected.
self.assertEqual(mock_checkout.mock_calls,
[mock.call(mock_repo, options)])
@@ -188,16 +180,17 @@
# Ensure we invoke cbuildbot, as expected.
self.assertCommandCalled(
[
- '/root/repository/chromite/bin/cbuildbot',
+ '/root/chromite/bin/cbuildbot',
'config',
- '-r', '/root/repository',
+ '-r', '/root',
'--workspace', '/root/workspace',
- '--cache-dir', '/root/repository/.cache',
+ '--workspace', '/root/workspace',
+ '--cache-dir', '/root/.cache',
# The duplication is a bug, but not harmful.
- '--cache-dir', '/root/repository/.cache',
+ '--cache-dir', '/root/.cache',
],
extra_env={'PATH': mock.ANY},
- cwd='/root/repository',
+ cwd='/root',
check=False)
# Ensure we saved the final state, as expected.
@@ -207,7 +200,7 @@
mock.call('/root', expected_build_state)])
# Ensure we clean the chroot, as expected.
- mock_cleanup_chroot.assert_called_once_with('/root/repository')
+ mock_cleanup_chroot.assert_called_once_with('/root')
def testMainMax(self):
"""Test a larger set of command line options."""
@@ -217,7 +210,7 @@
constants.REEXEC_API_MINOR))
mock_repo = mock.MagicMock()
mock_repo.branch = 'branch'
- mock_repo.directory = '/root/repository'
+ mock_repo.directory = '/root'
mock_summary = build_summary.BuildSummary(
build_number=313,
@@ -231,8 +224,6 @@
return_value=mock_summary)
mock_repo_create = self.PatchObject(repository, 'RepoRepository',
autospec=True, return_value=mock_repo)
- mock_clean = self.PatchObject(cbuildbot_launch, 'CleanBuildRoot',
- autospec=True)
mock_checkout = self.PatchObject(cbuildbot_launch, 'InitialCheckout',
autospec=True)
mock_cleanup_chroot = self.PatchObject(cbuildbot_launch, 'CleanupChroot',
@@ -243,6 +234,7 @@
'--branch', 'branch',
'--git-cache-dir', '/git-cache',
'--cache-dir', '/cache',
+ '--workspace', '/root/workspace',
'--remote-trybot',
'--master-build-id', '123456789',
'--buildnumber', '314',
@@ -252,26 +244,13 @@
# Did we create the repo instance correctly?
self.assertEqual(mock_repo_create.mock_calls,
- [mock.call(EXPECTED_MANIFEST_URL, '/root/repository',
+ [mock.call(EXPECTED_MANIFEST_URL, '/root',
git_cache_dir='/git-cache', branch='branch')])
# Ensure we look up the previous status.
self.assertEqual(mock_get_last_build_state.mock_calls, [
mock.call('/root')])
- # Ensure we clean, as expected.
- self.assertEqual(mock_clean.mock_calls, [
- mock.call('/root',
- mock_repo,
- '/cache',
- build_summary.BuildSummary(
- build_number=314,
- master_build_id=123456789,
- status=mock.ANY,
- branch='branch',
- buildroot_layout=2
- ))])
-
# Ensure we checkout, as expected.
self.assertEqual(mock_checkout.mock_calls,
[mock.call(mock_repo, options)])
@@ -279,12 +258,13 @@
# Ensure we invoke cbuildbot, as expected.
self.assertCommandCalled(
[
- '/root/repository/chromite/bin/cbuildbot',
+ '/root/chromite/bin/cbuildbot',
'config',
- '--buildroot', '/root/repository',
+ '--buildroot', '/root',
'--branch', 'branch',
'--git-cache-dir', '/git-cache',
'--cache-dir', '/cache',
+ '--workspace', '/root/workspace',
'--remote-trybot',
'--master-build-id', '123456789',
'--buildnumber', '314',
@@ -296,7 +276,7 @@
'--cache-dir', '/cache',
],
extra_env={'PATH': mock.ANY},
- cwd='/root/repository',
+ cwd='/root',
check=False)
# Ensure we write the final build state, as expected.
@@ -310,7 +290,7 @@
mock.call('/root', final_state)])
# Ensure we clean the chroot, as expected.
- mock_cleanup_chroot.assert_called_once_with('/root/repository')
+ mock_cleanup_chroot.assert_called_once_with('/root')
class CleanBuildRootTest(cros_test_lib.MockTempDirTestCase):
@@ -341,324 +321,6 @@
for f in (self.repo, self.chroot, self.general, self.distfiles):
osutils.Touch(f, makedirs=True)
- def testNoBuildroot(self):
- """Test CleanBuildRoot with no history."""
- self.mock_repo.branch = 'main'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='main')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'main')
- self.assertIsNotNone(new_summary.distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- self.assertExists(self.previous_build_state)
-
- def testBuildrootNoState(self):
- """Test CleanBuildRoot with no state information."""
- self.populateBuildroot()
- self.mock_repo.branch = 'main'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='main')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'main')
- self.assertIsNotNone(new_summary.distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- self.assertNotExists(self.repo)
- self.assertNotExists(self.chroot)
- self.assertNotExists(self.general)
- self.assertNotExists(self.distfiles)
- self.assertExists(self.previous_build_state)
-
- def testBuildrootFormatMismatch(self):
- """Test CleanBuildRoot with buildroot layout mismatch."""
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_PASSED,
- buildroot_layout=1,
- branch='main')
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'main'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='main')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'main')
- self.assertIsNotNone(new_summary.distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- self.assertNotExists(self.repo)
- self.assertNotExists(self.chroot)
- self.assertNotExists(self.general)
- self.assertNotExists(self.distfiles)
- self.assertExists(self.previous_build_state)
-
- def testBuildrootBranchChange(self):
- """Test CleanBuildRoot with a change in branches."""
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_PASSED,
- buildroot_layout=2,
- branch='branchA')
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchB'
- m = self.PatchObject(cros_sdk_lib, 'CleanupChrootMount')
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchB')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'branchB')
- self.assertIsNotNone(new_summary.distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- # self.assertExists(self.repo)
- self.assertExists(self.general)
- self.assertNotExists(self.distfiles)
- self.assertExists(self.previous_build_state)
- m.assert_called_with(self.chroot, delete=True)
-
- def testBuildrootBranchMatch(self):
- """Test CleanBuildRoot with no change in branch."""
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_PASSED,
- buildroot_layout=2,
- branch='branchA')
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchA'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchA')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'branchA')
- self.assertIsNotNone(new_summary.distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- self.assertExists(self.repo)
- self.assertExists(self.chroot)
- self.assertExists(self.general)
- self.assertExists(self.distfiles)
- self.assertExists(self.previous_build_state)
-
- def testBuildrootGitLocksPrevPass(self):
- """Verify not CleanStaleLocks, if previous build was in passed."""
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_PASSED,
- buildroot_layout=2,
- branch='branchA')
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchA'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchA')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- self.assertEqual(
- self.mock_repo.mock_calls, [
- mock.call.PreLoad(),
- mock.call.BuildRootGitCleanup(prune_all=True),
- ])
-
- def testBuildrootGitLocksPrevFail(self):
- """Verify not CleanStaleLocks, if previous build was in failed."""
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_FAILED,
- buildroot_layout=2,
- branch='branchA')
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchA'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchA')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- self.assertEqual(
- self.mock_repo.mock_calls, [
- mock.call.PreLoad(),
- mock.call.BuildRootGitCleanup(prune_all=True),
- ])
-
- def testBuildrootGitLocksPrevInFlight(self):
- """Verify CleanStaleLocks, if previous build was in flight."""
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=2,
- branch='branchA')
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchA'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchA')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
-
- self.assertEqual(
- self.mock_repo.method_calls, [
- mock.call.PreLoad(),
- mock.call.CleanStaleLocks(),
- mock.call.BuildRootGitCleanup(prune_all=True),
- ])
-
- def testBuildrootDistfilesRecentCache(self):
- """Test CleanBuildRoot does not delete distfiles when cache is recent."""
- seed_distfiles_ts = time.time() - 60
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_PASSED,
- buildroot_layout=2,
- branch='branchA',
- distfiles_ts=seed_distfiles_ts)
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchA'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchA')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'branchA')
- # Same cache creation timestamp is rewritten to state.
- self.assertEqual(new_summary.distfiles_ts, seed_distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- self.assertExists(self.repo)
- self.assertExists(self.chroot)
- self.assertExists(self.general)
- self.assertExists(self.distfiles)
- self.assertExists(self.previous_build_state)
-
- def testBuildrootDistfilesCacheExpired(self):
- """Test CleanBuildRoot when the distfiles cache is too old."""
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_PASSED,
- buildroot_layout=2,
- branch='branchA',
- distfiles_ts=100.0)
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchA'
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchA')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'branchA')
- self.assertIsNotNone(new_summary.distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- self.assertExists(self.repo)
- self.assertExists(self.chroot)
- self.assertExists(self.general)
- self.assertNotExists(self.distfiles)
- self.assertExists(self.previous_build_state)
-
- def testRootOwnedCache(self):
- """Test CleanBuildRoot with no history."""
- seed_distfiles_ts = time.time() - 60
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_PASSED,
- buildroot_layout=2,
- branch='branchA',
- distfiles_ts=seed_distfiles_ts)
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchA'
-
- osutils.Chown(self.cache, 'root', 'root')
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchA')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'branchA')
- # Same cache creation timestamp is rewritten to state.
- self.assertEqual(new_summary.distfiles_ts, seed_distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- self.assertExists(self.repo)
- self.assertExists(self.chroot)
- self.assertExists(self.general)
- self.assertNotExists(self.distfiles)
- self.assertExists(self.previous_build_state)
-
- def testBuildrootRepoCleanFailure(self):
- """Test CleanBuildRoot with repo checkout failure."""
- old_build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_PASSED,
- buildroot_layout=1,
- branch='branchA')
- self.populateBuildroot(previous_build_state=old_build_state.to_json())
- self.mock_repo.branch = 'branchA'
- self.mock_repo.BuildRootGitCleanup.side_effect = Exception
-
- build_state = build_summary.BuildSummary(
- status=constants.BUILDER_STATUS_INFLIGHT,
- buildroot_layout=cbuildbot_launch.BUILDROOT_BUILDROOT_LAYOUT,
- branch='branchA')
- cbuildbot_launch.CleanBuildRoot(
- self.root, self.mock_repo, self.cache, build_state)
-
- new_summary = cbuildbot_launch.GetLastBuildState(self.root)
- self.assertEqual(new_summary.buildroot_layout, 2)
- self.assertEqual(new_summary.branch, 'branchA')
- self.assertIsNotNone(new_summary.distfiles_ts)
- self.assertEqual(new_summary, build_state)
-
- self.assertNotExists(self.repo)
- self.assertNotExists(self.chroot)
- self.assertNotExists(self.general)
- self.assertNotExists(self.distfiles)
- self.assertExists(self.previous_build_state)
-
def testGetCurrentBuildStateNoArgs(self):
"""Tests GetCurrentBuildState without arguments."""
options = cbuildbot_launch.PreParseArguments([