crc-org / crc

CRC is a tool to help you run containers. It manages a local OpenShift 4.x cluster, Microshift or a Podman VM optimized for testing and development purposes
https://crc.dev
Apache License 2.0
1.26k stars 242 forks source link

bundle download & decompression: added Context argument #4405

Open redbeam opened 1 month ago

redbeam commented 1 month ago

Fixes: Issue #3998

Solution/Idea

Added a context.Context argument to functions such as bundle.Download and bundle.Extract and others, to allow them to gracefully stop (if they are running) when the stop command is issued through the HTTP API.

Context support was added to regular HTTP bundle downloads as well as bundle pulls from container registries (it was supported by the used libraries). However, for the bundle decompression, this had to be done manually, since there is no library used.

If there is no context available to be used, nil or context.Background() can be used.

Testing

$ curl --unix-socket ~/.crc/crc-http.sock  http://unix/api/start
# the start command hangs waiting for the bundle to be downloaded/decompressed

# in another terminal
$ curl --unix-socket ~/.crc/crc-http.sock  http://unix/api/stop
Instance is already stopped

Caveat

The stop command ends with Instance is already stopped error (error code 500 is returned) since it is intended to stop the machine, which is (at that moment) not running. But it stops the download/decompression, which is desired.

openshift-ci[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign anjannath for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/crc-org/crc/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
redbeam commented 1 month ago

@cfergeau yes, the s.startCancel() function called in cancelUnlocked is actually the cancel function of the context. And it works as expected.

https://github.com/crc-org/crc/blob/b174d0ccd2c5abdcda875468d1127caa5826036a/pkg/crc/machine/sync.go#L101-L112

And yes, the context is also provided to the decompression code.

openshift-ci[bot] commented 1 day ago

@redbeam: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-crc b6ffde2d4cd4b35bd070e7be5fcf44a64fb46cf2 link true /test e2e-crc

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).