[ovs-dev] ovn-controller: Fix the chassis row recreation issue

From: Numan Siddique <nusiddiq@redhat.com>

Before the commit [1], ovn-controller would always recreate its
chassis row if deleted externally. After this commit, it no longer
recreates it. This is regression and needs to be fixed.

[1] - 242f1799fc22("ovn-controller: Refactor chassis.c to abstract the string parsing")

Fixes: 242f1799fc22("ovn-controller: Refactor chassis.c to abstract the string parsing")
CC: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
---
 ovn/controller/chassis.c |  4 ++++
 tests/ovn-controller.at  | 29 +++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

On Wed, Jul 24, 2019 at 3:07 AM <nusiddiq@redhat.com> wrote:
>
> From: Numan Siddique <nusiddiq@redhat.com>
>
> Before the commit [1], ovn-controller would always recreate its
> chassis row if deleted externally. After this commit, it no longer
> recreates it. This is regression and needs to be fixed.
>
> [1] - 242f1799fc22("ovn-controller: Refactor chassis.c to abstract the
string parsing")
>
> Fixes: 242f1799fc22("ovn-controller: Refactor chassis.c to abstract the
string parsing")
> CC: Dumitru Ceara <dceara@redhat.com>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
> ---
>  ovn/controller/chassis.c |  4 ++++
>  tests/ovn-controller.at  | 29 +++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
>
> diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
> index 04b98d86c..b74a42cc8 100644
> --- a/ovn/controller/chassis.c
> +++ b/ovn/controller/chassis.c
> @@ -486,6 +486,10 @@ chassis_get_record(struct ovsdb_idl_txn
*ovnsb_idl_txn,
>          if (!chassis_rec) {
>              VLOG_WARN("Could not find Chassis : stored (%s) ovs (%s)",
>                        chassis_info_id(&chassis_state), chassis_id);
> +            if (ovnsb_idl_txn) {
> +                /* Recreate the chassis record.  */
> +                chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn);
> +            }
>          }
>      } else {
>          chassis_rec =
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 343c2abed..63b2581c0 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -292,3 +292,32 @@ as ovn-sb
>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>
>  AT_CLEANUP
> +
> +# Checks that ovn-controller recreates its chassis record when deleted
externally.
> +AT_SETUP([ovn-controller - Chassis self record])
> +AT_KEYWORDS([ovn])
> +ovn_init_db ovn-sb
> +
> +net_add n1
> +sim_add hv
> +as hv
> +ovs-vsctl \
> +    -- add-br br-phys \
> +    -- add-br br-eth0 \
> +    -- add-br br-eth1 \
> +    -- add-br br-eth2
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +OVS_WAIT_UNTIL([test xhv = x`ovn-sbctl --columns name --bare find
chassis`])
> +# Delete the chassis "hv"
> +ovn-sbctl chassis-del hv
> +# ovn-controller should recreate its chassis row.
> +OVS_WAIT_UNTIL([test xhv = x`ovn-sbctl --columns name --bare find
chassis`])
> +
> +# Gracefully terminate daemons
> +OVN_CLEANUP_SBOX([hv])
> +OVN_CLEANUP_VSWITCH([main])
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +AT_CLEANUP
> --
> 2.21.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Acked-by: Han Zhou <hzhou8@ebay.com>

On Wed, Jul 24, 2019 at 12:06 PM <nusiddiq@redhat.com> wrote:
>
> From: Numan Siddique <nusiddiq@redhat.com>
>
> Before the commit [1], ovn-controller would always recreate its
> chassis row if deleted externally. After this commit, it no longer
> recreates it. This is regression and needs to be fixed.
>
> [1] - 242f1799fc22("ovn-controller: Refactor chassis.c to abstract the string parsing")
>
> Fixes: 242f1799fc22("ovn-controller: Refactor chassis.c to abstract the string parsing")
> CC: Dumitru Ceara <dceara@redhat.com>
> Signed-off-by: Numan Siddique <nusiddiq@redhat.com>

Hi Numan,

Sorry about that and thanks for the fix! Looks good to me.

Acked-by: Dumitru Ceara <dceara@redhat.com>

> ---
>  ovn/controller/chassis.c |  4 ++++
>  tests/ovn-controller.at  | 29 +++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
>
> diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c
> index 04b98d86c..b74a42cc8 100644
> --- a/ovn/controller/chassis.c
> +++ b/ovn/controller/chassis.c
> @@ -486,6 +486,10 @@ chassis_get_record(struct ovsdb_idl_txn *ovnsb_idl_txn,
>          if (!chassis_rec) {
>              VLOG_WARN("Could not find Chassis : stored (%s) ovs (%s)",
>                        chassis_info_id(&chassis_state), chassis_id);
> +            if (ovnsb_idl_txn) {
> +                /* Recreate the chassis record.  */
> +                chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn);
> +            }
>          }
>      } else {
>          chassis_rec =
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 343c2abed..63b2581c0 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -292,3 +292,32 @@ as ovn-sb
>  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>
>  AT_CLEANUP
> +
> +# Checks that ovn-controller recreates its chassis record when deleted externally.
> +AT_SETUP([ovn-controller - Chassis self record])
> +AT_KEYWORDS([ovn])
> +ovn_init_db ovn-sb
> +
> +net_add n1
> +sim_add hv
> +as hv
> +ovs-vsctl \
> +    -- add-br br-phys \
> +    -- add-br br-eth0 \
> +    -- add-br br-eth1 \
> +    -- add-br br-eth2
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +OVS_WAIT_UNTIL([test xhv = x`ovn-sbctl --columns name --bare find chassis`])
> +# Delete the chassis "hv"
> +ovn-sbctl chassis-del hv
> +# ovn-controller should recreate its chassis row.
> +OVS_WAIT_UNTIL([test xhv = x`ovn-sbctl --columns name --bare find chassis`])
> +
> +# Gracefully terminate daemons
> +OVN_CLEANUP_SBOX([hv])
> +OVN_CLEANUP_VSWITCH([main])
> +as ovn-sb
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> +
> +AT_CLEANUP
> --
> 2.21.0
>