canaries failing due to lack backport-lzma build |
|||||
Issue description
canaries are failing with this error:
Traceback (most recent call last):
File "/b/c/cbuild/repository/chromite/lib/parallel.py", line 603, in TaskRunner
task(*x, **task_kwargs)
File "/b/c/cbuild/repository/chromite/lib/parallel.py", line 801, in <lambda>
fn = lambda idx, task_args: out_queue.put((idx, task(*task_args)))
File "/b/c/cbuild/repository/chromite/lib/paygen/paygen_build_lib.py", line 244, in _GenerateSinglePayload
dry_run=dry_run)
File "/b/c/cbuild/repository/chromite/lib/paygen/paygen_payload_lib.py", line 830, in CreateAndUploadPayload
dry_run=dry_run).Run()
File "/b/c/cbuild/repository/chromite/lib/paygen/paygen_payload_lib.py", line 710, in Run
self._drm(self._VerifyPayload)
File "/b/c/cbuild/repository/chromite/lib/paygen/dryrun_lib.py", line 46, in __call__
return self.Run(func, *args, **kwargs)
File "/b/c/cbuild/repository/chromite/lib/paygen/dryrun_lib.py", line 83, in Run
return self._Call(func, *args, **kwargs)
File "/b/c/cbuild/repository/chromite/lib/paygen/dryrun_lib.py", line 87, in _Call
return func(*args, **kwargs)
File "/b/c/cbuild/repository/chromite/lib/paygen/paygen_payload_lib.py", line 684, in _VerifyPayload
self._ApplyPayload(payload, is_delta)
File "/b/c/cbuild/repository/chromite/lib/paygen/paygen_payload_lib.py", line 644, in _ApplyPayload
**part_files)
File "/b/c/cbuild/repository/src/aosp/system/update_engine/scripts/update_payload/payload.py", line 320, in Apply
old_rootfs_part=old_rootfs_part)
File "/b/c/cbuild/repository/src/aosp/system/update_engine/scripts/update_payload/applier.py", line 647, in Run
self.payload.manifest.old_rootfs_info)
File "/b/c/cbuild/repository/src/aosp/system/update_engine/scripts/update_payload/applier.py", line 595, in _ApplyToPartition
new_part_file, new_part_info.size)
File "/b/c/cbuild/repository/src/aosp/system/update_engine/scripts/update_payload/applier.py", line 531, in _ApplyOperations
self._ApplyReplaceOperation(op, op_name, data, new_part_file, part_size)
File "/b/c/cbuild/repository/src/aosp/system/update_engine/scripts/update_payload/applier.py", line 254, in _ApplyReplaceOperation
out_data = lzma.decompress(out_data)
NameError: global name 'lzma' is not defined
The problem is that backports-lzma is not imported properly. This is is cased because chromium-os-sdk for some reason is not adding this dependency even if it is added in:
https://chromium.git.corp.google.com/chromiumos/overlays/chromiumos-overlay/+/1697f7bf464a33b649154ce315f1874739116c63/virtual/target-chromium-os-sdk/target-chromium-os-sdk-1.ebuild#483
,
Jan 28 2018
that code is run outside of the sdk. note the /b/c/cbuild/ prefix on paths.
,
Jan 28 2018
So, do you know off hand how to add that dependency to outside of sdk?!!! Does that even makes sense?
,
Jan 28 2018
it's possible, but typically the infra guys want to avoid that
,
Jan 28 2018
Well, lzma is a needed feature and one way or another we have to get it to work. I can temporary get the canaries green by reverting crrev.com/c/872125 if adding that dependency requires a push (which may take a few days to do). but I'll let the infra guys decide if they can add this dependency or not. This only needed for python 2.7.
,
Jan 29 2018
uploaded crrev.com/c/890384 to temporarily fix the issue.
,
Jan 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/update_engine/+/cb2da0df4382f546efd4269851ce814842a4b7fd commit cb2da0df4382f546efd4269851ce814842a4b7fd Author: Amin Hassani <ahassani@google.com> Date: Mon Jan 29 03:09:19 2018 update_engine: Temporarily disable XZ generation Disable generating XZ operations until crbug.com/806613 is fixed. BUG= chromium:806613 TEST=unit tests pass Change-Id: I64b9852b447f7ff1f1a45f10205e3b23a2591cca Reviewed-on: https://chromium-review.googlesource.com/890384 Commit-Ready: Amin Hassani <ahassani@chromium.org> Tested-by: Amin Hassani <ahassani@chromium.org> Reviewed-by: Amin Hassani <ahassani@chromium.org> [modify] https://crrev.com/cb2da0df4382f546efd4269851ce814842a4b7fd/payload_generator/delta_diff_utils.cc
,
Jan 29 2018
you can have cbuildbot execute commands inside of the chroot instead on a per-command basis
,
Jan 29 2018
Might be related to: https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/875260 Adding the corresponding peo;e
,
Jan 29 2018
This isn't related to https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/875260 . All that CL does is explicitly avoid building backports-lzma for Python versions >3 because lzma is already included in Python >3. We still build it for 2.7. Since 2.7 is being used here, that CL had no effective delta.
,
Jan 29 2018
that CL also only affects code inside the SDK while this code/crash is happening outside the SDK
,
Jan 29 2018
I think there is a reason we are running the code outside of chroot. dgarrentt@ might have more background on why. But my initial guess is that we gather all the files related to payload generation into a zip file and running them where ever necessary without being dependent on where the code is running or if the chroot is available. For example I think, and I'm probably wrong, the tryjob-payload builders do not necessarily build the sdk and chroot. Other reason could be that somehow omaha uses the zip file contents to create signed delta payloads? In any case if we run this inside chroot, then the dependencies of delta_generator can be mismatched with the ones in chroot. right?
,
Jan 29 2018
Ugh: https://cs.corp.google.com/chromeos_public/chromite/lib/paygen/paygen_payload_lib.py?q=CreateAndUploadPayload&l=33 paygen is importing and using python modules from update_engine. The right fix would be to figure out why we want to do this outside of chroot (there's actually an --outside-chroot argument in one of the calls below)
,
Jan 29 2018
@pprabhu: look at #12
,
Jan 30 2018
The au-generator.zip has all the binaries and libs to *generate* a payload, which doesn't use python at all. The python part is used to verify the payload against the spec in python and by the builder Paygen stage. The reason why the generator binraies run outside the chroot is that the chroot is at a potentially different version than the au-generator you are using, so having lib or executable dependencies into the chroot is a problem more than a solution. It used to be the case that we would run very old au-generator.zip files, nowadays we only run a somewhat old, probably the same version you just built like with canaries, but in other channels you sometimes generate deltas from older versions after the fact. Regarding the python verifier (paycheck.py) and its python dependencies, I don't think it is packaged with the au-generator, they come from the builder manifest (I think update_engine was added at some point to have the scripts there). So if you have a python dependency from paycheck you could either add the dependency to the chroot and run just paycheck inside the chroot as a subprocess (which is a totally different flow and might bring more problems), or somehow add that dependency to the builder, together with all the other python modules the builder is required to have. Yeah, both options look problematic =/
,
Jan 30 2018
What is the main problem with adding that dependency to the builders? lzma-backports is only a python wrapper for lzma. I don't think we need dependency to lzma itself (probably builders have it as part of XZ libraries), but just the wrapper.
,
Jan 30 2018
Re #12, the tryjob-payload builders don't build anything, they only download build artifacts from a previous build an use them. Downloading au-generator.zip was originally because Paygen ran on dedicated servers, not as part of the build. When paygen was merged into release builders, it was convenient to keep downloading for the sake of the tryjobs. Re #15 We used to use the oldest version of au-generator.zip for generating deltas to ensure compatibility with the build that had to apply the delta, but that's no longer true. We now use major/minor versions to handle compatibility issues, and the current generator binaries are supposed to "do the right thing" for old version numbers. This lets us fix bugs in the generators without all of the shenanigans we used to have to pull. You can see most of the logic around selecting/using au_generator.zip in chromite/lib/paygen/paygen_payload_lib.py in _PrepareGenerator, and _RunGeneratorCmd.
,
Jan 30 2018
Running paycheck inside the chroot is probably oka, but might mean some tweaking of paygen tryjobs is needed. Paycheck was originally intended for stand alone use, but that's unimportant now. We haven't seen a corrupted payload file in years, and checksums are being emitted into the json files describing the payloads.
,
Jan 30 2018
You could get rid of au-generator.zip altogether, and run everything inside the chroot, if you can come up with a reasonable solution for the tryjobs.
,
Jan 30 2018
So are you suggesting we move away from au-generator.zip and generate/paycheck everything in the chroot? What is the best path forward here? We need this feature to get going, since we need to for UE major version upgrade too.
,
Jan 30 2018
Sorry I didn't see your last comment. Where can I look at the tryjob configs to figure things out?
,
Jan 30 2018
Here is the custom builder class used for paygen tryjobs: chromite/cbuildbot/builders/release_builders.py: GeneratePayloadsBuilder Looks like it doesn't create a chroot, or do much of anything other than run paygen in parallel for every requested channel. Here is an example of a custom builder that goes through the normal build stages: test_builder.py: UnittestStressBuilder
,
Feb 2 2018
,
Feb 2 2018
re-comment #16: it isn't a problem with this specific package, it's a problem with adding any package. the c-i-t manages the OS images and they've been pushing back in general on doing any customization because it's painful for them to make sure everything stays in sync. they'd rather only rely on the base image and let clients (like us) deal with uncommon deps ourselves. if we can run everything inside the sdk, then that'd be their preferred answer.
,
Feb 2 2018
Yeah, sure. I'm in the process of it. for an initial plan look crbug.com/808495
,
Mar 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/update_engine/+/f7006be5079ed91c93223273f3ddb69d655ba858 commit f7006be5079ed91c93223273f3ddb69d655ba858 Author: Amin Hassani <ahassani@chromium.org> Date: Thu Mar 08 03:47:06 2018 Revert "update_engine: Temporarily disable XZ generation" This reverts commit cb2da0df4382f546efd4269851ce814842a4b7fd. Reason for revert: <paygens are running in chroot now sow lzma dependency should be fixed by now> BUG= chromium:806613 TEST=unittest Original change's description: > update_engine: Temporarily disable XZ generation > > Disable generating XZ operations until crbug.com/806613 is fixed. > > BUG= chromium:806613 > TEST=unit tests pass > > Change-Id: I64b9852b447f7ff1f1a45f10205e3b23a2591cca > Reviewed-on: https://chromium-review.googlesource.com/890384 > Commit-Ready: Amin Hassani <ahassani@chromium.org> > Tested-by: Amin Hassani <ahassani@chromium.org> > Reviewed-by: Amin Hassani <ahassani@chromium.org> TBR=benchan@chromium.org,senj@chromium.org,ahassani@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:806613 Change-Id: Ic918b7e4febf284f38b2ca3119c1fcbf57538a89 Reviewed-on: https://chromium-review.googlesource.com/952378 Commit-Ready: Amin Hassani <ahassani@chromium.org> Tested-by: Amin Hassani <ahassani@chromium.org> Reviewed-by: Amin Hassani <ahassani@chromium.org> [modify] https://crrev.com/f7006be5079ed91c93223273f3ddb69d655ba858/payload_generator/delta_diff_utils.cc
,
Mar 30 2018
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ahass...@chromium.org
, Jan 28 2018