SdkService/Create: Added check to avoid creating the chroot when it already exists.
BUG=None
TEST=run_tests
Change-Id: I0b0c2030df7b717e1a0180470afb040f83319aa4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1606650
Tested-by: Alex Klein <saklein@chromium.org>
Reviewed-by: Evan Hernandez <evanhernandez@chromium.org>
Commit-Queue: Dhanya Ganesh <dhanyaganesh@chromium.org>
diff --git a/service/sdk.py b/service/sdk.py
index 612be8d..f0aff89 100644
--- a/service/sdk.py
+++ b/service/sdk.py
@@ -11,6 +11,7 @@
from chromite.lib import constants
from chromite.lib import cros_build_lib
+from chromite.lib import cros_logging as logging
from chromite.lib import cros_sdk_lib
@@ -86,6 +87,10 @@
return args
+ @property
+ def chroot_path(self):
+ return self.paths.chroot_path
+
class UpdateArguments(object):
"""Value object to handle the update arguments."""
@@ -128,12 +133,27 @@
"""
cros_build_lib.AssertOutsideChroot()
+ chroot_exists = os.path.isdir(arguments.chroot_path)
+ version = GetChrootVersion(arguments.chroot_path)
+
+ chroot_broken = chroot_exists and not version
+ if not arguments.replace:
+ # Do some extra checks when we're just creating it.
+ if chroot_broken:
+ # Force replace when we can't get a version for a chroot that exists,
+ # since something must have gone wrong.
+ logging.info('Replacing broken chroot.')
+ arguments.replace = True
+ elif chroot_exists:
+ # It already exists and no need to replace it, we can exit now.
+ return version
+
cmd = [os.path.join(constants.CHROMITE_BIN_DIR, 'cros_sdk')]
cmd.extend(arguments.GetArgList())
cros_build_lib.RunCommand(cmd)
- return GetChrootVersion(arguments.paths.chroot_path)
+ return GetChrootVersion(arguments.chroot_path)
def GetChrootVersion(chroot_path=None):
diff --git a/service/sdk_unittest.py b/service/sdk_unittest.py
index 66cdef7..258dbb4 100644
--- a/service/sdk_unittest.py
+++ b/service/sdk_unittest.py
@@ -7,6 +7,8 @@
from __future__ import print_function
+import os
+
from chromite.lib import cros_build_lib
from chromite.lib import cros_test_lib
from chromite.service import sdk
@@ -82,14 +84,16 @@
self.assertIn(arg, result)
-class CreateTest(cros_test_lib.RunCommandTestCase):
+class CreateTest(cros_test_lib.RunCommandTempDirTestCase):
"""Create function tests."""
def testCreate(self):
"""Test the create function builds the command correctly."""
- arguments = sdk.CreateArguments()
+ arguments = sdk.CreateArguments(replace=True)
+ arguments.paths.chroot_path = os.path.join(self.tempdir, 'chroot')
expected_args = ['--arg', '--other', '--with-value', 'value']
expected_version = 1
+
self.PatchObject(arguments, 'GetArgList', return_value=expected_args)
self.PatchObject(sdk, 'GetChrootVersion', return_value=expected_version)
self.PatchObject(cros_build_lib, 'IsInsideChroot', return_value=False)
@@ -99,6 +103,22 @@
self.assertCommandContains(expected_args)
self.assertEqual(expected_version, version)
+ def testCreateExisting(self):
+ """Test the create doesn't make the call when it already exists."""
+ arguments = sdk.CreateArguments()
+ arguments.paths.chroot_path = self.tempdir
+ expected_args = ['--arg', '--other', '--with-value', 'value']
+ expected_version = 1
+
+ self.PatchObject(arguments, 'GetArgList', return_value=expected_args)
+ self.PatchObject(sdk, 'GetChrootVersion', return_value=expected_version)
+ self.PatchObject(cros_build_lib, 'IsInsideChroot', return_value=False)
+
+ version = sdk.Create(arguments)
+
+ self.assertCommandContains(expected_args, expected=False)
+ self.assertEqual(expected_version, version)
+
def testCreateInsideFails(self):
"""Test Create raises an error when called inside the chroot."""
# Make sure it fails inside the chroot.
