Patchwork [meta-networking,2/2] ntp: make servers configurable and default to none configured

login
register
mail settings
Submitter Paul Eggleton
Date Nov. 21, 2012, 5:29 p.m.
Message ID <1eef087d214b6c6ff27b60dff5511acfc2a3925e.1353518890.git.paul.eggleton@linux.intel.com>
Download mbox | patch
Permalink /patch/39423/
State Superseded
Headers show

Comments

Paul Eggleton - Nov. 21, 2012, 5:29 p.m.
People can't blindly use pool.ntp.org, especially if they are building
for a product or something that could be used in a product, so at least
try to get people to do the right thing and not use pool.ntp.org by
default.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta-networking/recipes-support/ntp/ntp.inc |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
Martin Jansa - Nov. 21, 2012, 5:37 p.m.
On Wed, Nov 21, 2012 at 05:29:30PM +0000, Paul Eggleton wrote:
> People can't blindly use pool.ntp.org, especially if they are building
> for a product or something that could be used in a product, so at least
> try to get people to do the right thing and not use pool.ntp.org by
> default.
> 
> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> ---
>  meta-networking/recipes-support/ntp/ntp.inc |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/meta-networking/recipes-support/ntp/ntp.inc b/meta-networking/recipes-support/ntp/ntp.inc
> index d20bc2e..b54608a 100644
> --- a/meta-networking/recipes-support/ntp/ntp.inc
> +++ b/meta-networking/recipes-support/ntp/ntp.inc
> @@ -33,12 +33,22 @@ PACKAGECONFIG[openssl] = "--with-openssl-libdir=${STAGING_LIBDIR} \
>                            --without-openssl --without-crypto, \
>                            openssl"
>  
> +# This should be set in the distro configuration
> +NTP_SERVERS ??= ""
> +
> +python __anonymous() {
> +    if not d.getVar("NTP_SERVERS", True):
> +        raise bb.parse.SkipPackage("Please set NTP_SERVERS in order to build ntp - see http://www.openembedded.org/wiki/NTP for details")
> +}

Cannot we move this logic to target?

I mean ntpdate can be usefull for end-user even without this cron job.

We can add something like
/etc/default/ntp
NTP_SERVERS=""

And change cron job as well as that systemd .service file we talked
about before to show
"Please set correct NPT_SERVERS in /etc/default/ntp"
instead of failing (in systemd .service file case) or not being able to
build ntpdate package unless you have distro with own ntp pool.

On other hand distro with own ntp pool can provide own value in
/etc/default/ntp with .bbappend, so their users won't ever see this on
target.

Cheers,

> +
>  do_install_append() {
>  	install -d ${D}/${sysconfdir}/init.d
>  	install -m 644 ${WORKDIR}/ntp.conf ${D}/${sysconfdir}
>  	install -m 755 ${WORKDIR}/ntpd ${D}/${sysconfdir}/init.d
>  	install -d ${D}/${sysconfdir}/network/if-up.d
>  	install -m 755 ${WORKDIR}/ntpdate ${D}/${sysconfdir}/network/if-up.d
> +
> +	sed -i "s/pool.ntp.org/${NTP_SERVERS}/g" ${D}/${sysconfdir}/ntp.conf ${D}/${sysconfdir}/network/if-up.d/ntpdate
>  }
>  
>  PACKAGES += "ntpdate ${PN}-bin ${PN}-tickadj ${PN}-utils"
> @@ -71,7 +81,7 @@ else
>          if ! grep -q -s ntpdate /var/spool/cron/root; then
>                  echo "adding crontab"
>                  test -d /var/spool/cron || mkdir -p /var/spool/cron
> -                echo "30 * * * *    /usr/bin/ntpdate -b -s -u pool.ntp.org" >> /var/spool/cron/root
> +                echo "30 * * * *    /usr/bin/ntpdate -b -s -u ${NTP_SERVERS}" >> /var/spool/cron/root
>          fi
>  fi
>  }
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel
Paul Eggleton - Nov. 21, 2012, 5:51 p.m.
On Wednesday 21 November 2012 18:37:54 Martin Jansa wrote:
> On Wed, Nov 21, 2012 at 05:29:30PM +0000, Paul Eggleton wrote:
> > People can't blindly use pool.ntp.org, especially if they are building
> > for a product or something that could be used in a product, so at least
> > try to get people to do the right thing and not use pool.ntp.org by
> > default.
> > 
> > Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
> > ---
> > 
> >  meta-networking/recipes-support/ntp/ntp.inc |   12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/meta-networking/recipes-support/ntp/ntp.inc
> > b/meta-networking/recipes-support/ntp/ntp.inc index d20bc2e..b54608a
> > 100644
> > --- a/meta-networking/recipes-support/ntp/ntp.inc
> > +++ b/meta-networking/recipes-support/ntp/ntp.inc
> > @@ -33,12 +33,22 @@ PACKAGECONFIG[openssl] =
> > "--with-openssl-libdir=${STAGING_LIBDIR} \> 
> >                            --without-openssl --without-crypto, \
> >                            openssl"
> > 
> > +# This should be set in the distro configuration
> > +NTP_SERVERS ??= ""
> > +
> > +python __anonymous() {
> > +    if not d.getVar("NTP_SERVERS", True):
> > +        raise bb.parse.SkipPackage("Please set NTP_SERVERS in order to
> > build ntp - see http://www.openembedded.org/wiki/NTP for details") +}
> 
> Cannot we move this logic to target?
> 
> I mean ntpdate can be usefull for end-user even without this cron job.
> 
> We can add something like
> /etc/default/ntp
> NTP_SERVERS=""
> 
> And change cron job as well as that systemd .service file we talked
> about before to show
> "Please set correct NPT_SERVERS in /etc/default/ntp"
> instead of failing (in systemd .service file case) or not being able to
> build ntpdate package unless you have distro with own ntp pool.
> 
> On other hand distro with own ntp pool can provide own value in
> /etc/default/ntp with .bbappend, so their users won't ever see this on
> target.

Sure, this is an alternative approach. It will work for everything except 
ntp.conf, although for that we can just clear out that value in the file and 
get the distro to set their own custom version if they're going to use ntpd.

Cheers,
Paul
ml@communistcode.co.uk - Nov. 22, 2012, 9:34 a.m.
On 21/11/12 17:51, Paul Eggleton wrote:
> On Wednesday 21 November 2012 18:37:54 Martin Jansa wrote:
>> On Wed, Nov 21, 2012 at 05:29:30PM +0000, Paul Eggleton wrote:
>>> People can't blindly use pool.ntp.org, especially if they are building
>>> for a product or something that could be used in a product, so at least
>>> try to get people to do the right thing and not use pool.ntp.org by
>>> default.
>>>
>>> Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
>>> ---
>>>
>>>   meta-networking/recipes-support/ntp/ntp.inc |   12 +++++++++++-
>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/meta-networking/recipes-support/ntp/ntp.inc
>>> b/meta-networking/recipes-support/ntp/ntp.inc index d20bc2e..b54608a
>>> 100644
>>> --- a/meta-networking/recipes-support/ntp/ntp.inc
>>> +++ b/meta-networking/recipes-support/ntp/ntp.inc
>>> @@ -33,12 +33,22 @@ PACKAGECONFIG[openssl] =
>>> "--with-openssl-libdir=${STAGING_LIBDIR} \>
>>>                             --without-openssl --without-crypto, \
>>>                             openssl"
>>>
>>> +# This should be set in the distro configuration
>>> +NTP_SERVERS ??= ""
>>> +
>>> +python __anonymous() {
>>> +    if not d.getVar("NTP_SERVERS", True):
>>> +        raise bb.parse.SkipPackage("Please set NTP_SERVERS in order to
>>> build ntp - see http://www.openembedded.org/wiki/NTP for details") +}
>> Cannot we move this logic to target?
>>
>> I mean ntpdate can be usefull for end-user even without this cron job.
>>
>> We can add something like
>> /etc/default/ntp
>> NTP_SERVERS=""
>>
>> And change cron job as well as that systemd .service file we talked
>> about before to show
>> "Please set correct NPT_SERVERS in /etc/default/ntp"
>> instead of failing (in systemd .service file case) or not being able to
>> build ntpdate package unless you have distro with own ntp pool.
>>
>> On other hand distro with own ntp pool can provide own value in
>> /etc/default/ntp with .bbappend, so their users won't ever see this on
>> target.
> Sure, this is an alternative approach. It will work for everything except
> ntp.conf, although for that we can just clear out that value in the file and
> get the distro to set their own custom version if they're going to use ntpd.
>
> Cheers,
> Paul
>

This would be beneficial for me too as we intend to ship ntpd but it 
will be mainly for use with internal servers so I will not want or need 
a default server set.

Cheers,

Patch

diff --git a/meta-networking/recipes-support/ntp/ntp.inc b/meta-networking/recipes-support/ntp/ntp.inc
index d20bc2e..b54608a 100644
--- a/meta-networking/recipes-support/ntp/ntp.inc
+++ b/meta-networking/recipes-support/ntp/ntp.inc
@@ -33,12 +33,22 @@  PACKAGECONFIG[openssl] = "--with-openssl-libdir=${STAGING_LIBDIR} \
                           --without-openssl --without-crypto, \
                           openssl"
 
+# This should be set in the distro configuration
+NTP_SERVERS ??= ""
+
+python __anonymous() {
+    if not d.getVar("NTP_SERVERS", True):
+        raise bb.parse.SkipPackage("Please set NTP_SERVERS in order to build ntp - see http://www.openembedded.org/wiki/NTP for details")
+}
+
 do_install_append() {
 	install -d ${D}/${sysconfdir}/init.d
 	install -m 644 ${WORKDIR}/ntp.conf ${D}/${sysconfdir}
 	install -m 755 ${WORKDIR}/ntpd ${D}/${sysconfdir}/init.d
 	install -d ${D}/${sysconfdir}/network/if-up.d
 	install -m 755 ${WORKDIR}/ntpdate ${D}/${sysconfdir}/network/if-up.d
+
+	sed -i "s/pool.ntp.org/${NTP_SERVERS}/g" ${D}/${sysconfdir}/ntp.conf ${D}/${sysconfdir}/network/if-up.d/ntpdate
 }
 
 PACKAGES += "ntpdate ${PN}-bin ${PN}-tickadj ${PN}-utils"
@@ -71,7 +81,7 @@  else
         if ! grep -q -s ntpdate /var/spool/cron/root; then
                 echo "adding crontab"
                 test -d /var/spool/cron || mkdir -p /var/spool/cron
-                echo "30 * * * *    /usr/bin/ntpdate -b -s -u pool.ntp.org" >> /var/spool/cron/root
+                echo "30 * * * *    /usr/bin/ntpdate -b -s -u ${NTP_SERVERS}" >> /var/spool/cron/root
         fi
 fi
 }