toolchain_util: Add function to modify kernel afdo metadata
Update kernel json metadata file in bundle function, as described
in go/cros-recipe-builder-commits.
BUG=chromium:1081332
TEST=unittest
Change-Id: I8c0ab0482cf97326e88b6aab28af09c17833f651
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/2606285
Commit-Queue: Tiancong Wang <tcwang@google.com>
Tested-by: Tiancong Wang <tcwang@google.com>
Reviewed-by: LaMont Jones <lamontjones@chromium.org>
diff --git a/lib/toolchain_util.py b/lib/toolchain_util.py
index a5aaf04..ca24cf6 100644
--- a/lib/toolchain_util.py
+++ b/lib/toolchain_util.py
@@ -79,8 +79,8 @@
BZ2_COMPRESSION_SUFFIX = '.bz2'
XZ_COMPRESSION_SUFFIX = '.xz'
KERNEL_AFDO_COMPRESSION_SUFFIX = '.gcov.xz'
-TOOLCHAIN_UTILS_PATH = os.path.join(
- constants.SOURCE_ROOT, 'src/third_party/toolchain-utils')
+TOOLCHAIN_UTILS_PATH = os.path.join(constants.SOURCE_ROOT,
+ 'src/third_party/toolchain-utils')
AFDO_PROFILE_PATH_IN_CHROMIUM = 'src/chromeos/profiles/%s.afdo.newest.txt'
MERGED_AFDO_NAME = 'chromeos-chrome-amd64-%s'
@@ -2344,16 +2344,19 @@
if not kernel_version:
raise BundleArtifactsHandlerError('kernel_version not provided.')
kernel_version = kernel_version.replace('.', '_')
- profile_name = self._GetArtifactVersionInEbuild(
- 'chromeos-kernel-%s' % kernel_version,
- 'AFDO_PROFILE_VERSION') + KERNEL_AFDO_COMPRESSION_SUFFIX
+ profile_version = self._GetArtifactVersionInEbuild(
+ f'chromeos-kernel-{kernel_version}', 'AFDO_PROFILE_VERSION')
+ profile_name = profile_version + KERNEL_AFDO_COMPRESSION_SUFFIX
# The verified profile is in the sysroot with a name similar to:
# /usr/lib/debug/boot/chromeos-kernel-4_4-R82-12874.0-1581935639.gcov.xz
profile_path = os.path.join(
self.chroot.path, self.sysroot_path[1:], 'usr', 'lib', 'debug', 'boot',
- 'chromeos-kernel-%s-%s' % (kernel_version, profile_name))
+ f'chromeos-kernel-{kernel_version}-{profile_name}')
verified_profile = os.path.join(self.output_dir, profile_name)
shutil.copy2(profile_path, verified_profile)
+
+ # Also change the kernel JSON file, in order for PUpr to pick up the change
+ self._UpdateKernelMetadata(kernel_version, profile_version)
return [verified_profile]
def _BundleUnverifiedChromeCwpAfdoFile(self):
@@ -2375,6 +2378,28 @@
XZ_COMPRESSION_SUFFIX)
@staticmethod
+ def _UpdateKernelMetadata(kernel_version: str, profile_version: str):
+ """Update afdo_metadata json file"""
+ json_file = os.path.join(TOOLCHAIN_UTILS_PATH, 'afdo_metadata',
+ 'kernel_afdo.json')
+ afdo_versions = json.loads(osutils.ReadFile(json_file))
+ assert kernel_version in afdo_versions, \
+ f'To update {kernel_version}, the entry should be in kernel_afdo.json'
+ old_value = afdo_versions[kernel_version]['name']
+ update_to_newer_profile = _RankValidCWPProfiles(
+ old_value) < _RankValidCWPProfiles(profile_version)
+ # This function is called by Bundle, so normally the profile is newer
+ # is guaranteed because Bundle function only runs when a new profile is
+ # needed to verify at the beginning of the builder. This check is to
+ # make sure there's no other updates happen between the start of the
+ # builder and the time of this function call.
+ assert update_to_newer_profile, (
+ f'Failed to update JSON file because {profile_version} is not '
+ f'newer than {old_value}')
+ afdo_versions[kernel_version]['name'] = profile_version
+ pformat.json(afdo_versions, fp=json_file)
+
+ @staticmethod
def _ListTransitiveFiles(base_directory: str):
for dir_path, _dir_names, file_names in os.walk(base_directory):
for file_name in file_names:
diff --git a/lib/toolchain_util_unittest.py b/lib/toolchain_util_unittest.py
index 2873ac9..8bdde76 100644
--- a/lib/toolchain_util_unittest.py
+++ b/lib/toolchain_util_unittest.py
@@ -528,7 +528,10 @@
self.artifact_type = 'Unspecified'
self.outdir = None
self.afdo_tmp_path = None
- self.profile_info = {}
+ self.kernel_version = '4_4'
+ self.profile_info = {
+ 'kernel_version': self.kernel_version.replace('_', '.'),
+ }
self.orderfile_name = (
'chromeos-chrome-orderfile-field-78-3877.0-1567418235-'
'benchmark-78.0.3893.0-r1.orderfile')
@@ -537,6 +540,7 @@
self.debug_binary_name = 'chromeos-chrome-amd64-78.0.3893.0_rc-r1.debug'
self.merged_afdo_name = (
'chromeos-chrome-amd64-78.0.3893.0_rc-r1-merged.afdo')
+ self.kernel_name = 'R89-13638.0-1607337135'
self.gen_order = self.PatchObject(
toolchain_util.GenerateChromeOrderfile, 'Bundle', new=_Bundle)
@@ -725,6 +729,24 @@
print_cmd=True,
)
+ def testBundleVerifiedKernelCwpAfdoFile(self):
+ self.SetUpBundle('VerifiedKernelCwpAfdoFile')
+ mock_update = self.PatchObject(self.obj, '_UpdateKernelMetadata')
+ mock_ebuild = self.PatchObject(
+ self.obj, '_GetArtifactVersionInEbuild', return_value=self.kernel_name)
+ ret = self.obj.Bundle()
+ profile_name = self.kernel_name + \
+ toolchain_util.KERNEL_AFDO_COMPRESSION_SUFFIX
+ verified_profile = os.path.join(self.outdir, profile_name)
+ self.assertEqual([verified_profile], ret)
+ mock_update.assert_called_once_with(self.kernel_version, self.kernel_name)
+ mock_ebuild.assert_called_once_with(
+ f'chromeos-kernel-{self.kernel_version}', 'AFDO_PROFILE_VERSION')
+ profile_path = os.path.join(
+ self.chroot.path, self.sysroot[1:], 'usr', 'lib', 'debug', 'boot',
+ f'chromeos-kernel-{self.kernel_version}-{profile_name}')
+ self.copy2.assert_called_once_with(profile_path, verified_profile)
+
def runToolchainBundleTest(self, artifact_path, tarball_name, input_files,
expected_output_files):
"""Asserts that the given artifact_path is tarred up properly.
@@ -796,6 +818,58 @@
)
+class UpdateKernelMetadataTest(PrepareBundleTest):
+ """Test _UpdateKernelMetadata() function in BundleArtifactHandler."""
+
+ def setUp(self):
+ # Prepare a JSON file containing metadata
+ toolchain_util.TOOLCHAIN_UTILS_PATH = self.tempdir
+ osutils.SafeMakedirs(os.path.join(self.tempdir, 'afdo_metadata'))
+ self.json_file = os.path.join(self.tempdir,
+ 'afdo_metadata/kernel_afdo.json')
+ self.kernel = '4_14'
+ self.kernel2 = '4_4'
+ self.afdo_sorted_by_freshness = [
+ 'R78-3865.0-1560000000.afdo', 'R78-3869.38-1562580965.afdo',
+ 'R78-3866.0-1570000000.afdo'
+ ]
+ self.afdo_versions = {
+ self.kernel: {
+ 'name': self.afdo_sorted_by_freshness[0],
+ },
+ self.kernel2: {
+ 'name': self.afdo_sorted_by_freshness[1],
+ },
+ }
+
+ with open(self.json_file, 'w') as f:
+ json.dump(self.afdo_versions, f)
+
+ def testUpdateKernelMetadataFailureWithInvalidKernel(self):
+ with self.assertRaises(AssertionError) as context:
+ toolchain_util.BundleArtifactHandler._UpdateKernelMetadata('3_8', None)
+ self.assertIn('the entry should be in', str(context.exception))
+
+ def testUpdateKernelMetadataFailureWithOlderProfile(self):
+ with self.assertRaises(AssertionError) as context:
+ toolchain_util.BundleArtifactHandler._UpdateKernelMetadata(
+ self.kernel2, self.afdo_sorted_by_freshness[0])
+ self.assertIn('is not newer than', str(context.exception))
+
+ def testUpdateKernelMetadataPass(self):
+ toolchain_util.BundleArtifactHandler._UpdateKernelMetadata(
+ self.kernel, self.afdo_sorted_by_freshness[2])
+ # Check changes in JSON file
+ new_afdo_versions = json.loads(osutils.ReadFile(self.json_file))
+ self.assertEqual(len(self.afdo_versions), len(new_afdo_versions))
+ self.assertEqual(new_afdo_versions[self.kernel]['name'],
+ self.afdo_sorted_by_freshness[2])
+ for k in self.afdo_versions:
+ # Make sure other fields are not changed
+ if k != self.kernel:
+ self.assertEqual(self.afdo_versions[k], new_afdo_versions[k])
+
+
class ReleaseChromeAFDOProfileTest(PrepareBundleTest):
"""Test functions related to create a release CrOS profile.
@@ -2303,8 +2377,8 @@
with open(self.json_file, 'w') as f:
json.dump(self.afdo_versions, f)
- self.PatchObject(git, 'GetTrackingBranch',
- return_value=git.RemoteRef('origin', 'main'))
+ self.PatchObject(
+ git, 'GetTrackingBranch', return_value=git.RemoteRef('origin', 'main'))
GitStatus = collections.namedtuple('GitStatus', ['output'])
self.mock_git = self.PatchObject(
git, 'RunGit', return_value=GitStatus(output='non-empty'))
@@ -2338,8 +2412,7 @@
self.afdo_sorted_by_freshness[1],
self.afdo_sorted_by_freshness[2])
calls = [
- mock.call(
- self.tempdir, ['pull', 'origin'], print_cmd=True),
+ mock.call(self.tempdir, ['pull', 'origin'], print_cmd=True),
mock.call(
self.tempdir, ['status', '--porcelain', '-uno'],
capture_output=True,
@@ -2348,10 +2421,7 @@
mock.call(
self.tempdir, ['commit', '-a', '-m', message], print_cmd=True),
mock.call(
- self.tempdir, [
- 'push', 'origin',
- 'HEAD:main%submit'
- ],
+ self.tempdir, ['push', 'origin', 'HEAD:main%submit'],
capture_output=True,
print_cmd=True)
]
