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

## v0.7.0 #5

Closed
wants to merge 12 commits into from
Closed

Conversation

atomic-penguin
Copy link

  • General clean up, per Foodcritic compliance
  • Fix several minor platform bugs on RedHat/CentOS
  • Change service assumptions regarding Redhat/Centos
  • Added several missing dependencies
  • Fenced off debian/ubuntu specific cases in case switches
  • Changed sshkey generation to use pure Ruby via gem, instead of execute.
  • Made basic auth optional in proxy_apache2, as Jenkins comes with several of its own auth plugins

* General clean up, per Foodcritic compliance
* Fix several minor platform bugs on RedHat/CentOS
* Change service assumptions regarding Redhat/Centos
* Added several missing dependencies
* Fenced off debian/ubuntu specific cases in case switches
* Changed sshkey generation to use pure Ruby via gem, instead of execute.
* Made basic auth optional in proxy_apache2, as Jenkins comes with several of its own auth plugins
@guilhem
Copy link

guilhem commented May 29, 2012

nice patch

@atomic-penguin
Copy link
Author

@guilhem I did a lot of work with this on Friday (http://imgur.com/dKDVE). Figured I would share my progress. I'm not sure about the ruby_blocks to block_until_operational, and status check on netstat. That seems almost like a guard against an edge case for a broken service provider on some platform? Therefore I didn't touch those bits which I didn't fully understand.

gbloquel and others added 9 commits June 12, 2012 15:41
The variables home, user, port can be updated.
* Add a few attribute knobs for java_options, pid and war file locations.
* Make sysconfig template platform neutral for redhat/debian distributions
Fix missing terminating double quote
Fix missing ERB output
If using a persistent volume that will end up migrating from node to
node, such as an EBS volume on EC2, then the cookbook will now always
defer to the files on disk and not overwrite them. It will also update
the node pubkey setting from the data on disk.
@patcon
Copy link
Contributor

patcon commented Jul 11, 2012

Really like a lot of this, but thoughts on how I could put the work into splitting it out for easier audit by the maintainer? I can go through, but figured the author would know best. Also, changelog updates are awesome, but i think it's normal practice in pull requests in to leave versioning untouched and leave that to the maintainer, right?

Thoughts would be

  • foodcritic/formatting
  • rhel/centos support (with ubuntu switches)
  • optional basic auth (I unknowingly duplicated effort in Allow basic auth to be optional in apache conf #11, if you think that looks ok -- saves need for extraneous attr)
  • ssh key generation
  • I'd move those metadata deps into recommends or suggests -- no need to force them on folks who might not use them :)

@atomic-penguin
Copy link
Author

...thoughts on how I could put the work into splitting it out for easier audit by the maintainer? I can go through, but figured the author would know best.

Did AJ ask for your help with heading up that effort? Don't get me wrong, if you want to cherry-pick these into separate pull requests go ahead. I'm still working on another branch of this which incorporates the WAR refactor pull request changes. I think this is still sitting here, because the fine folks working on this cookbook are busy, not because the changes are too many, or hard, to decipher.

...i think it's normal practice in pull requests in to leave versioning untouched and leave that to the maintainer, right?

Its normal practice in that Opscode asks contributors not to bump versions in their pull requests. Its quite normal for me to bump versions, so I can discern the difference in versions for my own use.

optional basic auth (I unknowingly duplicated effort in #11, if you think that looks ok -- saves need for extraneous attr)

I think .nil? conditionals would be sufficient in this case. The reason I added a boolean attribute was to maintain backwards compatibility. The cookbook still ships with a default username/password of jenkins/jenkins in this case.

I'd move those metadata deps into recommends or suggests -- no need to force them on folks who might not use them :)

There is a ticket open for discussion on the dependency mapping issue CHEF-3282 to make this feature platform_family aware. I think that addresses your concern, in this case, appropriately.

The recommends and suggests metadata have always been non-functional dummy options. You put the onus on the end user to enforce dependency mapping through external roles, instead of codifying those dependencies within your cookbook. As it stands, I would rather have unused configuration artifacts (cookbooks) downloaded to my nodes. As opposed to accidentally missing a required library/provider component, due to a soft dependency not being explicitly mapped during the compilation. After all, we can always add platform conditional guards to keep inappropriate artifacts from executing on our node, for example, runit or apt on the Redhat platform.

@atomic-penguin
Copy link
Author

Elaborating on the part about the WAR refactor, and I do like the idea of that direction. As it stands, there isn't a suitable process monitor (cookbook support) for the redhat platform, yet.

Another option would be to run jenkins as a tomcat managed service, just add WAR. Yet another option would be to factor out common parts of package/war recipes, and let the end user choose the install method. The direction this might be taking may make it harder to signal an inconsistent service interface for a configuration reload, or restart. Just my two cents, on what I've been thinking about.

@patcon
Copy link
Contributor

patcon commented Jul 12, 2012

Did AJ ask for your help with heading up that effort?

No, it was just that I noticed some pulls requests seemed stagnant, and I had some spare time to do housekeeping :)

Its quite normal for me to bump versions

sounds good.

I think .nil? conditionals would be sufficient in this case.

I mentioned it in the issue, but nils don't work, as you apparently can't override a non-nil default with nil in a role. And my pull request is also backwards compat -- nothing different until you override default username or password to be an empty string (doc'd in README).

As it stands, I would rather have unused configuration artifacts (cookbooks) downloaded to my nodes.

I'll defer to you, but I do disagree. I was the one who opened the issue that resulted in that discussion you linked -- mysql cookbook required windows cookbook and broke the chef run. But honestly, my main interest is more meta. I use librarian to build projects to share with folks who might never have used chef before. Having 20 cookbooks when only 4 are needed can over-complicate those projects. I'd like to keep them as simple as possible. If everyone listed any possible cookbook as a dep, then I feel it would get out of hand pretty quickly. I'd simply rather those deps weren't imposed on me :)

If you still feel strongly, that's cool.

@atomic-penguin
Copy link
Author

I mentioned it in the issue, but nils don't work, as you apparently can't override a non-nil default with nil in a role. And my pull request is also backwards compat -- nothing different until you override default username or password to be an empty string (doc'd in README).

That totally makes sense. Either way would be fine with me.

I'll defer to you, but I do disagree. I was the one who opened the issue that resulted in that discussion you linked -- mysql cookbook required windows cookbook and broke the chef run. But honestly, my main interest is more meta. I use librarian to build projects to share with folks who might never have used chef before. Having 20 cookbooks when only 4 are needed can over-complicate those projects. I'd like to keep them as simple as possible. If everyone listed any possible cookbook as a dep, then I feel it would get out of hand pretty quickly. I'd simply rather those deps weren't imposed on me :)

If you still feel strongly, that's cool.

Its great that you opened the issue, I think it is a brilliant idea and the best solution as an alternative to soft dependencies. Having dependencies mapped out explicitly per platform would solve the problem of having dependencies stated for correctness, and also solve the problem of pulling in unneeded components on edge case platforms.

I totally agree that it is a problem. Its not that I feel strongly about it either, its a principle of least surprise and reusability. Not everyone is using solutions like Berkshelf or Librarian. On the one hand, you end up leaving the responsibility of managing dependencies on the user, by declaring them as soft dependencies. In that sense, the dependencies only exist as a form of documentation in metadata.

On the other hand when declared as hard dependencies, someone using Librarian might have this problem externalized and have to resort to editing out the stated dependencies which are unwanted. I completely understand where you are coming from, having been to dependency hell, all the while squashing bugs in cookbooks I don't even need/use.

@patcon
Copy link
Contributor

patcon commented Jul 12, 2012

Firstly, thanks for putting Berkshelf on my radar!

Just wanted to explain my angle, but I'm fine with whatever gets committed. :)

Always defer to private/public keys on filesystem
@atomic-penguin
Copy link
Author

Abandoning this for now as I haven't had time to work on it in over 5 months.

Will re-open a pull request if that changes.

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

Successfully merging this pull request may close these issues.

4 participants