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

docker_group set in init.pp is not referenced in docker::run #321

Closed
jeefberkey opened this issue Aug 30, 2018 · 7 comments
Closed

docker_group set in init.pp is not referenced in docker::run #321

jeefberkey opened this issue Aug 30, 2018 · 7 comments

Comments

@jeefberkey
Copy link

$docker_group = $docker::params::docker_group

I have set docker::docker_group a docker class declaration, but I am unable to use the docker::run define because it always checks the params class, which contains the incorrect default. My system uses dockerroot as the docker_group

@davejrt
Copy link
Contributor

davejrt commented Sep 4, 2018

Can you post an example of you're actually defining the class and where it isn't working?

If you're including this module this is how I'd expect it do be done, and I'd expect this to work.

class { 'docker':
  docker_group => 'dockerroot',
}

@jeefberkey
Copy link
Author

I have a profile module with acceptance tests here: https://github.com/simp/pupmod-simp-simp_docker/

In the default suite, I just include my profile and use a bunch of the defines in this module.

The profile module works by building up a hash of default params depending on a few facts, then using a splat operator. https://github.com/simp/pupmod-simp-simp_docker/blob/master/manifests/init.pp#L81-L83

@davejrt
Copy link
Contributor

davejrt commented Sep 6, 2018

Happy to take a PR with passing tests to fix this. We've had one previously that was submitted without tests passing so we weren't able to merge it at that point.

PR #250

@esalberg
Copy link
Contributor

esalberg commented Sep 6, 2018

I'm working on the spec tests for #250 currently - the strict variables are being difficult, but I'm making progress.

@jeefberkey
Copy link
Author

@esalberg thanks for pointing me there, I didn't look at PRs. I will follow that issue

@esalberg
Copy link
Contributor

esalberg commented Sep 6, 2018

@davejrt FYI - fixed the spec tests on #250 - if you could look at my question regarding overall strategy for the code, that would be great (e.g. including docker in docker::run).

@davejrt
Copy link
Contributor

davejrt commented Jan 16, 2019

I'm going to close this due to a lack of inactivity here and on the PR in question. The PR looked fine aside from the feedback that was originally left. Please feel free to reopen the PR addressing the feedback discussed and we'll be happy to look at getting it merged.

@davejrt davejrt closed this as completed Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants