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

Add docker support #369

Closed
1 of 3 tasks
phil-opp opened this issue Nov 24, 2017 · 13 comments
Closed
1 of 3 tasks

Add docker support #369

phil-opp opened this issue Nov 24, 2017 · 13 comments
Labels
enhancement Potential improvements for the code or the blog. feedback wanted We want to hear opinions about this change. help wanted We could use help for this issue/pull request!

Comments

@phil-opp
Copy link
Owner

phil-opp commented Nov 24, 2017

Continuing the discussion from #368 and #222. Relevant comments: [1] [2] [3] [4]

Required steps for docker support:

  • Add a Dockerfile that allows building (and running) this project (Docker: Add a Dockerfile and Makefile targets #373)
  • Add a short tutorial that explains how to use docker with this project.
  • Mention the docker support in the first post (alongside/instead the mention of Vagrant).
@phil-opp phil-opp added feedback wanted We want to hear opinions about this change. enhancement Potential improvements for the code or the blog. labels Nov 24, 2017
@andre-richter
Copy link
Contributor

Hi phil,

thanks for opening. Short summary from previous discussions:
I would propose to use the redox Dockerfile + entrypoint as a basis.

As a first shot, I would envision providing a docker image that the user can use interactively like shown in
https://github.com/redox-os/redox/tree/master/docker#demo
https://github.com/andre-richter/docker-rust-persistent#demo

The advantage of this approach is that it is a transparent add-on to the native file structure of the project. Once a console into the docker container has been spawned, there is no difference to native setup in which the rust toolchain was installed locally.

I will be on vacation soon, so I guess I cannot start working on this before January.

@robert-w-gries
Copy link
Contributor

I added Docker support to my kernel recently, using @andre-richter's kernel for reference. I found that adding these Makefile rules simplified the process most for me.

When you publish the Docker image, the steps to have a functional development enviroment are simply:

wget -qO- https://get.docker.com/ | sh
# Hosts with systemctl: systemctl start docker
make docker_run
make run # Inside of container

Note that docker needs root privileges to run out of the box. It's possible to run with non-root, but it needs additional steps

@phil-opp phil-opp added the help wanted We could use help for this issue/pull request! label Nov 25, 2017
@Sh4d1
Copy link
Contributor

Sh4d1 commented Dec 6, 2017

Hey!

I did a PR #373 adding Docker support :)
I followed @andre-richter advice and used Redox idea.

Cheers :)

@robert-w-gries
Copy link
Contributor

@Sh4d1 Nice work!

My two cents regarding the Makefile rules, the docker image default should be called blog_os. Or if it's published by Phil it could be called blog_os/rust_os or similar.

@Sh4d1
Copy link
Contributor

Sh4d1 commented Dec 7, 2017

You are right :) changed it to blog_os!

@andre-richter
Copy link
Contributor

Awesome!

Some remarks from inspecting the code:

  • The docker_run target is only building the iso, so you would not need --it I guess.
  • Like redox, I think there would be a benefit of providing a target that spawns an interactive shell (docker_interactive ?). This would enable the user to repeat individual smaller make targets inside the docker shell, and not only the iso target.
  • Instructions for deleting the local cache of cargo and rustup like in the redox readme might be beneficial.
  • If the stuff above is included, maybe a separate readme makes sense so that the original readme doesn't get cluttered. I think one of phil's main objectives is to have the docker stuff clearly and cleanly separated from the "vanilla" way.

BR,
Andre

@Sh4d1
Copy link
Contributor

Sh4d1 commented Dec 7, 2017

Hey!

Okay, so I'll

  • Modify the Makefile to include the interactive mode and remove the useless --it
  • Modify the README to include instructions about deleting local cache and move the Building with docker part into another README located in the docker folder

Sounds good?

Patrik

@Sh4d1
Copy link
Contributor

Sh4d1 commented Dec 7, 2017

Okay, so now there are five rules concerning Docker in the Makefile

  • docker_build which builds the docker image
  • docker_iso which builds the iso
  • docker_run which builds the iso & run via qemu
  • docker_interactive which opens a shell in the container
  • docker_clean which cleans the two volumes used by cargo & rustup

Do you think it belongs in the Makefile? Or somewhere else?

Cheers,

Patrik

@andre-richter
Copy link
Contributor

Looks good to me. Provided two nitpick comments in the PR itself.

When using an interactive shell, I found it useful to have an indication via $PS1 that I am running in a Docker container. Otherwise you can easily get confused if you have several terminals open.

For redox, I pulled in a .bash_aliases to resolve this: https://github.com/redox-os/redox/blob/master/docker/.bash_aliases
You'd need to set the respective ENV variable too (https://github.com/redox-os/redox/blob/master/docker/Dockerfile#L3)

Kindly have a look if you think it is useful here too.

@Sh4d1
Copy link
Contributor

Sh4d1 commented Dec 7, 2017

Yep, I did not think about that! Pushed as well, but still the issue with the Makefile targets run and docker_build ..

@Gaelan
Copy link

Gaelan commented Mar 16, 2018

Is this obsolete now?

@robert-w-gries
Copy link
Contributor

Docker support looks done to me.

@phil-opp After closing this issue, you should mark the "Docker support" task in #360 as done too.

@phil-opp
Copy link
Owner Author

Yeah, I think this is done. Sorry for the delay. Thank you all for your contributions!

@phil-opp After closing this issue, you should mark the "Docker support" task in #360 as done too.

I updated the issue description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Potential improvements for the code or the blog. feedback wanted We want to hear opinions about this change. help wanted We could use help for this issue/pull request!
Projects
None yet
Development

No branches or pull requests

5 participants