p8952 / bocker

Docker implemented in around 100 lines of bash
https://www.p8952.info/
GNU General Public License v3.0
11.29k stars 718 forks source link

Execute commands in a container #7

Closed Fusion closed 9 years ago

Fusion commented 9 years ago

The exec command was not even available when Docker got started. Bash can do better than that! :)

A very simple way to enter a shell or execute a command in an existing container.

Sorry, the script is now 107 lines long. I'll leave it to you to decide how to best fix this error.

Fusion commented 9 years ago

Oops I realize that I forgot to use $btrfs_path -- sorry about that.

j8r commented 9 years ago

Now it can probably fit under 100 lines, but this will be harder and harder to stay behind this limit in the future...

Fusion commented 9 years ago

Obviously, wking, you make some valid points. I was trying to stick with the script's original style but I see that is was heavily modified and now actually fits your comments. I guess I will submit another pull request, matching the current compact style.

wking commented 9 years ago

On Sat, Jul 25, 2015 at 06:00:23PM -0700, Chris F Ravenscroft wrote:

I guess I will submit another pull request, matching the current compact style.

No need to open a new pull request. Just reroll your branch (e.g. by amending your commit) and force-push to Fusion:master. GitHub will notice the new branch tip and update this PR accordingly.

Fusion commented 9 years ago

Oh, cool. I had already pushed a new revision so it was a bit more work but I think I got it right: git reset --hard HEAD~2 && && git --amend --no-edit && git push origin --force. The resulting tree should look clean.

The merge itself, though, is a monster due to the difference between your repo then and now :)

p8952 commented 9 years ago

Thanks for the contribution @Fusion, I think some of my more recent changes have made enough room for this.

I'll have a look into merging it cleanly later today.

p8952 commented 9 years ago

Merged fine and works as expected, thanks @Fusion!

Will just write some documentation and test cases, then push.

Fusion commented 9 years ago

I checked something in and it got automatically added to this request, when I was planning on opening another request to keep things clean.

So, that's cgroups support. Not all of cgroups's toolbox, mind you, but enough to actually run in separate groups, allowing an admin to then fine tune cpu resources, etc. Blame reddit commenters for this!

It should degrade gracefully (no cgroups, no problem) and while I was at it I made a small modification to 'ps' so that it would display the containers based on time of creation rather than name. Feel free to not merge it if it goes against your design.

p8952 commented 9 years ago

Ok, exec support is merged :smile:

I think if I close this you should be able to open a new PR with the cgroup changes? I think that will be easier than merging them from here.