Don't use docker shim if only using a mounted docker.sock instead of docker-in-docker #67

Hoverbear · 2024-01-10T19:45:42Z

Description

I believe this fixes #65.

Checklist

Tested changes against a test repository
Added or updated relevant documentation (leave unchecked if not applicable)
(If this PR is for a release) Updated README to point to the new tag (leave unchecked if not applicable)

…docker-in-docker

cole-h

I think this looks sound overall, but I'm not extremely familiar with docker or cgroups and the like, so take this approval with a grain of salt...

cole-h · 2024-01-10T22:52:08Z

src/main.ts

+    }
+
+    const cgroups = cgroups_buffer.trim().split("\n");
+    const last_cgroup = cgroups[cgroups.length - 1];


Would it be worth it to check every cgroup line to see if there is /docker/ in it, or if it's in an earlier cgroup, is that a weird configuration we wouldn't want to support anyways?

I don't have access to every system and setup, only a few, but I think in this case we're only looking for the 0:: prefixed line which is always last.

colemickens

Also no cgroup expert, but the code matches what was described, and the description I was given sounded convincing for detecting this scenario and avoiding it.

… v10 (#82) [![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [DeterminateSystems/nix-installer-action](https://github.com/DeterminateSystems/nix-installer-action) | action | major | `v9` -> `v10` | --- ### Release Notes <details> <summary>DeterminateSystems/nix-installer-action (DeterminateSystems/nix-installer-action)</summary> ### [`v10`](https://github.com/DeterminateSystems/nix-installer-action/releases/tag/v10) [Compare Source](https://github.com/DeterminateSystems/nix-installer-action/compare/v9...v10) ##### What's Changed - action: post-run-job: try clean daemon container, warn on failure by [@colemickens](https://github.com/colemickens) in [DeterminateSystems/nix-installer-action#61 - No longer require sudo by [@Hoverbear](https://github.com/Hoverbear) in [DeterminateSystems/nix-installer-action#64 - Handle docker not existing. by [@Hoverbear](https://github.com/Hoverbear) in [DeterminateSystems/nix-installer-action#66 - Don't use docker shim if only using a mounted docker.sock instead of docker-in-docker by [@Hoverbear](https://github.com/Hoverbear) in [DeterminateSystems/nix-installer-action#67 - DETERMINATE_NIX_KVM fixup, support Magic Nix Cache + FlakeHub Cache on Namespace runners by [@grahamc](https://github.com/grahamc) in [DeterminateSystems/nix-installer-action#72 **Full Changelog**: DeterminateSystems/nix-installer-action@v9...v10 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/packethost/packet-networking).

Don't use docker shim if only using a mounted docker.sock instead of …

f4a0ffe

…docker-in-docker

Hoverbear self-assigned this Jan 10, 2024

Hoverbear requested a review from cole-h January 10, 2024 19:45

cole-h requested a review from colemickens January 10, 2024 21:55

cole-h approved these changes Jan 10, 2024

View reviewed changes

colemickens approved these changes Jan 11, 2024

View reviewed changes

Hoverbear merged commit e279ba5 into main Jan 11, 2024
5 checks passed

Hoverbear deleted the hoverbear/fh-161-after-running-in-act-hosts-nix-daemon-is-unusable branch January 11, 2024 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't use docker shim if only using a mounted docker.sock instead of docker-in-docker #67

Don't use docker shim if only using a mounted docker.sock instead of docker-in-docker #67

Hoverbear commented Jan 10, 2024

cole-h left a comment

cole-h Jan 10, 2024

Hoverbear Jan 11, 2024

colemickens left a comment

Don't use docker shim if only using a mounted docker.sock instead of docker-in-docker #67

Don't use docker shim if only using a mounted docker.sock instead of docker-in-docker #67

Conversation

Hoverbear commented Jan 10, 2024

Description

Checklist

cole-h left a comment

Choose a reason for hiding this comment

cole-h Jan 10, 2024

Choose a reason for hiding this comment

Hoverbear Jan 11, 2024

Choose a reason for hiding this comment

colemickens left a comment

Choose a reason for hiding this comment