Revert "cbuildbot_launch: Remove legacy cleanup from cache"
This reverts commit 207e2a1879c3ab0469e70f1f25debe7537f1e7fa.
Reason for revert: Sync works but now running into some other issue with performing a chown on the new mount cache. This is pointless as legacy is just a mousetrap of potential failures.
Original change's description:
> 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>
Bug: b:186752777
Change-Id: Iac4e11de7c4d9b4b9693f236ea2cf01acf48fb3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2909924
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
diff --git a/scripts/cbuildbot_launch_unittest.py b/scripts/cbuildbot_launch_unittest.py
index 24e6f31..8a9a6ff 100644
--- a/scripts/cbuildbot_launch_unittest.py
+++ b/scripts/cbuildbot_launch_unittest.py
@@ -9,6 +9,7 @@
import os
import sys
+import time
from chromite.cbuildbot import commands
from chromite.cbuildbot import repository
@@ -78,7 +79,8 @@
cbuildbot_launch.InitialCheckout(mock_repo, options)
self.assertEqual(mock_repo.mock_calls, [
- mock.call.Sync(jobs=20, detach=True, downgrade_repo=False),
+ mock.call.PreLoad('/preload/chromeos'),
+ mock.call.Sync(detach=True, downgrade_repo=False),
])
def testConfigureGlobalEnvironment(self):
@@ -148,10 +150,12 @@
constants.REEXEC_API_MINOR))
mock_repo = mock.MagicMock()
mock_repo.branch = 'main'
- mock_repo.directory = '/root'
+ mock_repo.directory = '/root/repository'
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',
@@ -163,16 +167,20 @@
build_number=0, master_build_id=0, status=mock.ANY,
buildroot_layout=2, branch='main')
- argv = ['-r', '/root', 'config',
- '--workspace', '/root/workspace']
+ argv = ['-r', '/root', 'config']
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',
+ [mock.call(EXPECTED_MANIFEST_URL, '/root/repository',
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)])
@@ -180,17 +188,16 @@
# Ensure we invoke cbuildbot, as expected.
self.assertCommandCalled(
[
- '/root/chromite/bin/cbuildbot',
+ '/root/repository/chromite/bin/cbuildbot',
'config',
- '-r', '/root',
+ '-r', '/root/repository',
'--workspace', '/root/workspace',
- '--workspace', '/root/workspace',
- '--cache-dir', '/root/.cache',
+ '--cache-dir', '/root/repository/.cache',
# The duplication is a bug, but not harmful.
- '--cache-dir', '/root/.cache',
+ '--cache-dir', '/root/repository/.cache',
],
extra_env={'PATH': mock.ANY},
- cwd='/root',
+ cwd='/root/repository',
check=False)
# Ensure we saved the final state, as expected.
@@ -200,7 +207,7 @@
mock.call('/root', expected_build_state)])
# Ensure we clean the chroot, as expected.
- mock_cleanup_chroot.assert_called_once_with('/root')
+ mock_cleanup_chroot.assert_called_once_with('/root/repository')
def testMainMax(self):
"""Test a larger set of command line options."""
@@ -210,7 +217,7 @@
constants.REEXEC_API_MINOR))
mock_repo = mock.MagicMock()
mock_repo.branch = 'branch'
- mock_repo.directory = '/root'
+ mock_repo.directory = '/root/repository'
mock_summary = build_summary.BuildSummary(
build_number=313,
@@ -224,6 +231,8 @@
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',
@@ -234,7 +243,6 @@
'--branch', 'branch',
'--git-cache-dir', '/git-cache',
'--cache-dir', '/cache',
- '--workspace', '/root/workspace',
'--remote-trybot',
'--master-build-id', '123456789',
'--buildnumber', '314',
@@ -244,13 +252,26 @@
# Did we create the repo instance correctly?
self.assertEqual(mock_repo_create.mock_calls,
- [mock.call(EXPECTED_MANIFEST_URL, '/root',
+ [mock.call(EXPECTED_MANIFEST_URL, '/root/repository',
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)])
@@ -258,13 +279,12 @@
# Ensure we invoke cbuildbot, as expected.
self.assertCommandCalled(
[
- '/root/chromite/bin/cbuildbot',
+ '/root/repository/chromite/bin/cbuildbot',
'config',
- '--buildroot', '/root',
+ '--buildroot', '/root/repository',
'--branch', 'branch',
'--git-cache-dir', '/git-cache',
'--cache-dir', '/cache',
- '--workspace', '/root/workspace',
'--remote-trybot',
'--master-build-id', '123456789',
'--buildnumber', '314',
@@ -276,7 +296,7 @@
'--cache-dir', '/cache',
],
extra_env={'PATH': mock.ANY},
- cwd='/root',
+ cwd='/root/repository',
check=False)
# Ensure we write the final build state, as expected.
@@ -290,7 +310,7 @@
mock.call('/root', final_state)])
# Ensure we clean the chroot, as expected.
- mock_cleanup_chroot.assert_called_once_with('/root')
+ mock_cleanup_chroot.assert_called_once_with('/root/repository')
class CleanBuildRootTest(cros_test_lib.MockTempDirTestCase):
@@ -321,6 +341,324 @@
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([