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

Problems with debian packages. #264

Closed
linsomniac opened this issue Oct 14, 2015 · 9 comments
Closed

Problems with debian packages. #264

linsomniac opened this issue Oct 14, 2015 · 9 comments
Assignees

Comments

@linsomniac
Copy link
Contributor

Thank you very much for the debian packages. However, there are a few things I think need to be changed:

The package installs things into opt sub-directories of other directories: /etc/opt/telegraf. These are just the wrong places for them. Really the config file should go into /etc/telegraf.conf, or possibly /etc/telegraf/telegraf.conf (though since there is only one file, it doesn't make sense to make a directory.

The logrotate file needs to include "copytruncate", since it doesn't signal telegraf to re-open the log-file, otherwise it leaves no log file and the current telegraf instance keeps writing the existing (removed) file until it is restarted.

The logrotate file should probably also have "dateext" to match influxdb?

Typically, packages don't put things into /opt, the packages manage the installed files so you would just put the daemon into /usr/sbin, the init scripts either directly into /etc/init or /etc/init.d, or into /usr/share and copy/link them into /etc/init in the post install.

The package also lists a file "/.". That probably doesn't hurt anything but also could be cleaned up.

linsomniac added a commit to realgo/telegraf that referenced this issue Oct 14, 2015
This adds copytruncate and dateext to match the Influxdb logrotate file,
and removes nocreate (again, to match influxdb).
@sparrc
Copy link
Contributor

sparrc commented Oct 14, 2015

Can't say I disagree with these points, I believe opt is actually the technical standard, but in practice I find usage of opt kind of pointless and ignored by almost everyone.

@linsomniac I'll merge the PR you sent up, and then talk to others here at Influx and see if we can eliminate usage of opt as well

@linsomniac
Copy link
Contributor Author

See the discussion on influxdb for my similar report and PR:

influxdata/influxdb#4446

@sparrc
Copy link
Contributor

sparrc commented Oct 14, 2015

Thanks, can you send up another PR to fix the rest of the points you made?

@linsomniac
Copy link
Contributor Author

Probably. The package.sh script is a slightly unusual way of doing it (instead of using a debian/ directory), but I can probably figure it out. From the influxdb side of things, it sounds like it is something they are open to, so it sounds like I could have some traction if I made the changes I proposed. Does the proposal sound good to you?

@sparrc
Copy link
Contributor

sparrc commented Oct 14, 2015

yes, except please link the binary in /usr/bin instead of putting it in /usr/sbin

@sparrc sparrc closed this as completed in 04e2db1 Oct 14, 2015
linsomniac added a commit to realgo/telegraf that referenced this issue Oct 15, 2015
Issue influxdata#264: This attempts to fix the /etc/opt and put the binary
linked into /bin

Note: I can't seem to find the right combination of packages for
getting package.sh to run, so I haven't been able to test this.
@linsomniac
Copy link
Contributor Author

Please re-open.

@amoghe
Copy link
Contributor

amoghe commented Oct 26, 2015

Since this bug tracks issues with installing from a debian pacakge, I'll add this here:

On installation, I see this:

/var/lib/dpkg/info/telegraf.postinst: 9: /var/lib/dpkg/info/telegraf.postinst: [[: not found

Probably due to bash syntax in a file that is executed under sh?

Happy to open a new issue to track this if needed.

Details of downloaded deb file:

root@4ec6b49cb4ce:/opt/influxdb# md5sum /tmp/telegraf_0.1.9_amd64.deb 
3ed63f3b83a7561ce73afd6114e10fe8  /tmp/telegraf_0.1.9_amd64.deb

@sparrc
Copy link
Contributor

sparrc commented Oct 26, 2015

That issue has been fixed, it will be available in the next release

@rossmcdonald
Copy link
Contributor

Fixed with #497

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants