go-graphite / carbonzipper

proxy to transparently merge graphite carbon backends
Other
104 stars 29 forks source link

Initial pull request to add RPM building capability #28

Closed salekseev closed 7 years ago

joelrebel commented 7 years ago

@salekseev This works well, I have a suggestion, since go build is run before the rpmbuild process, is there any reason to not use https://github.com/jordansissel/fpm instead of rpmbuild, using fpm would mean builds for debian and others can be supported as well :)

salekseev commented 7 years ago

Couple of reasons actually and I do use FPM for some other projects.

  1. FPM does require rpm-build to be installed as well.
  2. FPM requires ruby 1.9+ and CentOS/RedHat 6+ ships with 1.8.7. This is my target platform to deploy carbonzipper so I'm a bit egoistic about it. :)
tehlers320 commented 7 years ago

For anybody wanting to package an rpm here is an FPM line. fpm -s dir -t rpm -v ${VERSION} --config-files etc/carbonzipper/carbonzipper.conf --url 'https://github.com/dgryski/carbonzipper' -n carbonzipper -a x86_64 --description "carbonzipper proxy for carbonserver" --provides carbonzipper -C /fpm/ -p ${PACKAGE_NAME} .]

replace /fpm/ with wherever you built and put the files.

salekseev commented 7 years ago

Other than the no-quite complete init script are there any concerns with merging this code?

Civil commented 7 years ago

@tehlers320 is right and you need to move everything out of /usr/local to /usr/bin and /etc.

Also, there should be a support of RHEL/CO 7 or newer and by that I mean systemd unit.

Personally I'd prefer to have deploy renamed to contrib/rh to make a room to add an example deb package files.

I'm also not sure about having _vendor folder, which makes it incompatible with the go-vendoring stuff. But I hope @dgryski will suggest something here.

dgryski commented 7 years ago

I agree we should go with vendoring, however submodules is not the right way to do that. (We're using them at $WORK and were I to do it again I would choose something different.)

As for a particular tool, I will wait until dep is merged into the Go toolchain with 1.9 to avoid having to do it twice.

Civil commented 7 years ago

I've created a test packages for CentOS 6, can you please check them and tell me if you are satisfied with them? https://packagecloud.io/app/go-graphite/autobuilds/search?dist=el%2F6

For now I'm building packages using fpm. There are several CentOS6 specific steps I'm doing to do that:

  1. I'm installing ruby193 from centos-sclo-rh repo, to build fpm.
  2. For CarbonAPI I'm building my own static version of cairo (1.14.8 at this moment, without any extra patches), because CentOS 6 have cairo 1.8, but carbonapi requires 1.11.1+.

I've also built carbonzipper and carbonapi for CentOS 7, Ubuntu 14.04, Ubuntu 16.04: https://packagecloud.io/go-graphite/stable - for stable releases https://packagecloud.io/go-graphite/autobuilds - for autobuilds (master)

Civil commented 7 years ago

Ok, I've added autobuilds and release builds for CentOS 6, CentOS 7, Ubuntu 14.04, Ubuntu 16.04

Can you please check if RPMs I've produced for CentOS 6 works ok for you?

You can find latest autobuilds here: https://packagecloud.io/go-graphite/autobuilds

salekseev commented 7 years ago

Well, it would be great to have CarbonZipper built as a static binary for Centos 6 as lack of newer cairo makes it not very useful otherwise. Also missing startup/initV script.

Civil commented 7 years ago

Package is statically linked with cairo 1.14. but you are right, I've missed in it script

salekseev commented 7 years ago

Well, I honestly didn't get past RPM requires that includes cairo 1.11+, for example https://packagecloud.io/go-graphite/autobuilds/packages/el/6/carbonzipper-0.72-4.gba69.x86_64.rpm. If it's compile statically then maybe that RPM requires could be lifted as well.

Civil commented 7 years ago

Oopps, that's a bug in my build script. I thought I've got rid of that before building the packages.

I still wanted to depend on cairo, just drop the version requirement, otherwise I'll need to explicitly specify all it's deps (pixman, etc)

I'll try to fix it tonight.

Civil commented 7 years ago

In current master I've fixed dependency for carbonzipper and added an init script for CentOS 6. It's mostly based on your work (I've fixed few minor things + /usr/local/bin -> /usr/bin).

Can you please check if it works correctly?

Civil commented 7 years ago

As there was no response, I'm assuming that it's fixed.