Attempt to fix "Extracting compiled package archive failed" error #2454
Conversation
For clarity of flow + variable naming Signed-off-by: Brian Upton <bupton@vmware.com>
Previously, there were cases where a package existed the name and
fingerprint, but did not match stemcell, was mistakenly excluded from
upload. This results in errors like the following:
```
Task 1939132 | 20:03:34 | Creating new compiled packages: routing-golang-1-linux/8c04109541f4d504f5be559da433998bd459b0f45cd3654557cc3642cc4d2f60 for ubuntu-jammy/1.147 (00:00:00)
L Error: Extracting compiled package archive failed. Check task debug log for details.
```
Now, when deciding which packages do not need to be uploaded, we
consider name, fingerprint, dependency key, and stemcell os/version.
Signed-off-by: Aram Price <pricear@vmware.com>
ystros
requested review from
a team,
jpalermo and
nouseforaname
and removed request for
a team
| else | ||
| fingerprint_list = manifest_hash['packages'].map { |package| package['fingerprint'] }.compact | ||
| Models::Package.where(fingerprint: fingerprint_list) | ||
| .where(Sequel.~(sha1: nil)) |
There was a problem hiding this comment.
could these result in issues where ppl are using the sha256 hack for releases.
where they provide sha1: sha256:HASH256 ?
There was a problem hiding this comment.
Possibly - this code no moved, and not new so if this is an issue we should probably address it in a separate change.
There was a problem hiding this comment.
The column in the database is always called sha1, regardless of whether an actual sha1 or a sha256 is provided. Sequel.~(sha1: nil) should translate to SQL sha1 IS NOT NULL, so it should cover both sha1 and sha256 cases.
Regardless, this query is not something we changed, simply reordered things to make the structure of both the /matches and /matches_compiled controller actions similar. The diff is messy, but you can see the query in the previously existing code:
|
|
||
| json_encode(matching_packages.map(&:fingerprint).compact.uniq) | ||
| fingerprints_that_are_already_uploaded - fingerprints_that_need_upload |
There was a problem hiding this comment.
Just wondering about naming here, is this subtraction the right way?
There was a problem hiding this comment.
Nevermind, just realized that the method is called
compiled_package_fingerprints_not_to_be_uploaded
Previously, there were cases where a package existed the name and fingerprint, but did not match stemcell, was mistakenly excluded from upload. This results in errors like the following:
Now, when deciding which packages do not need to be uploaded, we consider name, fingerprint, dependency key, and stemcell os/version.
We ended up refactoring quite a bit in an effort to make the code more understandable (and understand it ourselves...). The most relevant logic change is in the
compiled_package_fingerprints_not_to_be_uploadedmethod.