Bug 208940 - New port: net/ntpa - A tool dedicated to analyze the operation of time servers
Summary: New port: net/ntpa - A tool dedicated to analyze the operation of time servers
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Guido Falsi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-20 21:00 UTC by Carsten Larsen
Modified: 2016-05-28 16:56 UTC (History)
3 users (show)

See Also:
cs: maintainer-feedback+


Attachments
port source (9.27 KB, application/x-shar)
2016-04-20 21:00 UTC, Carsten Larsen
no flags Details
Revised port (9.74 KB, application/x-shar)
2016-05-09 17:07 UTC, Guido Falsi
no flags Details
Revised port with missing dependency (14.23 KB, application/x-shar)
2016-05-10 19:18 UTC, Carsten Larsen
no flags Details
new port file (14.88 KB, application/x-shar)
2016-05-13 14:41 UTC, Guido Falsi
no flags Details
New sample configuration file (8.31 KB, text/x-matlab)
2016-05-13 20:41 UTC, Carsten Larsen
no flags Details
port source (port version 0.6.0) (8.21 KB, application/x-shar)
2016-05-22 12:50 UTC, Carsten Larsen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Carsten Larsen 2016-04-20 21:00:46 UTC
Created attachment 169502 [details]
port source

NTP Analyzer is a tool dedicated to analyze the operation of time
servers.

NTP Analyzer works by collecting data from the ntp daemon. Graphs
and web pages can then be generated to visualize the activities of
hosts and peers.

WWW: http://github.com/llsth/ntp
Comment 1 Kurt Jaeger freebsd_committer freebsd_triage 2016-04-25 09:05:26 UTC
The real URL is https://github.com/llsth/ntpa
Comment 2 Guido Falsi freebsd_committer freebsd_triage 2016-05-09 17:07:24 UTC
Created attachment 170149 [details]
Revised port

Hi,

Sorry for the delay.

I tested the port and made a few changes:

- used rclint (from devel/rclint) to check the rc script. I made a few changes to silence some errors. Since in your rc script you want to allow creating more instances using symlinks I copied some code from the openvpn rc script which does the same and has a tested way of dong that.

- I fixed the sample config file installation to leverage the @sample keyword and also to install the configuration file with correct permissions.

- Since the sample configuration puts log files in /var/log/ntpa I added an @dir with correct permissions to the port, so it is automatically created.

Can you please review these changes and report back if there are any problems or not?

Thanks.
Comment 3 Carsten Larsen 2016-05-10 19:18:59 UTC
Created attachment 170188 [details]
Revised port with missing dependency

Hi

I tested the port on a clean machine and made a few modifications:
Added missing dependency to webfonts, added a configuration file and corrected a few textual mistakes in the ntpa.rc file.

The configuration now hold a unified statics and web-front setup. Running multiple instances should be optional.

Portlint still complains about LIB_DEPENDS has to appear earlier but if I switch with RUN_DEPENDS then I get another warning.

The LICENSE is a bit complicated since the MySQL connector is GPL, Twitter Bootstrap is Apache and jQuery is MIT. In the next version is should be simpler as Twitter Bootstrap changed to MIT in their latest version.

So I didn’t add a LICENSE in this version of ntpa.

Please let me know is something is still missing.
Comment 4 Carsten Larsen 2016-05-10 20:13:30 UTC
(In reply to Guido Falsi from comment #2)
The revised port does not have any problems except the missing dependency to webfonts. I had to do a chmod 775 on /usr/local/www/ntpa in order to allow file generation but I don’t know if this is a problem which should be handled automatically.

And of cause: Thank you for testing and taking your time to make the port mature.
Comment 5 Guido Falsi freebsd_committer freebsd_triage 2016-05-11 11:00:46 UTC
I'm looking at these changes, they look good, only one detail, since 02-04-2016 the ${PORTSDIR}/ part in dependencies is not required anymore, and in fact deprecated, so your run depends becomes:

RUN_DEPENDS=    ${LOCALBASE}/share/fonts/webfonts/arial.ttf:x11-fonts/webfonts

(don't worry with sending a patch just for this I'll fix it here).

Regarding the LICENSE, it's not mandatory, and if it's complicated it's ok to leave it out.

The portlint error for ordering happens because portlint expects no empty lines between the *_DEPENDS.

I'm investigating the permission issue. In general if you need to instal files with special permissions you should use the pkg-plist directives documented in pkg-create(8).

Look at @owner and @group.

I'll send you another revision so you can see it in practice once I am sure which permissions are needed.
Comment 6 Carsten Larsen 2016-05-11 21:13:57 UTC
(In reply to Guido Falsi from comment #5)

Hi

This patch to pkg-plist should fix all the required permissions and also delete any remaining pid files in /var/run/ntpa


--- pkg-plist.orig	2016-05-12 00:09:33.620268000 +0200
+++ pkg-plist	2016-05-12 01:08:47.575828000 +0200
@@ -20,6 +20,9 @@
 libexec/ntpa/Ntp.Monitor.Server.dll
 libexec/ntpa/Ntp.Process.dll
 @sample(,ntpa,640) %%ETCDIR%%/ntpa.conf.sample
+@mode 755
+@owner www
+@group www
 %%WWWDIR%%/index.html
 %%WWWDIR%%/LICENSE-bootstrap
 %%WWWDIR%%/LICENSE-jQuery
@@ -38,8 +41,17 @@
 %%WWWDIR%%/js/bootstrap.min.js
 %%WWWDIR%%/js/jquery.js
 %%WWWDIR%%/js/jquery.min.js
+@mode 444
+@owner
+@group
 %%EXAMPLESDIR%%/ntpa.graph.conf
 %%EXAMPLESDIR%%/ntpa.stat.conf
 %%EXAMPLESDIR%%/ntpa.web.conf
 %%EXAMPLESDIR%%/ntpa.web.small.conf
+@mode
 @dir(ntpa,ntpa,750) /var/log/ntpa
+@dir(www,ntpa,775) %%WWWDIR%%
+@dir(www,www,755) %%WWWDIR%%/css
+@dir(www,www,755) %%WWWDIR%%/fonts
+@dir(www,www,755) %%WWWDIR%%/js
+@postunexec rm -Rf /var/run/ntpa
Comment 7 Carsten Larsen 2016-05-11 21:33:09 UTC
If appropriate a small pkg-message file could also be added.

---
You should create a user in your MySQL database with:

CREATE DATABASE ntpa
CREATE USER 'ntpau'@'localhost' IDENTIFIED BY 'choosepasseword';
GRANT ALL PRIVILEGES ON ntpa.* TO 'ntpau'@'localhost';
FLUSH PRIVILEGES;
---
Comment 8 Guido Falsi freebsd_committer freebsd_triage 2016-05-13 11:47:03 UTC
(In reply to cs from comment #6)

> --- pkg-plist.orig	2016-05-12 00:09:33.620268000 +0200
> +++ pkg-plist	2016-05-12 01:08:47.575828000 +0200
> @@ -20,6 +20,9 @@
>  libexec/ntpa/Ntp.Monitor.Server.dll
>  libexec/ntpa/Ntp.Process.dll
>  @sample(,ntpa,640) %%ETCDIR%%/ntpa.conf.sample
> +@mode 755
> +@owner www
> +@group www
>  %%WWWDIR%%/index.html
>  %%WWWDIR%%/LICENSE-bootstrap
>  %%WWWDIR%%/LICENSE-jQuery
> @@ -38,8 +41,17 @@
>  %%WWWDIR%%/js/bootstrap.min.js
>  %%WWWDIR%%/js/jquery.js
>  %%WWWDIR%%/js/jquery.min.js
> +@mode 444

This is not needed, since the files will get these permissions anyway.

@mode is needed only when permissions need to be fixed by pkg after extracting the files from the pkg, due to the packaged file not reflecing correct permissions.

This could happen due to many causes, for example setuid/setgid always need to be specified via @mode, and also special owners need this, since staging can happena as a normal user who can't assign files to custom users (ntpa for example)

> +@owner
> +@group
>  %%EXAMPLESDIR%%/ntpa.graph.conf
>  %%EXAMPLESDIR%%/ntpa.stat.conf
>  %%EXAMPLESDIR%%/ntpa.web.conf
>  %%EXAMPLESDIR%%/ntpa.web.small.conf
> +@mode
>  @dir(ntpa,ntpa,750) /var/log/ntpa
> +@dir(www,ntpa,775) %%WWWDIR%%
> +@dir(www,www,755) %%WWWDIR%%/css
> +@dir(www,www,755) %%WWWDIR%%/fonts
> +@dir(www,www,755) %%WWWDIR%%/js

I have something similar here.


> +@postunexec rm -Rf /var/run/ntpa

This should not be necessary.

plist exec directives are to be used when strictly necessary. A stray pid file should not be there if the daemon was correctly stopped, and even if it happens it's correct, since then the daemon is still running.

the @dir(ntpa,ntpa,750) /var/log/ntpa above will warrant that the directory will be removed if empty.
Comment 9 Guido Falsi freebsd_committer freebsd_triage 2016-05-13 11:48:00 UTC
(In reply to cs from comment #7)
> If appropriate a small pkg-message file could also be added.
> 
> ---
> You should create a user in your MySQL database with:
> 
> CREATE DATABASE ntpa
> CREATE USER 'ntpau'@'localhost' IDENTIFIED BY 'choosepasseword';
> GRANT ALL PRIVILEGES ON ntpa.* TO 'ntpau'@'localhost';
> FLUSH PRIVILEGES;
> ---

Yes, this could be useful. I'll add it to my testing.
Comment 10 Guido Falsi freebsd_committer freebsd_triage 2016-05-13 11:56:13 UTC
(In reply to Guido Falsi from comment #8)

> the @dir(ntpa,ntpa,750) /var/log/ntpa above will warrant that the directory
> will be removed if empty.

Sorry, this comment is wrong. I was confused with /ver/run.

anyway adding:

@dir(ntpa,ntpa,755) /var/run/ntpa

will create an appropriate directory which will be removed on deinstallation if empty.
Comment 11 Guido Falsi freebsd_committer freebsd_triage 2016-05-13 14:41:55 UTC
Created attachment 170249 [details]
new port file

Here is a new shar of the port, including your suggestions and a few other fixes, please review it, also run test it please, and report back if it looks ok to you or if you have any isues.

Thanks!
Comment 12 Carsten Larsen 2016-05-13 20:40:14 UTC
(In reply to Guido Falsi from comment #11)
I did an initial test which was successful. The supplied configuration file does not work thou. I will attach a new one. This one I will use for further testing.

During the review I found only one mistake:
CREATE DATABASE ntpa
should be:
CREATE DATABASE ntpa;

I will then follow the test plan as described below and report back when all tests passes without errors.

Fresh install of 10.3-RELEASE #0 r297264 and with a fresh portstree from Fri May 13 02:01:37 CEST 2016.
- Installs from portstree
- Daemon start, stop, reload
- OS reboot and daemon starts up
- Data is collected
- Graphs and pages are generated
- Log files looks reasonable
- Multiple instances
Comment 13 Carsten Larsen 2016-05-13 20:41:20 UTC
Created attachment 170254 [details]
New sample configuration file
Comment 14 Guido Falsi freebsd_committer freebsd_triage 2016-05-13 21:10:14 UTC
(In reply to cs from comment #13)
> Created attachment 170254 [details]
> New sample configuration file

Is the full length lorem ipsum strictly needed in the content element?

While it is fun it just makes the file longer for no reason, can it be replace by a short text like "replace this text with a description of the server machine"?
Comment 15 Carsten Larsen 2016-05-13 21:24:04 UTC
(In reply to Guido Falsi from comment #14)
No, of cause not. I though about the same. Please make it shorter and also please feel free to change anything else you find unreasonable.

I had to add the peer pages and peer graphs sections since 0.5.2 does not work without these 2 sections. Also the "default" host page seems faulty. The page does not show any peers. Therefore I changed to the bootstrap theme.

I will take care of this in the upcoming 0.6.0 version but for now I guess I have to accept these shortcomings.
Comment 16 Carsten Larsen 2016-05-14 06:48:50 UTC
The port works now. I found one last thing regarding rcorder. If ntpa is running on the same machine as the database then mysql is required to start before ntpa.

# REQUIRE: networking mysql

Otherwise all is clear.

Thank you.
Comment 17 Carsten Larsen 2016-05-15 22:18:36 UTC
I found more bugs in the permission module. I need to do a few bugfixes and make a new release before this port is ready.

I am really sorry but please but
Comment 18 Carsten Larsen 2016-05-15 22:20:24 UTC
I am really sorry but please postpone it (however that is done in bugzilla).
Comment 19 Guido Falsi freebsd_committer freebsd_triage 2016-05-16 16:58:50 UTC
(In reply to cs from comment #18)
> I am really sorry but please postpone it (however that is done in bugzilla).

Ok, no problem, I'll wait for some further notice from you before proceeding with this port.
Comment 20 Carsten Larsen 2016-05-22 12:50:58 UTC
Created attachment 170545 [details]
port source (port version 0.6.0)

Hi

I updated ntpa to 0.6.0. Changes include support for PostgreSQL and lots of bugfixes. The port now also has port options and the mastersite has changed.

I hope all is good.
Comment 21 Guido Falsi freebsd_committer freebsd_triage 2016-05-23 10:49:58 UTC
I am looking at the new port, why the ALWAYS_KEEP_DISTFILES=  yes directive? why is that needed for the MYSQL case?
Comment 22 Carsten Larsen 2016-05-23 15:09:58 UTC
(In reply to Guido Falsi from comment #21)
The ALWAYS_KEEP_DISTFILES is mentioned in FreeBSD Porter's Handbook, section 5.4.10. After reading more carefully I can see it is a mistake. The section only mentions *binary* distfiles and ntpa contains only sources. ALWAYS_KEEP_DISTFILES should be removed.
Comment 23 Carsten Larsen 2016-05-23 15:17:08 UTC
Also, the project homepage has moved to:
https://bitbucket.org/anguist/ntpa

I don’t know if its worth to mention in the pkg-descr file.
Comment 24 Guido Falsi freebsd_committer freebsd_triage 2016-05-27 09:41:36 UTC
I'm testing with a simple MySQL server and the default configuration file.

While the daemon runs and I see it saving data in the DB, I'm not seeing any web pages being generated in /usr/local/www/ntpa.

Is this expected? if not what could be the reason? Anything I need to modify to get graphs there?

I'm doing this as a final test before committing.

Thanks.
Comment 25 Carsten Larsen 2016-05-27 17:30:10 UTC
(In reply to Guido Falsi from comment #24)
You should edit the configuration in /usr/local/etc/ntpa/ntpa.conf

In the default config the FilePath is set to /usr/local/www instead of /usr/local/www/ntpa
Comment 26 Guido Falsi freebsd_committer freebsd_triage 2016-05-27 17:37:56 UTC
I see.

If you have no objection I will add a directive to patch it in 'post-patch' so that the installed sample has the correct path in it.

I'll try again later and report back.
Comment 27 Carsten Larsen 2016-05-27 19:07:52 UTC
Mr. Guido, I could change it directly in the source (together with a few other minor bugs) and bump the port version to 0.6.1. But that would require you make a new hash sum. If it is acceptable you then I can do it.
Comment 28 Guido Falsi freebsd_committer freebsd_triage 2016-05-27 19:15:49 UTC
(In reply to Carsten Larsen from comment #27)
> Mr. Guido, I could change it directly in the source (together with a few
> other minor bugs) and bump the port version to 0.6.1. But that would require
> you make a new hash sum. If it is acceptable you then I can do it.

Generating a new hash for a new port is absolutely no problem, so if you prefer this solution I'm ok with it.
Comment 29 Carsten Larsen 2016-05-27 19:49:59 UTC
Great! Hang on then. Changes are already committed to the master branch. I will test it and make a new release package when it's done.
Comment 30 Carsten Larsen 2016-05-28 07:55:31 UTC
Version 0.6.1 is now ready. Thank you for your good work !
Comment 31 commit-hook freebsd_committer freebsd_triage 2016-05-28 14:47:38 UTC
A commit references this bug:

Author: madpilot
Date: Sat May 28 14:46:44 UTC 2016
New revision: 416000
URL: https://svnweb.freebsd.org/changeset/ports/416000

Log:
  New port: net/ntpa

  NTP Analyzer is a tool dedicated to analyze the operation of time
  servers.

  NTP Analyzer works by collecting data from the ntp daemon. Graphs
  and web pages can then be generated to visualize the activities of
  hosts and peers.

  WWW: https://bitbucket.org/anguist/ntpa

  PR:		208940
  Submitted by:	Carsten Larsen <cs at innolan.dk>

Changes:
  head/GIDs
  head/UIDs
  head/net/Makefile
  head/net/ntpa/
  head/net/ntpa/Makefile
  head/net/ntpa/distinfo
  head/net/ntpa/files/
  head/net/ntpa/files/ntpa.in
  head/net/ntpa/pkg-descr
  head/net/ntpa/pkg-message
  head/net/ntpa/pkg-plist
Comment 32 Guido Falsi freebsd_committer freebsd_triage 2016-05-28 14:49:25 UTC
Port added.

I forgot to mention, you have a right to have your name added to the FreeBSD additional contributors list [1].


Are you ok to getting your name in there as Carsten Larsen <cs@innolan.dk>

I'm restraining from closing the PR until I'm also done with this part.

[1] https://www.freebsd.org/doc/en/articles/contributors/contrib-additional.html
Comment 33 Carsten Larsen 2016-05-28 16:04:19 UTC
Hi Guido Falsi

> I forgot to mention, you have a right to have your name added to the FreeBSD additional contributors list [1].
> Are you ok to getting your name in there as Carsten Larsen <cs@innolan.dk>

Name and email as you proposed is perfect. And yes, please add me to the list.

Thank you very much !
Comment 34 commit-hook freebsd_committer freebsd_triage 2016-05-28 16:55:48 UTC
A commit references this bug:

Author: madpilot
Date: Sat May 28 16:55:39 UTC 2016
New revision: 48870
URL: https://svnweb.freebsd.org/changeset/doc/48870

Log:
  For net/ntpa

  PR:		208940

Changes:
  head/en_US.ISO8859-1/articles/contributors/contrib.additional.xml
Comment 35 Guido Falsi freebsd_committer freebsd_triage 2016-05-28 16:56:06 UTC
(In reply to Carsten Larsen from comment #33)
> Hi Guido Falsi
> 
> > I forgot to mention, you have a right to have your name added to the FreeBSD additional contributors list [1].
> > Are you ok to getting your name in there as Carsten Larsen <cs@innolan.dk>
> 
> Name and email as you proposed is perfect. And yes, please add me to the
> list.
> 
> Thank you very much !

Done. Thanks!