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

War Conversion #4

Closed
wants to merge 2 commits into from
Closed

War Conversion #4

wants to merge 2 commits into from

Conversation

damm
Copy link

@damm damm commented Apr 12, 2012

AJ,

My apologies about getting this to you in a timely manner. I likely did a few :/ things but overall should be good enough.

Thanks

@webframp
Copy link

looks pretty good, thanks! Hopefully AJ can review today when he comes online

@fujin
Copy link
Contributor

fujin commented Apr 13, 2012

I'm not super keen on this shipping an init script -- mostly because I'd have to maintain it. How about Runit?

Some of the other changes we specifically adjusted from earlier versions of this cookbook have slipped back in. Thanks for the PR, will work through it.

@damm
Copy link
Author

damm commented Apr 13, 2012

Perfectly fine, I don't have time today to get to it today so i'll have find time this weekend hopefully.

@fujin
Copy link
Contributor

fujin commented Apr 13, 2012

May just accept this as is and internalize the modifications I have proposed if you don't have the time for it -- then we can cut a new version and release it once we have both reviewed. Sound good?

@damm
Copy link
Author

damm commented Apr 13, 2012

sounds good

@guilhem
Copy link

guilhem commented May 29, 2012

Sorry but the apt way to install MUST be the default for debian and Ubuntu.

If you want a manual way to install, chef is enough powerful to let you choose an option for it.

@gregsymons
Copy link

Then submit a patch to do so. This changes from using the Jenkins apt-repo (not the ubuntu or debian default repo) to install to using the war file. If I understand damm's commit comment, the Jenkins apt package had some issues with preserving user configuration changes as a good package should.

The Ubuntu package is significantly out of date with respect to the stable Jenkins release (1.409.1 on oneiric vs. 1.447.1 on the jenkins website as of today) . I'd prefer to be using a recent version of Jenkins, myself. Also, apt and applications with plugins don't play that well together, IMO. Finally, the Ubuntu package deploys Jenkins inside of Tomcat rather than running the standalone war, which just adds unnecessary system dependencies.

@guilhem
Copy link

guilhem commented May 30, 2012

I use the Jenkins apt package since a long time in production (for debian and ubuntu) and I don't see any problems (with many plugins / configurations / nodes ...)

Maybe some more information can be a good start !

PS : jenkins package is only a package with init.d, logrotate, logfolder create, run folder... AND .war file.
This patch do exactly the same work as the package... but without the strength of an apt package and with duplicate work.
( have a look with file-roller or 7zip... to this file it's quite easy to understand : http://pkg.jenkins-ci.org/debian/binary/jenkins_1.466_all.deb )

@damm
Copy link
Author

damm commented May 30, 2012

When you modify /etc/default/jenkins you need to apply UCF rules to purge it from the list or when the cookbook tries to run apt-get install jenkins again (Upgrade?) you should get a prompt to accept or reject changes to /etc/default/jenkins.

This behavior does not work properly in Chef and can cause you to have to run apt-get -f install and manually clean it up before the packaging system is useable again.

Now we can patch the UCF Behavior in the Chef recipe to work around that. However I was told that the WAR Pull request would be gladly accepted as that would help make this cookbook work for a wider audience.

@fujin if your still running low on time I can work in some time now to make it work under runit.

Thanks

@guilhem
Copy link

guilhem commented May 30, 2012

it's the same problem for ALL the apt packages. Glad that everyone don't bypass it with runit.

runit is a good things in option (any new feature is a good thing), but the apt-package must be the rule (maybe with options "--force-yes").

@damm
Copy link
Author

damm commented May 30, 2012

@guilhem --force-yes isn't valid here. If you read the above comment I mention how to fix the problem. As this pull request hasn't been accepted yet is there anything we can help you here with? or did you just want to voice your opinion that the war package isn't valid?

@guilhem
Copy link

guilhem commented May 30, 2012

IMPOV, your patch must be like this :
in default :

if(node[:jenkins][:install_method] == 'war')
  include_recipe 'jenkins::war'
else

in attribute.rb ->

default[:jenkins][:install_method] = 'package'

see nginx official cookbook for a functionnal example.

of course all you work must be in the recipe "war.rb"

@damm
Copy link
Author

damm commented May 30, 2012

If you wish to enact your opinion you should fork said cookbook and make that change yourself and then re-submit the pull request. However I don't see what value your adding here at this time?

@guilhem
Copy link

guilhem commented May 30, 2012

I don't need your patch, at the opposite, your patch break ALL the current install of anyone.

So patch it in a good way for all (as a pull request must be) and everyone will be happy.

status_command "test -f #{pid_file} && kill -0 `cat #{pid_file}`"
action [:start,:enable]
end
execute "start jenkins" do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused by this execute block here. Why do we have to explicitly try to start it? Shouldn't that be handled by the service block directly above? Also, why do we now have two service blocks?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These'll be purged when I add the s6 supervisor support. Cheers!

@fujin
Copy link
Contributor

fujin commented May 30, 2012

@guilhem while I appreciate your interaction with the community none of your suggestions mirror any of Heavy Water's practices or principles or even client request approximations. We have many disparage installations of Pennyworth and managing them all via .WAR will reduce significant complexity -- it was my intention to complete this work eventually regardless ("on the roadmap").

One of our major gripes with the upstream packages is the extraneous junk they include and of course the implicit behaviour of starting the service, leading us to perform multiple tricks during the automation to install plugins and restart Jenkins and then poll it until it is correctly available. With a WAR, we'd simply put the plugin on disk before warming up Jenkins -- the package wouldn't auto start it and need to be restarted, polled, etc.

I'll be pulling in the other outstanding patches, pulling this in, and modifying it to use the s6 skarnet's small and secure software supervision suite instead of Runit, to match the rest of our supervised services. [0][1]

Eventually Heavy Water will offer a public repository for s6 packages statically compiled for Debian/Ubuntu, instead of our current compile-from-source model (even though it is incredibly fast and lightweight, I'd prefer not to rely on compilation at all)

Thanks for playing along!

[0] http://www.skarnet.org/software/s6/
[1] https://github.com/heavywater/chef-s6

@guilhem
Copy link

guilhem commented May 30, 2012

@fujin I understand your needs. OpenSource is the place where everyone can find his happiness.
However, your needs aren't all needs (as mine).

Separate your work in a "war" recipe that you can call with an attribute or directly recipe is a good solution IMPOV.
If you need some help, don't hesitate. (But when I see your work, I'm sure that it will not take to much time ^^)

For s6, I will have a look to it. But for the moment I migrate many of my services to upstart (for the "beauty" of his conf files) and, when Ubuntu will include it, to systemd for its cgroup capability...
Yet Another Init Alternative... too much for me :)

@fujin
Copy link
Contributor

fujin commented May 31, 2012

@guilhem thanks for your input. for a time, we will adopt a model as you suggested, supporting multiple installation methods. the Package based installation method will be deprecated and we will likely not devote resources to enhancing it any further and will make use of WAR internally. Perhaps I'll abstract the service supervisor portion as well.

@@ -36,7 +36,7 @@
end

default[:jenkins][:server][:port] = 8080
default[:jenkins][:server][:host] = node[:fqdn]
default[:jenkins][:server][:host] = "0.0.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double-plus-good!

@patcon
Copy link
Contributor

patcon commented Jul 12, 2012

When you modify /etc/default/jenkins you need to apply UCF rules to purge it from the list or when the cookbook tries to run apt-get install jenkins again (Upgrade?) you should get a prompt to accept or reject changes to /etc/default/jenkins.

This behavior does not work properly in Chef and can cause you to have to run apt-get -f install and manually clean it up before the packaging system is useable again.

Hey @damm, I totally get that the WAR makes sense for now, but think we could try to make some headway on helping the jenkins package maintainers fix the issue? This is new territory for me, so wondering if it's known to be a slow process, or something like that

Thanks guys!

@patcon
Copy link
Contributor

patcon commented Jul 12, 2012

8:15p - patcon:
hey guys, wondering who the debian package maintainer is for the official jenkins package repo
8:15p - patcon:
trying to figure out whether these sysadmin concerns might be able to be dealt with: https://github.com/heavywater/chef-jenkins/pull/4#issuecomment-6011742
8:15p - kohsuke:
patcon: one in Ubuntu is jamespage. pkg.jenkins-ci.org is me
8:16p - patcon:
thanks kohsuke. are you familiar with the concern about /etc/default/jenkins and UCF behavior?
8:17p - patcon:
• patcon is parroting the sysadmins who actually understand the issue :)
8:17p - patcon:
kohsuke: but happy to do some learning to figure out a solution, if possible
8:17p - kohsuke:
patcon: no, I'm not familiar with UCF
8:17p - kohsuke:
let me read it
8:18p - patcon:
kohsuke: it's long. you're busy. I'll let them know you'll give it an ear, and see if they can summarize later
8:18p - kohsuke:
I'm willing to merge pull requests to the debian packages so long as those who know what they are doing can vouch for it
8:18p - patcon:
dealio. thanks kohsuke

Anyone intersested in this approach in the longterm? Fix the issue at it's origins? As I said, I'll take a stab, but if someone understands the relevant components and can do it in their sleep...

Cheers

@fujin
Copy link
Contributor

fujin commented Jul 12, 2012

Sounds like the most effective way forward. I'm preparing a v2 branch
of this which will do.. Much less than this? A little more
opinionated, but retaining attribute compatibility.

I hadn't noticed the issues @damm was calling out (ever), but I
generally nuke Jenkins and have it rebuilt (phoenix)

On 12 July 2012 12:22, Patrick Connolly
[email protected]
wrote:

8:15p - patcon:
hey guys, wondering who the debian package maintainer is for the official jenkins package repo
8:15p - patcon:
trying to figure out whether these sysadmin concerns might be able to be dealt with: https://github.com/heavywater/chef-jenkins/pull/4#issuecomment-6011742
8:15p - kohsuke:
patcon: one in Ubuntu is jamespage. pkg.jenkins-ci.org is me
8:16p - patcon:
thanks kohsuke. are you familiar with the concern about /etc/default/jenkins and UCF behavior?
8:17p - patcon:
• patcon is parroting the sysadmins who actually understand the issue :)
8:17p - patcon:
kohsuke: but happy to do some learning to figure out a solution, if possible
8:17p - kohsuke:
patcon: no, I'm not familiar with UCF
8:17p - kohsuke:
let me read it
8:18p - patcon:
kohsuke: it's long. you're busy. I'll let them know you'll give it an ear, and see if they can summarize later
8:18p - kohsuke:
I'm willing to merge pull requests to the debian packages so long as those who know what they are doing can vouch for it
8:18p - patcon:
dealio. thanks kohsuke

Anyone intersested in this approach in the longterm? Fix the issue at it's origins? As I said, I'll take a stab, but if someone understands the relevant components and can do it in their sleep...

Cheers


Reply to this email directly or view it on GitHub:
#4 (comment)

@patcon
Copy link
Contributor

patcon commented Mar 1, 2013

See #23 :)

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.

7 participants