diff mbox

[ovs-dev] debian, xenserver: Update logrotate config to match RHEL.

Message ID 20170413174755.1131-1-blp@ovn.org
State Accepted
Headers show

Commit Message

Ben Pfaff April 13, 2017, 5:47 p.m. UTC
Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
does not exist") updated the RHEL logrotate configuration.  This commit
makes similar changes for Debian, by synchronizing with the RHEL version.

In particular:

    - Indent to match logrotate.conf(5) examples.

    - Use "sharedscripts" flag, because the postrotate script only needs to
      run once regardless of the number of rotations.

    - Drop "delaycompress", because the postrotate script does make daemons
      reopen their log files.

    - Ignore errors calling vlog/reopen.

Also make similar changes to the xenserver logrotate script.  I really
don't know if anyone uses the xenserver packaging anymore though.

CC: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 debian/openvswitch-switch.logrotate   | 14 +++++++-------
 xenserver/etc_logrotate.d_openvswitch | 22 ++++++++++++----------
 2 files changed, 19 insertions(+), 17 deletions(-)

Comments

Gregory Rose April 14, 2017, 4:05 p.m. UTC | #1
On Thu, 2017-04-13 at 10:47 -0700, Ben Pfaff wrote:
> Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> does not exist") updated the RHEL logrotate configuration.  This commit
> makes similar changes for Debian, by synchronizing with the RHEL version.
> 
> In particular:
> 
>     - Indent to match logrotate.conf(5) examples.
> 
>     - Use "sharedscripts" flag, because the postrotate script only needs to
>       run once regardless of the number of rotations.
> 
>     - Drop "delaycompress", because the postrotate script does make daemons
>       reopen their log files.
> 
>     - Ignore errors calling vlog/reopen.
> 
> Also make similar changes to the xenserver logrotate script.  I really
> don't know if anyone uses the xenserver packaging anymore though.

Hi Ben,

I'm actually trying to build the latest upstream OVS from github on a
Xen Server 7.1 system.  I have followed the instructions as well as I
could and everything seems to build pretty well in the Xen Server 7.1
DDK VM until finally it fails with the following error output:

Checking for unpackaged
file(s): /usr/lib/rpm/check-files /prj/gvrose/rpmbuild/BUILDROOT/openvswitch-2.7.90-1.x86_64
error: Installed (but unpackaged) file(s) found:
   /usr/bin/ovs-tcpdump
   /usr/share/man/man7/ovs-fields.7.gz
   /usr/share/man/man8/ovs-tcpdump.8.gz
   /usr/share/openvswitch/scripts/ovndb-servers.ocf


RPM build errors:
    Installed (but unpackaged) file(s) found:
   /usr/bin/ovs-tcpdump
   /usr/share/man/man7/ovs-fields.7.gz
   /usr/share/man/man8/ovs-tcpdump.8.gz
   /usr/share/openvswitch/scripts/ovndb-servers.ocf

I suppose there is some file list that needs updating?

Any pointers will be much appreciated and help me get rolling.

Thanks,

- Greg

> 
> CC: Timothy Redaelli <tredaelli@redhat.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>  debian/openvswitch-switch.logrotate   | 14 +++++++-------
>  xenserver/etc_logrotate.d_openvswitch | 22 ++++++++++++----------
>  2 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/debian/openvswitch-switch.logrotate b/debian/openvswitch-switch.logrotate
> index a7a71bdd90ad..7752af90cfed 100644
> --- a/debian/openvswitch-switch.logrotate
> +++ b/debian/openvswitch-switch.logrotate
> @@ -1,16 +1,16 @@
>  /var/log/openvswitch/*.log {
>      daily
>      compress
> +    sharedscripts
>      create 640 root adm
> -    delaycompress
>      missingok
>      rotate 30
>      postrotate
> -    # Tell Open vSwitch daemons to reopen their log files
> -    if [ -d /var/run/openvswitch ]; then
> -        for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> -            ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
> -        done
> -    fi
> +	# Tell Open vSwitch daemons to reopen their log files
> +	if [ -d /var/run/openvswitch ]; then
> +	    for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> +		ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
> +	    done
> +	fi
>      endscript
>  }
> diff --git a/xenserver/etc_logrotate.d_openvswitch b/xenserver/etc_logrotate.d_openvswitch
> index 73751d4578b0..cd7b3a9d569d 100644
> --- a/xenserver/etc_logrotate.d_openvswitch
> +++ b/xenserver/etc_logrotate.d_openvswitch
> @@ -1,4 +1,4 @@
> -# Copyright (C) 2009, 2010, 2011, 2012 Nicira, Inc.
> +# Copyright (C) 2009, 2010, 2011, 2012, 2017 Nicira, Inc.
>  #
>  # Copying and distribution of this file, with or without modification,
>  # are permitted in any medium without royalty provided the copyright
> @@ -6,14 +6,16 @@
>  # without warranty of any kind.
>  
>  /var/log/openvswitch/*.log {
> -	daily
> -	compress
> -	sharedscripts
> -	missingok
> -	postrotate
> +    daily
> +    compress
> +    sharedscripts
> +    missingok
> +    postrotate
>  	# Tell Open vSwitch daemons to reopen their log files
> -        for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> -            ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
> -        done
> -	endscript
> +        if [ -d /var/run/openvswitch ]; then
> +	    for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> +		ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
> +	    done
> +	fi
> +    endscript
>  }
Ben Pfaff April 14, 2017, 5:03 p.m. UTC | #2
On Fri, Apr 14, 2017 at 09:05:41AM -0700, Greg Rose wrote:
> On Thu, 2017-04-13 at 10:47 -0700, Ben Pfaff wrote:
> > Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> > does not exist") updated the RHEL logrotate configuration.  This commit
> > makes similar changes for Debian, by synchronizing with the RHEL version.
> > 
> > In particular:
> > 
> >     - Indent to match logrotate.conf(5) examples.
> > 
> >     - Use "sharedscripts" flag, because the postrotate script only needs to
> >       run once regardless of the number of rotations.
> > 
> >     - Drop "delaycompress", because the postrotate script does make daemons
> >       reopen their log files.
> > 
> >     - Ignore errors calling vlog/reopen.
> > 
> > Also make similar changes to the xenserver logrotate script.  I really
> > don't know if anyone uses the xenserver packaging anymore though.
> 
> Hi Ben,
> 
> I'm actually trying to build the latest upstream OVS from github on a
> Xen Server 7.1 system.  I have followed the instructions as well as I
> could and everything seems to build pretty well in the Xen Server 7.1
> DDK VM until finally it fails with the following error output:
> 
> Checking for unpackaged
> file(s): /usr/lib/rpm/check-files /prj/gvrose/rpmbuild/BUILDROOT/openvswitch-2.7.90-1.x86_64
> error: Installed (but unpackaged) file(s) found:
>    /usr/bin/ovs-tcpdump
>    /usr/share/man/man7/ovs-fields.7.gz
>    /usr/share/man/man8/ovs-tcpdump.8.gz
>    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> 
> 
> RPM build errors:
>     Installed (but unpackaged) file(s) found:
>    /usr/bin/ovs-tcpdump
>    /usr/share/man/man7/ovs-fields.7.gz
>    /usr/share/man/man8/ovs-tcpdump.8.gz
>    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> 
> I suppose there is some file list that needs updating?

Yes, please look in xenserver/openvswitch-xen.spec.in.

However, this may be a good sign that no one cares about XenServer,
since Open vSwitch 2.6.0, released in September last year, also installs
at least /usr/bin/ovs-tcpdump, and no one has complained.  Perhaps we
should just delete the packaging, instead, and save ourselves some
trouble.
Gregory Rose April 14, 2017, 5:19 p.m. UTC | #3
On Fri, 2017-04-14 at 09:05 -0700, Greg Rose wrote:
> On Thu, 2017-04-13 at 10:47 -0700, Ben Pfaff wrote:
> > Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> > does not exist") updated the RHEL logrotate configuration.  This commit
> > makes similar changes for Debian, by synchronizing with the RHEL version.
> > 
> > In particular:
> > 
> >     - Indent to match logrotate.conf(5) examples.
> > 
> >     - Use "sharedscripts" flag, because the postrotate script only needs to
> >       run once regardless of the number of rotations.
> > 
> >     - Drop "delaycompress", because the postrotate script does make daemons
> >       reopen their log files.
> > 
> >     - Ignore errors calling vlog/reopen.
> > 
> > Also make similar changes to the xenserver logrotate script.  I really
> > don't know if anyone uses the xenserver packaging anymore though.
> 
> Hi Ben,
> 
> I'm actually trying to build the latest upstream OVS from github on a
> Xen Server 7.1 system.  I have followed the instructions as well as I
> could and everything seems to build pretty well in the Xen Server 7.1
> DDK VM until finally it fails with the following error output:
> 
> Checking for unpackaged
> file(s): /usr/lib/rpm/check-files /prj/gvrose/rpmbuild/BUILDROOT/openvswitch-2.7.90-1.x86_64
> error: Installed (but unpackaged) file(s) found:
>    /usr/bin/ovs-tcpdump
>    /usr/share/man/man7/ovs-fields.7.gz
>    /usr/share/man/man8/ovs-tcpdump.8.gz
>    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> 
> 
> RPM build errors:
>     Installed (but unpackaged) file(s) found:
>    /usr/bin/ovs-tcpdump
>    /usr/share/man/man7/ovs-fields.7.gz
>    /usr/share/man/man8/ovs-tcpdump.8.gz
>    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> 
> I suppose there is some file list that needs updating?
> 
> Any pointers will be much appreciated and help me get rolling.
> 
> Thanks,
> 
> - Greg

I found the file list in the xenserver/openvswitch-xen.spec and fixed it
up.  I've got the RPMs all built now.

I'll generate a patch and send later today.

Thanks,

- Greg

> 
> > 
> > CC: Timothy Redaelli <tredaelli@redhat.com>
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > ---
> >  debian/openvswitch-switch.logrotate   | 14 +++++++-------
> >  xenserver/etc_logrotate.d_openvswitch | 22 ++++++++++++----------
> >  2 files changed, 19 insertions(+), 17 deletions(-)
> > 
> > diff --git a/debian/openvswitch-switch.logrotate b/debian/openvswitch-switch.logrotate
> > index a7a71bdd90ad..7752af90cfed 100644
> > --- a/debian/openvswitch-switch.logrotate
> > +++ b/debian/openvswitch-switch.logrotate
> > @@ -1,16 +1,16 @@
> >  /var/log/openvswitch/*.log {
> >      daily
> >      compress
> > +    sharedscripts
> >      create 640 root adm
> > -    delaycompress
> >      missingok
> >      rotate 30
> >      postrotate
> > -    # Tell Open vSwitch daemons to reopen their log files
> > -    if [ -d /var/run/openvswitch ]; then
> > -        for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> > -            ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
> > -        done
> > -    fi
> > +	# Tell Open vSwitch daemons to reopen their log files
> > +	if [ -d /var/run/openvswitch ]; then
> > +	    for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> > +		ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
> > +	    done
> > +	fi
> >      endscript
> >  }
> > diff --git a/xenserver/etc_logrotate.d_openvswitch b/xenserver/etc_logrotate.d_openvswitch
> > index 73751d4578b0..cd7b3a9d569d 100644
> > --- a/xenserver/etc_logrotate.d_openvswitch
> > +++ b/xenserver/etc_logrotate.d_openvswitch
> > @@ -1,4 +1,4 @@
> > -# Copyright (C) 2009, 2010, 2011, 2012 Nicira, Inc.
> > +# Copyright (C) 2009, 2010, 2011, 2012, 2017 Nicira, Inc.
> >  #
> >  # Copying and distribution of this file, with or without modification,
> >  # are permitted in any medium without royalty provided the copyright
> > @@ -6,14 +6,16 @@
> >  # without warranty of any kind.
> >  
> >  /var/log/openvswitch/*.log {
> > -	daily
> > -	compress
> > -	sharedscripts
> > -	missingok
> > -	postrotate
> > +    daily
> > +    compress
> > +    sharedscripts
> > +    missingok
> > +    postrotate
> >  	# Tell Open vSwitch daemons to reopen their log files
> > -        for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> > -            ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
> > -        done
> > -	endscript
> > +        if [ -d /var/run/openvswitch ]; then
> > +	    for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> > +		ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
> > +	    done
> > +	fi
> > +    endscript
> >  }
> 
>
Gregory Rose April 14, 2017, 5:41 p.m. UTC | #4
On Fri, 2017-04-14 at 10:03 -0700, Ben Pfaff wrote:
> On Fri, Apr 14, 2017 at 09:05:41AM -0700, Greg Rose wrote:
> > On Thu, 2017-04-13 at 10:47 -0700, Ben Pfaff wrote:
> > > Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> > > does not exist") updated the RHEL logrotate configuration.  This commit
> > > makes similar changes for Debian, by synchronizing with the RHEL version.
> > > 
> > > In particular:
> > > 
> > >     - Indent to match logrotate.conf(5) examples.
> > > 
> > >     - Use "sharedscripts" flag, because the postrotate script only needs to
> > >       run once regardless of the number of rotations.
> > > 
> > >     - Drop "delaycompress", because the postrotate script does make daemons
> > >       reopen their log files.
> > > 
> > >     - Ignore errors calling vlog/reopen.
> > > 
> > > Also make similar changes to the xenserver logrotate script.  I really
> > > don't know if anyone uses the xenserver packaging anymore though.
> > 
> > Hi Ben,
> > 
> > I'm actually trying to build the latest upstream OVS from github on a
> > Xen Server 7.1 system.  I have followed the instructions as well as I
> > could and everything seems to build pretty well in the Xen Server 7.1
> > DDK VM until finally it fails with the following error output:
> > 
> > Checking for unpackaged
> > file(s): /usr/lib/rpm/check-files /prj/gvrose/rpmbuild/BUILDROOT/openvswitch-2.7.90-1.x86_64
> > error: Installed (but unpackaged) file(s) found:
> >    /usr/bin/ovs-tcpdump
> >    /usr/share/man/man7/ovs-fields.7.gz
> >    /usr/share/man/man8/ovs-tcpdump.8.gz
> >    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> > 
> > 
> > RPM build errors:
> >     Installed (but unpackaged) file(s) found:
> >    /usr/bin/ovs-tcpdump
> >    /usr/share/man/man7/ovs-fields.7.gz
> >    /usr/share/man/man8/ovs-tcpdump.8.gz
> >    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> > 
> > I suppose there is some file list that needs updating?
> 
> Yes, please look in xenserver/openvswitch-xen.spec.in.
> 
> However, this may be a good sign that no one cares about XenServer,
> since Open vSwitch 2.6.0, released in September last year, also installs
> at least /usr/bin/ovs-tcpdump, and no one has complained.  Perhaps we
> should just delete the packaging, instead, and save ourselves some
> trouble.

LOL - sure. Sounds good to me.  ;^)

In that case I won't bother sending a patch.

- Greg
Ben Pfaff April 16, 2017, 8:19 p.m. UTC | #5
On Fri, Apr 14, 2017 at 10:41:01AM -0700, Greg Rose wrote:
> On Fri, 2017-04-14 at 10:03 -0700, Ben Pfaff wrote:
> > On Fri, Apr 14, 2017 at 09:05:41AM -0700, Greg Rose wrote:
> > > On Thu, 2017-04-13 at 10:47 -0700, Ben Pfaff wrote:
> > > > Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> > > > does not exist") updated the RHEL logrotate configuration.  This commit
> > > > makes similar changes for Debian, by synchronizing with the RHEL version.
> > > > 
> > > > In particular:
> > > > 
> > > >     - Indent to match logrotate.conf(5) examples.
> > > > 
> > > >     - Use "sharedscripts" flag, because the postrotate script only needs to
> > > >       run once regardless of the number of rotations.
> > > > 
> > > >     - Drop "delaycompress", because the postrotate script does make daemons
> > > >       reopen their log files.
> > > > 
> > > >     - Ignore errors calling vlog/reopen.
> > > > 
> > > > Also make similar changes to the xenserver logrotate script.  I really
> > > > don't know if anyone uses the xenserver packaging anymore though.
> > > 
> > > Hi Ben,
> > > 
> > > I'm actually trying to build the latest upstream OVS from github on a
> > > Xen Server 7.1 system.  I have followed the instructions as well as I
> > > could and everything seems to build pretty well in the Xen Server 7.1
> > > DDK VM until finally it fails with the following error output:
> > > 
> > > Checking for unpackaged
> > > file(s): /usr/lib/rpm/check-files /prj/gvrose/rpmbuild/BUILDROOT/openvswitch-2.7.90-1.x86_64
> > > error: Installed (but unpackaged) file(s) found:
> > >    /usr/bin/ovs-tcpdump
> > >    /usr/share/man/man7/ovs-fields.7.gz
> > >    /usr/share/man/man8/ovs-tcpdump.8.gz
> > >    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> > > 
> > > 
> > > RPM build errors:
> > >     Installed (but unpackaged) file(s) found:
> > >    /usr/bin/ovs-tcpdump
> > >    /usr/share/man/man7/ovs-fields.7.gz
> > >    /usr/share/man/man8/ovs-tcpdump.8.gz
> > >    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> > > 
> > > I suppose there is some file list that needs updating?
> > 
> > Yes, please look in xenserver/openvswitch-xen.spec.in.
> > 
> > However, this may be a good sign that no one cares about XenServer,
> > since Open vSwitch 2.6.0, released in September last year, also installs
> > at least /usr/bin/ovs-tcpdump, and no one has complained.  Perhaps we
> > should just delete the packaging, instead, and save ourselves some
> > trouble.
> 
> LOL - sure. Sounds good to me.  ;^)
> 
> In that case I won't bother sending a patch.

I guess you should send it, because I've heard from some XenServer users
via Twitter.
Gregory Rose April 17, 2017, 4:10 a.m. UTC | #6
On Sun, 2017-04-16 at 13:19 -0700, Ben Pfaff wrote:
> On Fri, Apr 14, 2017 at 10:41:01AM -0700, Greg Rose wrote:
> > On Fri, 2017-04-14 at 10:03 -0700, Ben Pfaff wrote:
> > > On Fri, Apr 14, 2017 at 09:05:41AM -0700, Greg Rose wrote:
> > > > On Thu, 2017-04-13 at 10:47 -0700, Ben Pfaff wrote:
> > > > > Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> > > > > does not exist") updated the RHEL logrotate configuration.  This commit
> > > > > makes similar changes for Debian, by synchronizing with the RHEL version.
> > > > > 
> > > > > In particular:
> > > > > 
> > > > >     - Indent to match logrotate.conf(5) examples.
> > > > > 
> > > > >     - Use "sharedscripts" flag, because the postrotate script only needs to
> > > > >       run once regardless of the number of rotations.
> > > > > 
> > > > >     - Drop "delaycompress", because the postrotate script does make daemons
> > > > >       reopen their log files.
> > > > > 
> > > > >     - Ignore errors calling vlog/reopen.
> > > > > 
> > > > > Also make similar changes to the xenserver logrotate script.  I really
> > > > > don't know if anyone uses the xenserver packaging anymore though.
> > > > 
> > > > Hi Ben,
> > > > 
> > > > I'm actually trying to build the latest upstream OVS from github on a
> > > > Xen Server 7.1 system.  I have followed the instructions as well as I
> > > > could and everything seems to build pretty well in the Xen Server 7.1
> > > > DDK VM until finally it fails with the following error output:
> > > > 
> > > > Checking for unpackaged
> > > > file(s): /usr/lib/rpm/check-files /prj/gvrose/rpmbuild/BUILDROOT/openvswitch-2.7.90-1.x86_64
> > > > error: Installed (but unpackaged) file(s) found:
> > > >    /usr/bin/ovs-tcpdump
> > > >    /usr/share/man/man7/ovs-fields.7.gz
> > > >    /usr/share/man/man8/ovs-tcpdump.8.gz
> > > >    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> > > > 
> > > > 
> > > > RPM build errors:
> > > >     Installed (but unpackaged) file(s) found:
> > > >    /usr/bin/ovs-tcpdump
> > > >    /usr/share/man/man7/ovs-fields.7.gz
> > > >    /usr/share/man/man8/ovs-tcpdump.8.gz
> > > >    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> > > > 
> > > > I suppose there is some file list that needs updating?
> > > 
> > > Yes, please look in xenserver/openvswitch-xen.spec.in.
> > > 
> > > However, this may be a good sign that no one cares about XenServer,
> > > since Open vSwitch 2.6.0, released in September last year, also installs
> > > at least /usr/bin/ovs-tcpdump, and no one has complained.  Perhaps we
> > > should just delete the packaging, instead, and save ourselves some
> > > trouble.
> > 
> > LOL - sure. Sounds good to me.  ;^)
> > 
> > In that case I won't bother sending a patch.
> 
> I guess you should send it, because I've heard from some XenServer users
> via Twitter.

Alright, I'll get that out first thing tomorrow morning.

- Greg
Ben Pfaff April 17, 2017, 3:03 p.m. UTC | #7
On Sun, Apr 16, 2017 at 09:10:59PM -0700, Greg Rose wrote:
> On Sun, 2017-04-16 at 13:19 -0700, Ben Pfaff wrote:
> > On Fri, Apr 14, 2017 at 10:41:01AM -0700, Greg Rose wrote:
> > > On Fri, 2017-04-14 at 10:03 -0700, Ben Pfaff wrote:
> > > > On Fri, Apr 14, 2017 at 09:05:41AM -0700, Greg Rose wrote:
> > > > > On Thu, 2017-04-13 at 10:47 -0700, Ben Pfaff wrote:
> > > > > > Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> > > > > > does not exist") updated the RHEL logrotate configuration.  This commit
> > > > > > makes similar changes for Debian, by synchronizing with the RHEL version.
> > > > > > 
> > > > > > In particular:
> > > > > > 
> > > > > >     - Indent to match logrotate.conf(5) examples.
> > > > > > 
> > > > > >     - Use "sharedscripts" flag, because the postrotate script only needs to
> > > > > >       run once regardless of the number of rotations.
> > > > > > 
> > > > > >     - Drop "delaycompress", because the postrotate script does make daemons
> > > > > >       reopen their log files.
> > > > > > 
> > > > > >     - Ignore errors calling vlog/reopen.
> > > > > > 
> > > > > > Also make similar changes to the xenserver logrotate script.  I really
> > > > > > don't know if anyone uses the xenserver packaging anymore though.
> > > > > 
> > > > > Hi Ben,
> > > > > 
> > > > > I'm actually trying to build the latest upstream OVS from github on a
> > > > > Xen Server 7.1 system.  I have followed the instructions as well as I
> > > > > could and everything seems to build pretty well in the Xen Server 7.1
> > > > > DDK VM until finally it fails with the following error output:
> > > > > 
> > > > > Checking for unpackaged
> > > > > file(s): /usr/lib/rpm/check-files /prj/gvrose/rpmbuild/BUILDROOT/openvswitch-2.7.90-1.x86_64
> > > > > error: Installed (but unpackaged) file(s) found:
> > > > >    /usr/bin/ovs-tcpdump
> > > > >    /usr/share/man/man7/ovs-fields.7.gz
> > > > >    /usr/share/man/man8/ovs-tcpdump.8.gz
> > > > >    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> > > > > 
> > > > > 
> > > > > RPM build errors:
> > > > >     Installed (but unpackaged) file(s) found:
> > > > >    /usr/bin/ovs-tcpdump
> > > > >    /usr/share/man/man7/ovs-fields.7.gz
> > > > >    /usr/share/man/man8/ovs-tcpdump.8.gz
> > > > >    /usr/share/openvswitch/scripts/ovndb-servers.ocf
> > > > > 
> > > > > I suppose there is some file list that needs updating?
> > > > 
> > > > Yes, please look in xenserver/openvswitch-xen.spec.in.
> > > > 
> > > > However, this may be a good sign that no one cares about XenServer,
> > > > since Open vSwitch 2.6.0, released in September last year, also installs
> > > > at least /usr/bin/ovs-tcpdump, and no one has complained.  Perhaps we
> > > > should just delete the packaging, instead, and save ourselves some
> > > > trouble.
> > > 
> > > LOL - sure. Sounds good to me.  ;^)
> > > 
> > > In that case I won't bother sending a patch.
> > 
> > I guess you should send it, because I've heard from some XenServer users
> > via Twitter.
> 
> Alright, I'll get that out first thing tomorrow morning.

Thanks!
Ben Pfaff April 24, 2017, 6:33 p.m. UTC | #8
On Thu, Apr 13, 2017 at 10:47:55AM -0700, Ben Pfaff wrote:
> Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> does not exist") updated the RHEL logrotate configuration.  This commit
> makes similar changes for Debian, by synchronizing with the RHEL version.
> 
> In particular:
> 
>     - Indent to match logrotate.conf(5) examples.
> 
>     - Use "sharedscripts" flag, because the postrotate script only needs to
>       run once regardless of the number of rotations.
> 
>     - Drop "delaycompress", because the postrotate script does make daemons
>       reopen their log files.
> 
>     - Ignore errors calling vlog/reopen.
> 
> Also make similar changes to the xenserver logrotate script.  I really
> don't know if anyone uses the xenserver packaging anymore though.
> 
> CC: Timothy Redaelli <tredaelli@redhat.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>

This still needs a review.

Thanks,

Ben.
Gurucharan Shetty April 24, 2017, 7:25 p.m. UTC | #9
On 13 April 2017 at 10:47, Ben Pfaff <blp@ovn.org> wrote:

> Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> does not exist") updated the RHEL logrotate configuration.  This commit
> makes similar changes for Debian, by synchronizing with the RHEL version.
>
> In particular:
>
>     - Indent to match logrotate.conf(5) examples.
>
>     - Use "sharedscripts" flag, because the postrotate script only needs to
>       run once regardless of the number of rotations.
>
>     - Drop "delaycompress", because the postrotate script does make daemons
>       reopen their log files.
>
>     - Ignore errors calling vlog/reopen.
>
> Also make similar changes to the xenserver logrotate script.  I really
> don't know if anyone uses the xenserver packaging anymore though.
>
> CC: Timothy Redaelli <tredaelli@redhat.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
>

Acked-by: Gurucharan Shetty <guru@ovn.org>


> ---
>  debian/openvswitch-switch.logrotate   | 14 +++++++-------
>  xenserver/etc_logrotate.d_openvswitch | 22 ++++++++++++----------
>  2 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/debian/openvswitch-switch.logrotate
> b/debian/openvswitch-switch.logrotate
> index a7a71bdd90ad..7752af90cfed 100644
> --- a/debian/openvswitch-switch.logrotate
> +++ b/debian/openvswitch-switch.logrotate
> @@ -1,16 +1,16 @@
>  /var/log/openvswitch/*.log {
>      daily
>      compress
> +    sharedscripts
>      create 640 root adm
> -    delaycompress
>      missingok
>      rotate 30
>      postrotate
> -    # Tell Open vSwitch daemons to reopen their log files
> -    if [ -d /var/run/openvswitch ]; then
> -        for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> -            ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
> -        done
> -    fi
> +       # Tell Open vSwitch daemons to reopen their log files
> +       if [ -d /var/run/openvswitch ]; then
> +           for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> +               ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null
> || :
> +           done
> +       fi
>      endscript
>  }
> diff --git a/xenserver/etc_logrotate.d_openvswitch
> b/xenserver/etc_logrotate.d_openvswitch
> index 73751d4578b0..cd7b3a9d569d 100644
> --- a/xenserver/etc_logrotate.d_openvswitch
> +++ b/xenserver/etc_logrotate.d_openvswitch
> @@ -1,4 +1,4 @@
> -# Copyright (C) 2009, 2010, 2011, 2012 Nicira, Inc.
> +# Copyright (C) 2009, 2010, 2011, 2012, 2017 Nicira, Inc.
>  #
>  # Copying and distribution of this file, with or without modification,
>  # are permitted in any medium without royalty provided the copyright
> @@ -6,14 +6,16 @@
>  # without warranty of any kind.
>
>  /var/log/openvswitch/*.log {
> -       daily
> -       compress
> -       sharedscripts
> -       missingok
> -       postrotate
> +    daily
> +    compress
> +    sharedscripts
> +    missingok
> +    postrotate
>         # Tell Open vSwitch daemons to reopen their log files
> -        for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> -            ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
> -        done
> -       endscript
> +        if [ -d /var/run/openvswitch ]; then
> +           for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
> +               ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null
> || :
> +           done
> +       fi
> +    endscript
>  }
> --
> 2.10.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Ben Pfaff April 24, 2017, 7:52 p.m. UTC | #10
On Mon, Apr 24, 2017 at 12:25:51PM -0700, Guru Shetty wrote:
> On 13 April 2017 at 10:47, Ben Pfaff <blp@ovn.org> wrote:
> 
> > Commit 618a5b45ae8b ("rhel: Avoid logrotate error if /var/run/openvswitch
> > does not exist") updated the RHEL logrotate configuration.  This commit
> > makes similar changes for Debian, by synchronizing with the RHEL version.
> >
> > In particular:
> >
> >     - Indent to match logrotate.conf(5) examples.
> >
> >     - Use "sharedscripts" flag, because the postrotate script only needs to
> >       run once regardless of the number of rotations.
> >
> >     - Drop "delaycompress", because the postrotate script does make daemons
> >       reopen their log files.
> >
> >     - Ignore errors calling vlog/reopen.
> >
> > Also make similar changes to the xenserver logrotate script.  I really
> > don't know if anyone uses the xenserver packaging anymore though.
> >
> > CC: Timothy Redaelli <tredaelli@redhat.com>
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> >
> 
> Acked-by: Gurucharan Shetty <guru@ovn.org>

Thanks!  I applied this to master.
diff mbox

Patch

diff --git a/debian/openvswitch-switch.logrotate b/debian/openvswitch-switch.logrotate
index a7a71bdd90ad..7752af90cfed 100644
--- a/debian/openvswitch-switch.logrotate
+++ b/debian/openvswitch-switch.logrotate
@@ -1,16 +1,16 @@ 
 /var/log/openvswitch/*.log {
     daily
     compress
+    sharedscripts
     create 640 root adm
-    delaycompress
     missingok
     rotate 30
     postrotate
-    # Tell Open vSwitch daemons to reopen their log files
-    if [ -d /var/run/openvswitch ]; then
-        for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
-            ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
-        done
-    fi
+	# Tell Open vSwitch daemons to reopen their log files
+	if [ -d /var/run/openvswitch ]; then
+	    for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
+		ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
+	    done
+	fi
     endscript
 }
diff --git a/xenserver/etc_logrotate.d_openvswitch b/xenserver/etc_logrotate.d_openvswitch
index 73751d4578b0..cd7b3a9d569d 100644
--- a/xenserver/etc_logrotate.d_openvswitch
+++ b/xenserver/etc_logrotate.d_openvswitch
@@ -1,4 +1,4 @@ 
-# Copyright (C) 2009, 2010, 2011, 2012 Nicira, Inc.
+# Copyright (C) 2009, 2010, 2011, 2012, 2017 Nicira, Inc.
 #
 # Copying and distribution of this file, with or without modification,
 # are permitted in any medium without royalty provided the copyright
@@ -6,14 +6,16 @@ 
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
-	daily
-	compress
-	sharedscripts
-	missingok
-	postrotate
+    daily
+    compress
+    sharedscripts
+    missingok
+    postrotate
 	# Tell Open vSwitch daemons to reopen their log files
-        for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
-            ovs-appctl -t "${pidfile%%.pid}" vlog/reopen
-        done
-	endscript
+        if [ -d /var/run/openvswitch ]; then
+	    for pidfile in `cd /var/run/openvswitch && echo *.pid`; do
+		ovs-appctl -t "${pidfile%%.pid}" vlog/reopen 2>/dev/null || :
+	    done
+	fi
+    endscript
 }