search

crops / poky-container

A container image that is able to run bitbake/poky. It has helpers to create users and groups within the container. This is so that the output generated in the container will be readable by the user on the host.
GNU General Public License v2.0
206 stars 94 forks source link

ENTRYPOINT compatibility and Ubuntu dash fix #37

Closed btgoodwin closed 4 years ago

btgoodwin commented 4 years ago

This merge adds two things:

  1. Extends the ENTRYPOINT behavior of #34 to include multi-line scripts by retaining line endings.
  2. Adds a check for dash to not use it where applicable because of compatibility problems with some layers and dash.

For (1), this also provides a fix for situations like #24. You can verify this by having a test.sh file:

sh -c if [ -x /usr/local/bin/bash ]; then
    exec /usr/local/bin/bash 
elif [ -x /usr/bin/bash ]; then
    exec /usr/bin/bash 
elif [ -x /bin/bash ]; then
    exec /bin/bash 
elif [ -x /usr/local/bin/sh ]; then
    exec /usr/local/bin/sh 
elif [ -x /usr/bin/sh ]; then
    exec /usr/bin/sh 
elif [ -x /bin/sh ]; then
    exec /bin/sh 
elif [ -x /busybox/sh ]; then
    exec /busybox/sh 
else
    echo shell not found
    # exit 1
fi

And then run the command:

docker run -it --name gitlabtest crops/poky:ubuntu-18.04 $(cat test.sh)
rewitt1 commented 4 years ago

Ok @btgoodwin,

I merged #34 and also made the updates I wanted. It'd be great if you could update this pull request, thanks!

btgoodwin commented 4 years ago

Ok @btgoodwin,

I merged #34 and also made the updates I wanted. It'd be great if you could update this pull request, thanks!

Yep -- I was definitely not trying to take credit for their work but wasn't sure how to contribute to fixing up that MR to address your concerns.

I rebased mine against your master, accepting your changes. For me, build-and-test.sh fails on run-workdir-check.sh:

docker: Error response from daemon: OCI runtime create failed: container_linux.go:345: starting container process caused "chdir to cwd (\"/workdir\") set in config.json failed: permission denied": unknown.

Any thoughts?

Edit: It's the first check where workdir is being passed to docker that fails.

rewitt1 commented 4 years ago

Ok @btgoodwin, I merged #34 and also made the updates I wanted. It'd be great if you could update this pull request, thanks!

Yep -- I was definitely not trying to take credit for their work but wasn't sure how to contribute to fixing up that MR to address your concerns.

I understand. I didn't think you were trying to take credit. I wanted to make sure you understood why I merged #34 the way I did.

I rebased mine against your master, accepting your changes. For me, build-and-test.sh fails on run-workdir-check.sh:

docker: Error response from daemon: OCI runtime create failed: container_linux.go:345: starting container process caused "chdir to cwd (\"/workdir\") set in config.json failed: permission denied": unknown.

Any thoughts?

Edit: It's the first check where workdir is being passed to docker that fails.

My guess is that it's your umask and so the docker daemon user(whatever it is) won't work if o+x isn't on for the directory. Now https://github.com/crops/poky-container/pull/34#discussion-diff-279552853R28 makes more sense. I need to add it back in, which I have no problem doing now that I know the reasoning.

I'll do that shortly and ping you again.

rewitt1 commented 4 years ago

@btgoodwin,

I pushed the change https://github.com/crops/poky-container/commit/7aa9f8fd8cb57fb621dc55558262da00354ac550. Try rebasing and see if it fixes the issue you referenced in https://github.com/crops/poky-container/pull/37#issuecomment-528395140.

btgoodwin commented 4 years ago

Yes, that fixed it for me. I've rewritten my history to add a comment about ubuntu and fixed the test we discussed earlier. It's all working fine on my end.

Edit: I failed to mention I added a patch for a nuisance warning/error that is caused by the underlying crops/yocto images. I haven't gotten around to submitting a patch for that since it sounds like there's a suggested fix already in the queue but it's stale also. I'm hoping by the end of testing this to publish something about how to use these images along with gitlab-ci to setup an internal build server. So far it's working pretty well; I just have a few kinks to fix.

rewitt1 commented 4 years ago

Edit: I failed to mention I added a patch for a nuisance warning/error that is caused by the underlying crops/yocto images.

Let's drop https://github.com/crops/poky-container/pull/37/commits/6d0a5f7a2796b3b6795a6d8d06eea7d919e3a3a2 from this pull request. If there is a problem in the scripts in https://github.com/crops/extsdk-container, let's fix it and discuss there.

I haven't gotten around to submitting a patch for that since it sounds like there's a suggested fix already in the queue but it's stale also

The "suggested fix" didn't get merged due to a lack of response.(partly my fault for being so busy) But I'm more than happy to try and get things working the way they need to facilitate different workflows I don't use myself.

btgoodwin commented 4 years ago

Edit: I failed to mention I added a patch for a nuisance warning/error that is caused by the underlying crops/yocto images.

Let's drop 6d0a5f7 from this pull request. If there is a problem in the scripts in https://github.com/crops/extsdk-container, let's fix it and discuss there.

I haven't gotten around to submitting a patch for that since it sounds like there's a suggested fix already in the queue but it's stale also

The "suggested fix" didn't get merged due to a lack of response.(partly my fault for being so busy) But I'm more than happy to try and get things working the way they need to facilitate different workflows I don't use myself.

Understood. I backed out that last commit and am prepping an PR for the other repo.

Edit: for the sake of continuity

btgoodwin commented 4 years ago

:( ... I'll re-look at that ubuntu/debian dash fix in the morning.

btgoodwin commented 4 years ago

My fixes all work but now I need #38 to eliminate that opensuse version so that the tests will pass.

btgoodwin commented 4 years ago

I was about to merge this and then realized this commit message is now stale. It doesn't borrow anything from #34 now, and no tests are changed.

Edit: To clarify, the commit message for 1e139f5

I've updated the commit message to reflect it.

rewitt1 commented 4 years ago

I was about to merge this and then realized this commit message is now stale. It doesn't borrow anything from #34 now, and no tests are changed. Edit: To clarify, the commit message for 1e139f5

I've updated the commit message to reflect it.

Did you forget to push? I don't see the changes here.

btgoodwin commented 4 years ago

Yep. I fixed the merge message but not the commit (done now). On the upside I have a working example of how to use CROPS in gitlab-ci.yml for standing up an in-house sstate cache and downloads mirror.

btgoodwin commented 4 years ago

Pleasure working with you. See you 'round 👍

a1fred0 commented 4 years ago

Yep. I fixed the merge message but not the commit (done now). On the upside I have a working example of how to use CROPS in gitlab-ci.yml for standing up an in-house sstate cache and downloads mirror.

@btgoodwin - I've also been trying to achieve this. When I use the latest image and your basic .gitlab-ci.yml example from #24 I get the following error: image

Any thoughts on what is wrong. I'd like to try and understand the issue.

btgoodwin commented 4 years ago

@a1fred0 - Yes, that's because of how those two restrict scripts are written in the upstream container. See my pull request here. Once it or some other similar change is merged, re-entering the container will not crash out like that.

Locally I'm running on my own fork of the crops/poky:ubuntu-18.04 that has all of these fixes already implemented. Until that PR is resolved, you're welcome to use it. It's on dockerhub under my company account, geontech/poky:ubuntu-18.04.

FWIW, my setup volume-mounts to the runner an NFS-shared path that the builds use for sstate_cache and downloads cache. For the gitlab CI file, I change the entrypoint like this:

.ubuntu: &ubuntu-env
  tags:
    - yocto
  image:
    name: geontech/poky:ubuntu-18.04
    entrypoint:  ["/usr/bin/dumb-init", "--", "/usr/bin/poky-entry.py", "--workdir", "/opt/yocto"]

Where /opt/yocto is that volume mount point (the yocto tag is so it grabs the correctly-configured runner).

I've managed to get all the way through a world build for qemuarm (but not qemuarm64) and then ran into the bigger problem of how to describe a "deploy" stage given that the artifacts in question are going to be huge. I think what I may end up doing instead of running scripts inside the container is to use docker-in-docker so that my gitlab-ci pulls and runs the CROPs container, and then runs docker exec ... against that container, stopping it, tagging it, and then pushing it to the container registry between stages. That alleviates the need for "artifacts" or "cache" settings, but does result in pushing multi-gigabyte containers around. I'd love to hear your thoughts on this.

a1fred0 commented 4 years ago

@btgoodwin - thanks for the help. I understand now.

the bigger problem of how to describe a "deploy" stage

My plan was to have two volume mounts on the dedicated runner, one for the sstate and downloads and a second for the build directory of yocto. My goal is to push the image and sdk to an artifactory/network share repository for teams to pick up and use for development. FWIW, I plan to create the build volume (on host) with no journaling mke2fs -t ext4 -O ^has_journal /dev/sdb1 to help with build times.

btgoodwin commented 4 years ago

@a1fred0 thank's for sharing; we have very similar goals. We have been debating this exact route too (the second volume mount against the runner's builddir) since it implies that only one runner exists (which is true, for now). Is your shared state/downloads volume also configured without journaling?

a1fred0 commented 4 years ago

@btgoodwin - Left journaling in place for downloads and sstate as these are less frequently updated once the intial CI job completes and prefer them to be robust against power outages. For now this MR does what is needed. Thanks for all your help.

btgoodwin commented 4 years ago

@a1fred0 A pleasure! Thanks for yours, too. Your comment about disabling journaling has us thinking about the whole docker volume we have -- whether it would be worth disabling journaling for higher performance across the board (runners and services alike). I haven't managed to google-fu a case study for that yet though.

Cheers!