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 service_provider parameter to docker::run #376

Merged
merged 1 commit into from
May 2, 2019

Conversation

jameslikeslinux
Copy link
Contributor

I originally tried to contribute this in #86, but it was rejected due to some planned changes to OS handling logic. I believe those changes were made in #129 so I thought I'd rebase my work a year later and try again.

This MR unlocks existing functionality in the module. It does not add new features or increase the complexity of the module. It simply exposes a variable to the user that is already there. Specifically, the $service_provider parameter is exposed in the docker::run defined type, just as it is in the main docker class. I've taken care to write good unit tests for the behavior added by this change. I think you'll find the diff is quite clean.

This allows users of other systemd-based distributions to use this module in an explicitly unsupported fashion. For example, this module works 100% on Gentoo just by supplying some simple parameters, like in https://github.com/iamjamestl/puppet-nest/blob/9b097dcab71365fbbfc584d719119cc4aa1ec5d1/manifests/docker.pp#L7.

@davejrt
Copy link
Contributor

davejrt commented Dec 4, 2018

Apologies for the delay. I've spoken with the product team we and our support matrix will continue as it currently is, with our efforts focused on centos, ubuntu and rhel.

@jameslikeslinux
Copy link
Contributor Author

jameslikeslinux commented Dec 4, 2018

That is a sincere disappointment and an insult to the open source process and me personally as a Puppet advocate since 2009. This contribution is compatible with the abstraction model advanced by Puppet and should not have any impact on your ability to officially support enterprise distributions.

For what it's worth, I am a systems engineer at a major university where I lead the Puppet deployment and use the docker module on RHEL. This kind of rebuffing of community contributions makes me less interested in pursuing an enterprise license for my organization.

@jameslikeslinux
Copy link
Contributor Author

I want to acknowledge that I know Puppet is under no obligation to consider all community contributions or support every OS under the sun. I am upset because this is the second time I didn't even get the courtesy of a review on the technical merits. I believe Puppet's view on this issue is incompatible with the modern enterprise and will be harmful to the company in the long run.

Anecdotally, I am able to confidently promote the use of Puppet in my organization, including running major production services on Docker using this module on RHEL 7, because I experimented with it all years beforehand on a non-enterprise OS where I have access to the latest tech. Puppet has taken this community module, stripped out all of the non-enterprise OS support and artificially limited its compatibility with an if and a fail statement. It's an outdated view of modern Linux in which systemd is now more important than the distro itself. A modern Gentoo or Arch system has more in common with RHEL 7 than RHEL 7 does with RHEL 6.

Puppet's artificial restrictions placed on this module tell me that Puppet doesn't value the enthusiasts who make their product popular. And in my case, when I can't freely experiment with popular public modules without first removing a bunch of if statements, it affects my ability to learn and then do my job effectively, and that reduces my confidence in Puppet overall.

@carabasdaniel
Copy link
Contributor

Hi @iamjamestl , apologies for the late reply.

As we do with our other modules, please note that we will not officially support Operating Systems that do not have Agent support, but we will accept PRs from the community that do not brake the current functionality provided by the module.

Thank you for your contribution and please note that we are trying to merge this as soon as possible. Would you please rebase your changes to fix the existing conflicts ?

We sincerely apologies for the difficulties that you've encountered trying to contribute. We've taken note of this incident and we're going to try to do better job in the future.

This module already has the code to support all systemd-based
distributions, yet it is artificially locked to just Red Hat and
Ubuntu/Debian.  By exposing the `service_provider` parameter in the
`docker::run` defined type (a parameter that is also exposed in the main
`docker` class), this module becomes usable on most other distributions.

Other minor fixes have been added to set missing variables when running
under a non-Debian or non-RedHat operating system.
@jameslikeslinux
Copy link
Contributor Author

Thank you for the consideration, @carabasdaniel. I have rebased my branch and fixed the conflicts.

@carabasdaniel carabasdaniel merged commit a77179b into puppetlabs:master May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants