Bug 240882

Summary: sysutils/consul: Update to 1.6.2 and refactoring rc.script
Product: Ports & Packages Reporter: Dmitry Wagin <dmitry.wagin>
Component: Individual Port(s)Assignee: Steve Wills <swills>
Status: Closed FIXED    
Severity: Affects Some People Flags: bugzilla: maintainer-feedback? (swills)
Priority: ---    
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
consul.diff
none
consul.diff none

Description Dmitry Wagin 2019-09-27 20:36:16 UTC
Created attachment 207901 [details]
consul.diff

* configurable on build stage user, group and datadir
* added Consul debugging to syslog
* added Consul config validation (optional)
* renamed consul_dir -> consul_dir (with backward compatibility) for better corresponding with the configuration settings, and official documentation
Comment 1 Steve Wills freebsd_committer freebsd_triage 2019-09-27 22:50:34 UTC
(In reply to Dmitry Wagin from comment #0)
These changes still have the same issues I mentioned previously. Please revise.
Comment 2 Dmitry Wagin 2019-09-28 06:15:25 UTC
(In reply to Steve Wills from comment #1)

Please cite your claims in this PR, past posts give the impression that you haven't read daemon manual.

Please do not delete discussed fixes, they can be used by other people.
Comment 3 Steve Wills freebsd_committer freebsd_triage 2019-09-28 10:09:04 UTC
(In reply to Dmitry Wagin from comment #2)
There's no need to run daemon(8) as root, or specify the -u flag to daemon(8), since rc.subr(8) handles changing the user for you. And there's no need to package the datadir, it can be created at startup time.
Comment 4 Dmitry Wagin 2019-09-28 11:46:42 UTC
(In reply to Steve Wills from comment #3)
> There's no need to run daemon(8) as root, or specify the -u flag to daemon(8), since rc.subr(8) handles changing the user for you.

Of course I know that rc.subr(8) does use su(1) for change user, but it not create, lock and *remove* pidfile. for that need ugly hack with start_precmd.


> And there's no need to package the datadir, it can be created at startup time.

Most ports that store data create directories to store it during the packaging stage. It is very convenient to rely on this behavior when deploying and creating jail images etc.
Comment 5 Steve Wills freebsd_committer freebsd_triage 2019-09-28 14:39:19 UTC
(In reply to Dmitry Wagin from comment #4)

> Most ports that store data create directories to store it during the packaging stage. It is very convenient to rely on this behavior when deploying and creating jail images etc.

They should only do that if the dir needs to contain a sample config file. This does not have a sample config file, and it isn't needed.
Comment 6 Dmitry Wagin 2019-09-28 14:55:34 UTC
(In reply to Steve Wills from comment #5)

> They should only do that if the dir needs to contain a sample config file. This does not have a sample config file, and it isn't needed.

this is a directory for data, not just for configuration.
Comment 7 Steve Wills freebsd_committer freebsd_triage 2019-09-28 15:53:26 UTC
(In reply to Dmitry Wagin from comment #6)
Ok, but that doesn't change the fact the empty directory isn't needed in the package.
Comment 8 Dmitry Wagin 2019-09-28 16:07:02 UTC
(In reply to Steve Wills from comment #7)

Most ports that store data create directories to store it during the packaging stage. It is very convenient to rely on this behavior when deploying and creating jail images etc.

for example:
net-p2p/libswift
sysutils/anvil
sysutils/ipfs-go
sysutils/stanchion
sysutils/beats
sysutils/rubygem-hiera
sysutils/nut
...

I'm tired of listing
Comment 9 Dmitry Wagin 2019-09-28 16:08:07 UTC
(In reply to Dmitry Wagin from comment #8)

~200 ports
Comment 10 Steve Wills freebsd_committer freebsd_triage 2019-09-28 22:52:51 UTC
(In reply to Dmitry Wagin from comment #9)
Yeah, there's a lot of cleanup to do. Looking around, I found a lot of silly things, like xdm packaging /var/db as just one example.
Comment 11 Dmitry Wagin 2019-09-29 10:16:52 UTC
(In reply to Steve Wills from comment #10)

I don't think so. People do so because it is convenient to use. Its friendly for admins. In your opinion, i fixed the rc.script just like that, from the absence of other work.

What do you suggest: After Consul install, besides reading the documentation, I must create configuration directory, create data directory, generally do a lot of dumb work.

Just because you are not using the configuration files, only cli parameters?
Comment 12 Steve Wills freebsd_committer freebsd_triage 2019-09-29 13:05:14 UTC
(In reply to Dmitry Wagin from comment #11)
I don't understand your question. I use the config file. That doesn't mean everyone should or has to. And the config file could be in any directory as long as the user specified in consul_user can read it.
Comment 13 Dmitry Wagin 2019-12-15 03:20:13 UTC
Created attachment 209961 [details]
consul.diff

* update to 1.6.2
* configurable on build stage user, group and datadir
* added Consul debugging to syslog
* added Consul config validation (optional)
* renamed consul_dir -> consul_data_dir (with backward compatibility) for better corresponding with the configuration settings, and official documentation
Comment 14 commit-hook freebsd_committer freebsd_triage 2020-01-04 00:43:27 UTC
A commit references this bug:

Author: swills
Date: Sat Jan  4 00:42:40 UTC 2020
New revision: 521969
URL: https://svnweb.freebsd.org/changeset/ports/521969

Log:
  sysutils/consul: Update to 1.6.2 and refactoring rc.script

  PR:		240882
  Submitted by:	Dmitry Wagin <dmitry.wagin@ya.ru> (with changes)

Changes:
  head/sysutils/consul/Makefile
  head/sysutils/consul/distinfo
  head/sysutils/consul/files/consul.in