Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed podman-mac-helper uninstall to remove docker.sock symlink #21499

Merged

Conversation

tnk4on
Copy link
Contributor

@tnk4on tnk4on commented Feb 3, 2024

Fixed to remove not only plist but also /var/run/docker.sock(symlink) when podman-mac-helper uninstall command is executed.

Fix: #20650 and podman-desktop/podman-desktop#5324 .

Does this PR introduce a user-facing change?

Fix `podman-mac-helper uninstall` command does not remove docker.sock symlink.

[NO NEW TESTS NEEDED]

@Luap99
Copy link
Member

Luap99 commented Feb 6, 2024

@baude @ashley-cui @n1hility PTAL

@tnk4on tnk4on force-pushed the fix-remove-dockersock-symlink branch from 2c6b4f6 to 021713c Compare February 6, 2024 17:46
@rhatdan
Copy link
Member

rhatdan commented Feb 13, 2024

@tnk4on Yes that looks good, but can you use:
if errors.Is(err, fs.ErrNotExist) {

Rather then
if os.IsNotExist(err) {

The errors.Is it the new way to handle this.

@tnk4on tnk4on force-pushed the fix-remove-dockersock-symlink branch from 021713c to d569d7c Compare February 13, 2024 12:18
@tnk4on
Copy link
Contributor Author

tnk4on commented Feb 13, 2024

@rhatdan @n1hility
Fixes added. Please review.

@rhatdan
Copy link
Member

rhatdan commented Feb 14, 2024

LGTM
Looks like you have a trailing white space error.
/approve

Copy link
Contributor

openshift-ci bot commented Feb 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, tnk4on

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 14, 2024
@tnk4on tnk4on force-pushed the fix-remove-dockersock-symlink branch from d569d7c to 71b52bb Compare February 15, 2024 13:47
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@n1hility
Copy link
Member

LGTM once CI passes

@tnk4on tnk4on force-pushed the fix-remove-dockersock-symlink branch from 71b52bb to 6a9e41e Compare February 15, 2024 17:16
@tnk4on tnk4on force-pushed the fix-remove-dockersock-symlink branch from 6a9e41e to bd0a9e9 Compare February 16, 2024 03:36
@tnk4on
Copy link
Contributor Author

tnk4on commented Feb 16, 2024

CI still has errors. Do I need to retry?

@TomSweeneyRedHat
Copy link
Member

Changes LGTM
I've just rerun the failed tests 🤞

@rhatdan
Copy link
Member

rhatdan commented Feb 18, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 6c7f987 into containers:main Feb 18, 2024
91 of 94 checks passed
@tnk4on tnk4on deleted the fix-remove-dockersock-symlink branch February 19, 2024 02:12
@stale-locking-app 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 May 20, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators May 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman-mac-helper can't remove docker.sock
5 participants