Delete broken and unnecessary docker check

[dalehamel]

fixes https://github.com/hw-cookbooks/runit/issues/100

It looks like this check was introduced in https://github.com/hw-cookbooks/runit/pull/61 a while ago, but the actual reasoning for why it polling for supervise/ok was causing infinite loops was never given.

It might be that the underlying container filesystem didn't support pipes at the time, or maybe something to do with the way docker used to use lxc, but no reason is given.

There are no tests to support this logic either.

This check is preventing me from using the docker test kitchen driver. By removing this check, everything just works. Unless a concrete reason can be supplied for why this check is here, with test cases around it, I think that it should be removed, as I don't think it is valid anymore with a modern version of docker.

@slyness for review

@akranga can you provide more context on how the infinite loops were happening? Can you test this branch with a modern version of docker to see if the problem still happens?

I'd much prefer to solve this problem in a much less heavy handed way, if the problem still exists.

By updating the project kitchen.yml to use docker, I can now successfully run the test suite using docker here (which will actually probably make it easier to do a CI in travis with test kitchen now). So, I'm quite confident that this workaround is no longer needed, as the tests we do have pass without it , while using docker.

to reproduce my test kitchen tests with docker, here's the test kitchen file i've used: https://gist.github.com/dalehamel/9f801012342432224cf1725b3013e53c (a couple of them are commented out, because i haven't yet found docker images that will allow me to start systemd, which runsv requires for those distros)

fyi @coderanger

[dalehamel]

cc @dwradcliffe @andrewjamesbrown

[tas50]

It would be really great to get a release with this fix if it does indeed fix the kitchen-docker / kitchen-dokken issues. We'd love to get travis testing our PRs that involve runit.

[dalehamel]

@cwjohnston for review as well

[dalehamel]

@tas50 FYI i got this working in travis too:

https://github.com/hw-cookbooks/runit/compare/develop...dalehamel:docker-travis

https://travis-ci.org/dalehamel/runit/builds

There are a couple of builds that are commented out, this is not because of the docker driver but because the images being used for those builds don't have a proper init system running, which is needed to manage runsvdir, and start ssh for test kitchen. That is easily remedied by either finding an image that has systemd or whatever installed, or modifying the existing images likewise with kitchen-docker's provision_command

[cwjohnston]

Thanks for the PR @dalehamel.

It looks like this check was introduced in #61 a while ago, but the actual reasoning for why it polling for supervise/ok was causing infinite loops was never given. It might be that the underlying container filesystem didn't support pipes at the time, or maybe something to do with the way docker used to use lxc, but no reason is given.

In https://github.com/hw-cookbooks/runit/pull/138#issuecomment-115043195 @coderanger describes one reason for this change. To me it seems that the changes in #61 had the effect of optimizing for cases where folks are using this cookbook to build containers without operational init, for better or for worse.

I think that your proposed code removal is a good idea, in the sense that I perceive that it makes the cookbook more usable for a wider audience of people (e.g. test-kitchen users), but it also seems like a breaking change for users provisioning containers w/o init as described above. Perhaps this will merit incrementing the major version of the cookbook. I'm fine with that, as long as we also document the criteria for a compatible docker container. Can you provide some supporting links and/or add some detail on docker compatibility in the README?

[dalehamel]

I think that your proposed code removal is a good idea

:clap:

Perhaps this will merit incrementing the major version of the cookbook.

Makes sense to me!

I'm fine with that, as long as we also document the criteria for a compatible docker container. Can you provide some supporting links and/or add some detail on docker compatibility in the README?

Yes, i'll do that now.

[dalehamel]

@cwjohnston note to readme has been added.

FWIW a full docker file that can be used in travis is available on another branch: https://raw.githubusercontent.com/dalehamel/runit/d3cc50b28879a5e62c4c651493bf15eb85127703/.kitchen.docker.yml

That docker file is able to run all of the containers, but there is a test that is legitimately failing in them that I haven't yet had the time to debug.

Once this merges, I'll try debugging that further and will submit another PR to add travis support.

[cwjohnston]

Closes #148 Closes #171

[dalehamel]

Anything else needed to see this merged and released?

On Thursday, 2 June 2016, Cameron Johnston notifications@github.com wrote:

Closes #148 https://github.com/hw-cookbooks/runit/pull/148 Closes #171 https://github.com/hw-cookbooks/runit/pull/171

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hw-cookbooks/runit/pull/193#issuecomment-223345037, or mute the thread https://github.com/notifications/unsubscribe/AAlwd-nM1UJaWixfowqeihfnsnBv_QIRks5qHwPFgaJpZM4Ir_nX .

[cwjohnston]

@dalehamel I've merged this change but I am looking to close out the remaining issues in https://github.com/hw-cookbooks/runit/milestones/2.0.0 before cutting a new release. I just merged #192, adding a .travis.yml to the develop branch, running foodcritic, rubocop and rspec successfully. It currently skips installing the integration group; feel free to modify that in your PR to add .kitchen.travis.yml.

[cwjohnston]

@dalehamel is the failure you're experiencing similar to the one below? If so, I believe this can be solved by explicitly installing the file package in runit_test::service recipe.

       Failures:

         1) runit_test::service on {:family=>"redhat", :release=>"5.11", :arch=>"x86_64"} behaves like common runit_test services creates a service that uses the default svlog Command "file /var/log/default-svlog/*.s" stdout should contain "gzip compressed data"
            Failure/Error: DEFAULT_FAILURE_NOTIFIER = lambda { |failure, _opts| raise failure }
              expected "" to contain "gzip compressed data"
              /bin/sh -c file\ /var/log/default-svlog/\*.s

            Shared Example Group: "common runit_test services" called from /tmp/verifier/suites/serverspec/linux_spec.rb:50
            # /tmp/verifier/suites/serverspec/service_example_groups.rb:45:in `block (4 levels) in <top (required)>'

       Finished in 3.79 seconds (files took 0.47539 seconds to load)
       126 examples, 1 failure

       Failed examples:

       rspec /tmp/verifier/suites/serverspec/linux_spec.rb[1:2:3:4:1:1] # runit_test::service on {:family=>"redhat", :release=>"5.11", :arch=>"x86_64"} behaves like common runit_test services creates a service that uses the default svlog Command "file /var/log/default-svlog/*.s" stdout should contain "gzip compressed data"

edit: similar error on debian-like platforms indicates that binutils is required to provide strings.