mbox series

[ovs-dev,v2,0/2] OVN DB log fixes

Message ID 20171108085813.19899-1-nusiddiq@redhat.com
Headers show
Series OVN DB log fixes | expand

Message

Numan Siddique Nov. 8, 2017, 8:58 a.m. UTC 
From: Numan Siddique <nusiddiq@redhat.com>

v1 -> v2
-------
In patch 2, changed the approach. Instead of fixing the issue in
ovs-appctl, corrected the ovs logrotate script to use complete unix ctl
file path as suggested by Ben.


No changes to patch 1.

v1
--
In Openstack deployment with OVN HA (with v2.7.2) in RHEL, we see that
OVN DB log files are empty after deployment. Adding "-vfile:info" option
when starting ovsdb-servers fixes this issue. Another issue seen is when
openvswitch logrotate script [1] is called, it doesn't initialize the log files
for the OVN DB servers because of which the log file is empty.

This patch series fixes these issue.

It would be good if these fixes are applied to branches 2.8 and 2.7.

[1] - https://github.com/openvswitch/ovs/blob/master/rhel/etc_logrotate.d_openvswitch


Numan Siddique (2):
  ovn-ctl: Add -vfile:info option to OVN_NB/SB_LOG options
  OpenvSwitch logrotate: Use ctl file path as target in ovs-appctl to
    reset logs

 debian/openvswitch-switch.logrotate | 6 +++---
 ovn/utilities/ovn-ctl               | 4 ++--
 rhel/etc_logrotate.d_openvswitch    | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

Comments

Ben Pfaff Nov. 27, 2017, 12:32 a.m. UTC | #1 
I applied this series to master.  Let me know if you want backports.

On Wed, Nov 08, 2017 at 02:28:13PM +0530, nusiddiq@redhat.com wrote:
> From: Numan Siddique <nusiddiq@redhat.com>
> 
> v1 -> v2
> -------
> In patch 2, changed the approach. Instead of fixing the issue in
> ovs-appctl, corrected the ovs logrotate script to use complete unix ctl
> file path as suggested by Ben.
> 
> 
> No changes to patch 1.
> 
> v1
> --
> In Openstack deployment with OVN HA (with v2.7.2) in RHEL, we see that
> OVN DB log files are empty after deployment. Adding "-vfile:info" option
> when starting ovsdb-servers fixes this issue. Another issue seen is when
> openvswitch logrotate script [1] is called, it doesn't initialize the log files
> for the OVN DB servers because of which the log file is empty.
> 
> This patch series fixes these issue.
> 
> It would be good if these fixes are applied to branches 2.8 and 2.7.
> 
> [1] - https://github.com/openvswitch/ovs/blob/master/rhel/etc_logrotate.d_openvswitch
> 
> 
> Numan Siddique (2):
>   ovn-ctl: Add -vfile:info option to OVN_NB/SB_LOG options
>   OpenvSwitch logrotate: Use ctl file path as target in ovs-appctl to
>     reset logs
> 
>  debian/openvswitch-switch.logrotate | 6 +++---
>  ovn/utilities/ovn-ctl               | 4 ++--
>  rhel/etc_logrotate.d_openvswitch    | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> -- 
> 2.13.5
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Numan Siddique Nov. 27, 2017, 7:42 a.m. UTC | #2 
Hi Ben,

Thanks for the review and applying the patches.

The issue is seen with 2.7 branch. So It would be great if it is backported
to 2.8 and 2.7 branches.

Thanks again

Numan


On Mon, Nov 27, 2017 at 6:02 AM, Ben Pfaff <blp@ovn.org> wrote:

> I applied this series to master.  Let me know if you want backports.
>
> On Wed, Nov 08, 2017 at 02:28:13PM +0530, nusiddiq@redhat.com wrote:
> > From: Numan Siddique <nusiddiq@redhat.com>
> >
> > v1 -> v2
> > -------
> > In patch 2, changed the approach. Instead of fixing the issue in
> > ovs-appctl, corrected the ovs logrotate script to use complete unix ctl
> > file path as suggested by Ben.
> >
> >
> > No changes to patch 1.
> >
> > v1
> > --
> > In Openstack deployment with OVN HA (with v2.7.2) in RHEL, we see that
> > OVN DB log files are empty after deployment. Adding "-vfile:info" option
> > when starting ovsdb-servers fixes this issue. Another issue seen is when
> > openvswitch logrotate script [1] is called, it doesn't initialize the
> log files
> > for the OVN DB servers because of which the log file is empty.
> >
> > This patch series fixes these issue.
> >
> > It would be good if these fixes are applied to branches 2.8 and 2.7.
> >
> > [1] - https://github.com/openvswitch/ovs/blob/master/
> rhel/etc_logrotate.d_openvswitch
> >
> >
> > Numan Siddique (2):
> >   ovn-ctl: Add -vfile:info option to OVN_NB/SB_LOG options
> >   OpenvSwitch logrotate: Use ctl file path as target in ovs-appctl to
> >     reset logs
> >
> >  debian/openvswitch-switch.logrotate | 6 +++---
> >  ovn/utilities/ovn-ctl               | 4 ++--
> >  rhel/etc_logrotate.d_openvswitch    | 4 ++--
> >  3 files changed, 7 insertions(+), 7 deletions(-)
> >
> > --
> > 2.13.5
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Ben Pfaff Nov. 27, 2017, 7:17 p.m. UTC | #3 
OK, I did these backports.

For branch-2.7 I had to backport all of the following commits to make
these patches apply cleanly:

commit 2abbe32153b7e4719b39f477b35e7cc40231338a
Author: Numan Siddique <nusiddiq@redhat.com>
Date:   Wed Nov 8 14:28:49 2017 +0530

    ovn-ctl: Add -vfile:info option to OVN_NB/SB_LOG options
    
    In the RHEL environment, when OVN db servers are started using ovn-ctl,
    log files are empty. Adding "-vfile:info" option to ovsdb-server is
    resolving this issue. Running 'ovs-apptctl -t .. vlog/reopen" results in the
    logs appearing in the log files. This issue is seen with 2.7.2.
    
    "-vfile:info" option is passed to ovn-northd and ovn-controller when starting.
    There is no harm in adding this to OVN db servers.
    
    Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
    Signed-off-by: Ben Pfaff <blp@ovn.org>

commit 7c8ef11c7571e377975b297a2df5564d481c467b
Author: Numan Siddique <nusiddiq@redhat.com>
Date:   Wed Nov 8 14:29:07 2017 +0530

    OpenvSwitch logrotate: Use ctl file path as target in ovs-appctl to reset logs
    
    Presently, logrotate script, searches for the pid files in /var/log/openvswitch
    and passes the pid file name (without .pid) as target to ovs-appctl. This approach
    doesn't work for OVN DB servers since the ctl files are generated as "ovnnb_db.ctl"
    and "ovnsb_db.ctl". So search for the .ctl files instead and use them as target to
    ovs-appctl.
    
    Suggested-by: Ben Pfaff <blp@ovn.org>
    Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
    Signed-off-by: Ben Pfaff <blp@ovn.org>
    Acked-by: Mark Michelson <mmichels@redhat.com>

commit 19f46fc05301ae420606fb059e80a931b3ca5ae8
Author: Ben Pfaff <blp@ovn.org>
Date:   Thu Apr 13 10:47:55 2017 -0700

    debian, xenserver: Update logrotate config to match RHEL.
    
    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 confirmed
    via Twitter that the xenserver packaging still has users.
    
    CC: Timothy Redaelli <tredaelli@redhat.com>
    Signed-off-by: Ben Pfaff <blp@ovn.org>
    Acked-by: Gurucharan Shetty <guru@ovn.org>

commit 866e0852290c7c17ff0b3e47f5ff03c16b7ba427
Author: Timothy Redaelli <tredaelli@redhat.com>
Date:   Thu Apr 13 11:48:20 2017 +0200

    rhel: Avoid logrotate error if /var/run/openvswitch does not exist
    
    Avoid also errors if an ovs server didn't start correctly or it crashed without
    deleting the pid file.
    
    Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1441524
    Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
    Signed-off-by: Ben Pfaff <blp@ovn.org>

commit 9352d3d4f5766778371affe5874763421ada3114
Author: Timothy Redaelli <tredaelli@redhat.com>
Date:   Thu Apr 13 11:48:19 2017 +0200

    rhel/etc_logrotate.d_openvswitch: Fix coding style
    
    Replace tabs by 4 spaces and indent the postrotate script like the
    examples in 'man logrotate.conf'
    
    Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
    Signed-off-by: Ben Pfaff <blp@ovn.org>


On Mon, Nov 27, 2017 at 01:12:18PM +0530, Numan Siddique wrote:
> Hi Ben,
> 
> Thanks for the review and applying the patches.
> 
> The issue is seen with 2.7 branch. So It would be great if it is backported
> to 2.8 and 2.7 branches.
> 
> Thanks again
> 
> Numan
> 
> 
> On Mon, Nov 27, 2017 at 6:02 AM, Ben Pfaff <blp@ovn.org> wrote:
> 
> > I applied this series to master.  Let me know if you want backports.
> >
> > On Wed, Nov 08, 2017 at 02:28:13PM +0530, nusiddiq@redhat.com wrote:
> > > From: Numan Siddique <nusiddiq@redhat.com>
> > >
> > > v1 -> v2
> > > -------
> > > In patch 2, changed the approach. Instead of fixing the issue in
> > > ovs-appctl, corrected the ovs logrotate script to use complete unix ctl
> > > file path as suggested by Ben.
> > >
> > >
> > > No changes to patch 1.
> > >
> > > v1
> > > --
> > > In Openstack deployment with OVN HA (with v2.7.2) in RHEL, we see that
> > > OVN DB log files are empty after deployment. Adding "-vfile:info" option
> > > when starting ovsdb-servers fixes this issue. Another issue seen is when
> > > openvswitch logrotate script [1] is called, it doesn't initialize the
> > log files
> > > for the OVN DB servers because of which the log file is empty.
> > >
> > > This patch series fixes these issue.
> > >
> > > It would be good if these fixes are applied to branches 2.8 and 2.7.
> > >
> > > [1] - https://github.com/openvswitch/ovs/blob/master/
> > rhel/etc_logrotate.d_openvswitch
> > >
> > >
> > > Numan Siddique (2):
> > >   ovn-ctl: Add -vfile:info option to OVN_NB/SB_LOG options
> > >   OpenvSwitch logrotate: Use ctl file path as target in ovs-appctl to
> > >     reset logs
> > >
> > >  debian/openvswitch-switch.logrotate | 6 +++---
> > >  ovn/utilities/ovn-ctl               | 4 ++--
> > >  rhel/etc_logrotate.d_openvswitch    | 4 ++--
> > >  3 files changed, 7 insertions(+), 7 deletions(-)
> > >
> > > --
> > > 2.13.5
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
Numan Siddique Nov. 28, 2017, 8:39 a.m. UTC | #4 
Thank you Ben for backports and for taking the trouble to apply the
dependent patches.
Next time I make sure to apply locally before requesting for the branches.


Thanks
Numan

On Tue, Nov 28, 2017 at 12:47 AM, Ben Pfaff <blp@ovn.org> wrote:

> OK, I did these backports.
>
> For branch-2.7 I had to backport all of the following commits to make
> these patches apply cleanly:
>
> commit 2abbe32153b7e4719b39f477b35e7cc40231338a
> Author: Numan Siddique <nusiddiq@redhat.com>
> Date:   Wed Nov 8 14:28:49 2017 +0530
>
>     ovn-ctl: Add -vfile:info option to OVN_NB/SB_LOG options
>
>     In the RHEL environment, when OVN db servers are started using ovn-ctl,
>     log files are empty. Adding "-vfile:info" option to ovsdb-server is
>     resolving this issue. Running 'ovs-apptctl -t .. vlog/reopen" results
> in the
>     logs appearing in the log files. This issue is seen with 2.7.2.
>
>     "-vfile:info" option is passed to ovn-northd and ovn-controller when
> starting.
>     There is no harm in adding this to OVN db servers.
>
>     Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
>     Signed-off-by: Ben Pfaff <blp@ovn.org>
>
> commit 7c8ef11c7571e377975b297a2df5564d481c467b
> Author: Numan Siddique <nusiddiq@redhat.com>
> Date:   Wed Nov 8 14:29:07 2017 +0530
>
>     OpenvSwitch logrotate: Use ctl file path as target in ovs-appctl to
> reset logs
>
>     Presently, logrotate script, searches for the pid files in
> /var/log/openvswitch
>     and passes the pid file name (without .pid) as target to ovs-appctl.
> This approach
>     doesn't work for OVN DB servers since the ctl files are generated as
> "ovnnb_db.ctl"
>     and "ovnsb_db.ctl". So search for the .ctl files instead and use them
> as target to
>     ovs-appctl.
>
>     Suggested-by: Ben Pfaff <blp@ovn.org>
>     Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
>     Signed-off-by: Ben Pfaff <blp@ovn.org>
>     Acked-by: Mark Michelson <mmichels@redhat.com>
>
> commit 19f46fc05301ae420606fb059e80a931b3ca5ae8
> Author: Ben Pfaff <blp@ovn.org>
> Date:   Thu Apr 13 10:47:55 2017 -0700
>
>     debian, xenserver: Update logrotate config to match RHEL.
>
>     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
> confirmed
>     via Twitter that the xenserver packaging still has users.
>
>     CC: Timothy Redaelli <tredaelli@redhat.com>
>     Signed-off-by: Ben Pfaff <blp@ovn.org>
>     Acked-by: Gurucharan Shetty <guru@ovn.org>
>
> commit 866e0852290c7c17ff0b3e47f5ff03c16b7ba427
> Author: Timothy Redaelli <tredaelli@redhat.com>
> Date:   Thu Apr 13 11:48:20 2017 +0200
>
>     rhel: Avoid logrotate error if /var/run/openvswitch does not exist
>
>     Avoid also errors if an ovs server didn't start correctly or it
> crashed without
>     deleting the pid file.
>
>     Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1441524
>     Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
>     Signed-off-by: Ben Pfaff <blp@ovn.org>
>
> commit 9352d3d4f5766778371affe5874763421ada3114
> Author: Timothy Redaelli <tredaelli@redhat.com>
> Date:   Thu Apr 13 11:48:19 2017 +0200
>
>     rhel/etc_logrotate.d_openvswitch: Fix coding style
>
>     Replace tabs by 4 spaces and indent the postrotate script like the
>     examples in 'man logrotate.conf'
>
>     Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
>     Signed-off-by: Ben Pfaff <blp@ovn.org>
>
>
> On Mon, Nov 27, 2017 at 01:12:18PM +0530, Numan Siddique wrote:
> > Hi Ben,
> >
> > Thanks for the review and applying the patches.
> >
> > The issue is seen with 2.7 branch. So It would be great if it is
> backported
> > to 2.8 and 2.7 branches.
> >
> > Thanks again
> >
> > Numan
> >
> >
> > On Mon, Nov 27, 2017 at 6:02 AM, Ben Pfaff <blp@ovn.org> wrote:
> >
> > > I applied this series to master.  Let me know if you want backports.
> > >
> > > On Wed, Nov 08, 2017 at 02:28:13PM +0530, nusiddiq@redhat.com wrote:
> > > > From: Numan Siddique <nusiddiq@redhat.com>
> > > >
> > > > v1 -> v2
> > > > -------
> > > > In patch 2, changed the approach. Instead of fixing the issue in
> > > > ovs-appctl, corrected the ovs logrotate script to use complete unix
> ctl
> > > > file path as suggested by Ben.
> > > >
> > > >
> > > > No changes to patch 1.
> > > >
> > > > v1
> > > > --
> > > > In Openstack deployment with OVN HA (with v2.7.2) in RHEL, we see
> that
> > > > OVN DB log files are empty after deployment. Adding "-vfile:info"
> option
> > > > when starting ovsdb-servers fixes this issue. Another issue seen is
> when
> > > > openvswitch logrotate script [1] is called, it doesn't initialize the
> > > log files
> > > > for the OVN DB servers because of which the log file is empty.
> > > >
> > > > This patch series fixes these issue.
> > > >
> > > > It would be good if these fixes are applied to branches 2.8 and 2.7.
> > > >
> > > > [1] - https://github.com/openvswitch/ovs/blob/master/
> > > rhel/etc_logrotate.d_openvswitch
> > > >
> > > >
> > > > Numan Siddique (2):
> > > >   ovn-ctl: Add -vfile:info option to OVN_NB/SB_LOG options
> > > >   OpenvSwitch logrotate: Use ctl file path as target in ovs-appctl to
> > > >     reset logs
> > > >
> > > >  debian/openvswitch-switch.logrotate | 6 +++---
> > > >  ovn/utilities/ovn-ctl               | 4 ++--
> > > >  rhel/etc_logrotate.d_openvswitch    | 4 ++--
> > > >  3 files changed, 7 insertions(+), 7 deletions(-)
> > > >
> > > > --
> > > > 2.13.5
> > > >
> > > > _______________________________________________
> > > > dev mailing list
> > > > dev@openvswitch.org
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > >
>