Machine decompress.go refactoring follow-up #21864

l0rd · 2024-02-28T15:02:53Z

⚠️ This PR is based on #21592 but with a broader impact:

local xz implementation removed and implementation from c/image used (for consistency with other compressors)
the decompression implementation is independent from the OS (no
SparseWriter is always used on macOS (not on other platforms)

This PR should also fix #21772

Does this PR introduce a user-facing change?

None

Luap99 · 2024-02-29T13:36:38Z

Can you rebase since the other PR merged, also the go.mod changes must be made in the commit were the changes are needed.

mtrmac

The implementation mechanism LGTM.

I’ll defer to machine experts as to whether removing the external xz decompression path is worth worrying about.

Luap99 · 2024-02-29T15:31:20Z

@baude @mheon @ashley-cui PTAL

mheon · 2024-02-29T19:03:38Z

LGTM, but I'd like an ack from @baude before merging is considered

l0rd · 2024-03-12T23:57:42Z

There are 6 Cirrus CI failing.

bud remote fedora-39 root host sqlite

Error: creating build container: copying system image from manifest list: reading blob sha256:2c2d948710f21ad82dce71743b1654b45acb5c059cf5c19da491582cef6f2601: fetching blob: received unexpected HTTP status: 502 Bad Gateway

int remote rawhide root host sqlite

Error: initializing source docker://quay.io/libpod/cirros:latest: can't talk to a V1 container registry

Error: copying system image from manifest list: determining manifest MIME type for docker://quay.io/libpod/cirros:latest: reading manifest sha256:c7d58d6d463247a2540b8c10ff012c34fd443426462e891b13119a9c66dfd28a in quay.io/libpod/cirros: received unexpected HTTP status: 502 Bad Gateway

sys podman debian-13 rootless host sqlite

# 	* updating image for container 19eae661009f8dd5151d1fd67c906b51e870bfcdebab31ba98acc5dac25abea3: initializing source docker://quay.io/libpod/alpine:latest: can't talk to a V1 container registry
# 	* checking image updates for container 80605c476f439022d0a441d398a45e2ec0bf37be4196e4686e967db53a9615db: reading manifest latest in quay.io/libpod/busybox: received unexpected HTTP status: 502 Bad Gateway

sys podman rawhide root host sqlite

# Error: updating image for container 40d75534a0ae254277b96c99d253444caaa9964c51da08f3aea7d667bd69d97d: initializing source docker://quay.io/libpod/alpine:latest: can't talk to a V1 container registry

At first look those failures don't seam related to this PR.

I am going to rebase it to re-trigger the tests 🤞

1. Added the xz decompression unit tests 2. Removed the xz implementation to use the one from c/images 3. Removed the specific macos gzip, zstd compressor and use the generic compressor but with SparseWriter if GOOS == darwin Signed-off-by: Mario Loriedo <[email protected]>

packit-as-a-service · 2024-03-13T00:41:16Z

Cockpit tests failed for commit 8259714. @martinpitt, @jelly, @mvollmer please check.

Luap99 · 2024-03-13T10:26:07Z

@l0rd Yes these are your normal share of registry/network flakes, given so many tests have to pull images in some form we notice quickly if quay.io has some problems.

Luap99 · 2024-03-13T10:27:52Z

@baude This is waiting on you.

l0rd · 2024-03-13T11:00:40Z

@l0rd Yes these are your normal share of registry/network flakes, given so many tests have to pull images in some form we notice quickly if quay.io has some problems.

@Luap99 thank you for looking at it. After rebasing I still have a (different) test failing. It doesn't seem related to this PR though but it's kind of cryptic (is there a logformatter for packit-dev tests?):

> warning: Failed to update container stats: "Your session has been terminated."
> log: {"problem":"terminated","message":"Your session has been terminated."}
CDP: {"source":"other","level":"warning","text":"XrayWrapper denied access to property \"toString\" (reason: value is callable). See https://developer.mozilla.org/en-US/docs/Xray_vision for more information. Note that only the first denied property access from a given global object will be reported.","timestamp":1710289306538,"url":"chrome://remote/content/cdp/sessions/ContentProcessSession.sys.mjs","lineNumber":50}
> warning: IncompleteRead(0 bytes read)
> warning: IncompleteRead(0 bytes read)
> warning: Failed to update container stats: "Your session has been terminated."
> log: {"problem":"terminated","message":"Your session has been terminated."}
Connection to 127.0.0.1 closed by remote host.

> warning: transport closed: disconnected
Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Traceback (most recent call last):
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/check-application", line 432, in testBasicSystem
    self._testBasic(True)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/check-application", line 636, in _testBasic
    b.click(hello_sel + " .pf-v5-c-menu-toggle")
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 412, in click
    self.mouse(selector + ":not([disabled]):not([aria-disabled=true])", "click", 0, 0, 0)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 404, in mouse
    self.wait_visible(selector)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 648, in wait_visible
    self._wait_present(selector)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 639, in _wait_present
    self.wait_js_func('ph_is_present', selector)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 633, in wait_js_func
    self.wait_js_cond("%s(%s)" % (func, ','.join(map(jsquote, args))))
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 618, in wait_js_cond
    self.raise_cdp_exception("timeout\nwait_js_cond", cond, result["exceptionDetails"], trailer)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 319, in raise_cdp_exception
    raise Error("%s(%s): %s" % (func, arg, msg))
testlib.Error: timeout

I am going to rebase again to re-trigger the tests.

Luap99 · 2024-03-13T11:09:53Z

@l0rd Yes these are your normal share of registry/network flakes, given so many tests have to pull images in some form we notice quickly if quay.io has some problems.

@Luap99 thank you for looking at it. After rebasing I still have a (different) test failing. It doesn't seem related to this PR though but it's kind of cryptic (is there a logformatter for packit-dev tests?):

> warning: Failed to update container stats: "Your session has been terminated."
> log: {"problem":"terminated","message":"Your session has been terminated."}
CDP: {"source":"other","level":"warning","text":"XrayWrapper denied access to property \"toString\" (reason: value is callable). See https://developer.mozilla.org/en-US/docs/Xray_vision for more information. Note that only the first denied property access from a given global object will be reported.","timestamp":1710289306538,"url":"chrome://remote/content/cdp/sessions/ContentProcessSession.sys.mjs","lineNumber":50}
> warning: IncompleteRead(0 bytes read)
> warning: IncompleteRead(0 bytes read)
> warning: Failed to update container stats: "Your session has been terminated."
> log: {"problem":"terminated","message":"Your session has been terminated."}
Connection to 127.0.0.1 closed by remote host.

> warning: transport closed: disconnected
Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Exiting due to channel error.
Traceback (most recent call last):
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/check-application", line 432, in testBasicSystem
    self._testBasic(True)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/check-application", line 636, in _testBasic
    b.click(hello_sel + " .pf-v5-c-menu-toggle")
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 412, in click
    self.mouse(selector + ":not([disabled]):not([aria-disabled=true])", "click", 0, 0, 0)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 404, in mouse
    self.wait_visible(selector)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 648, in wait_visible
    self._wait_present(selector)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 639, in _wait_present
    self.wait_js_func('ph_is_present', selector)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 633, in wait_js_func
    self.wait_js_cond("%s(%s)" % (func, ','.join(map(jsquote, args))))
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 618, in wait_js_cond
    self.raise_cdp_exception("timeout\nwait_js_cond", cond, result["exceptionDetails"], trailer)
  File "/var/ARTIFACTS/work-podman-systemugyvyr9u/plans/cockpit-podman/podman-system/discover/default-0/tests/test/common/testlib.py", line 319, in raise_cdp_exception
    raise Error("%s(%s): %s" % (func, arg, msg))
testlib.Error: timeout

I am going to rebase again to re-trigger the tests.

No need to worry about them, cockpit revdeps are not blocking and this is not related to your PR so just ignore it.

baude · 2024-03-13T16:44:49Z

/approve
/lgtm

openshift-ci · 2024-03-13T16:44:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude, l0rd

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [baude]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added the release-note-none label Feb 28, 2024

github-actions bot added the machine label Feb 28, 2024

l0rd changed the title ~~Refactor machine decompress.go~~ Refactor machine decompress.go (broader impact) Feb 28, 2024

l0rd mentioned this pull request Feb 28, 2024

Refactor machine decompress.go #21592

Merged

l0rd force-pushed the compress-refactoring-v5-plus-plus branch 2 times, most recently from cd37402 to e161f01 Compare February 29, 2024 14:57

l0rd changed the title ~~Refactor machine decompress.go (broader impact)~~ Machine decompress.go refactoring follow-up Feb 29, 2024

mtrmac reviewed Feb 29, 2024

View reviewed changes

l0rd force-pushed the compress-refactoring-v5-plus-plus branch 2 times, most recently from abf9cd7 to a8ee793 Compare March 12, 2024 13:55

l0rd force-pushed the compress-refactoring-v5-plus-plus branch from a8ee793 to 8259714 Compare March 13, 2024 00:00

openshift-ci bot assigned baude Mar 13, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 13, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 13, 2024

openshift-merge-bot bot merged commit c25bfe1 into containers:main Mar 13, 2024
93 of 94 checks passed

stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Jun 12, 2024

stale-locking-app bot locked as resolved and limited conversation to collaborators Jun 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Machine decompress.go refactoring follow-up #21864

Machine decompress.go refactoring follow-up #21864

l0rd commented Feb 28, 2024 •

edited

Loading

Luap99 commented Feb 29, 2024

mtrmac left a comment

Luap99 commented Feb 29, 2024

mheon commented Feb 29, 2024

l0rd commented Mar 12, 2024

packit-as-a-service bot commented Mar 13, 2024

Luap99 commented Mar 13, 2024

Luap99 commented Mar 13, 2024

l0rd commented Mar 13, 2024

Luap99 commented Mar 13, 2024

baude commented Mar 13, 2024

openshift-ci bot commented Mar 13, 2024

Machine decompress.go refactoring follow-up #21864

Machine decompress.go refactoring follow-up #21864

Conversation

l0rd commented Feb 28, 2024 • edited Loading

Does this PR introduce a user-facing change?

Luap99 commented Feb 29, 2024

mtrmac left a comment

Choose a reason for hiding this comment

Luap99 commented Feb 29, 2024

mheon commented Feb 29, 2024

l0rd commented Mar 12, 2024

packit-as-a-service bot commented Mar 13, 2024

Luap99 commented Mar 13, 2024

Luap99 commented Mar 13, 2024

l0rd commented Mar 13, 2024

Luap99 commented Mar 13, 2024

baude commented Mar 13, 2024

openshift-ci bot commented Mar 13, 2024

l0rd commented Feb 28, 2024 •

edited

Loading