diff mbox series

[ovs-dev,v3] ovn-sb.ovsschema: Avoid duplicated IPs in Encap table.

Message ID 1542308831-15818-1-git-send-email-hzhou8@ebay.com
State Superseded
Headers show
Series [ovs-dev,v3] ovn-sb.ovsschema: Avoid duplicated IPs in Encap table. | expand

Commit Message

Han Zhou Nov. 15, 2018, 7:07 p.m. UTC 
From: Han Zhou <hzhou8@ebay.com>

When adding a new chassis, if there is an old chassis with same IP
existed in Encap table, it is allowed to be added today. However,
allowing it to be added results in problems:

1. The new chassis cannot work because none of the other chassises
   are able to create tunnel to it, because of the IP confliction
   with already existed tunnel to the old chassis.

2. All the other chassises will continuously retry creating the tunnel
   and complaining about the error.

So, instead of hiding the problem, it is better to expose it while
trying to add the second chassis with duplicated IP. This patch
ensures it from the ovsdb schema.

Signed-off-by: Han Zhou <hzhou8@ebay.com>
---
v2 -> v3: update NEWS for the potential DB upgrading failure

 NEWS                 | 5 +++++
 ovn/ovn-sb.ovsschema | 7 ++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

Comments

Ben Pfaff Nov. 15, 2018, 7:31 p.m. UTC | #1 
On Thu, Nov 15, 2018 at 11:07:11AM -0800, Han Zhou wrote:
> From: Han Zhou <hzhou8@ebay.com>
> 
> When adding a new chassis, if there is an old chassis with same IP
> existed in Encap table, it is allowed to be added today. However,
> allowing it to be added results in problems:
> 
> 1. The new chassis cannot work because none of the other chassises
>    are able to create tunnel to it, because of the IP confliction
>    with already existed tunnel to the old chassis.
> 
> 2. All the other chassises will continuously retry creating the tunnel
>    and complaining about the error.
> 
> So, instead of hiding the problem, it is better to expose it while
> trying to add the second chassis with duplicated IP. This patch
> ensures it from the ovsdb schema.
> 
> Signed-off-by: Han Zhou <hzhou8@ebay.com>
> ---
> v2 -> v3: update NEWS for the potential DB upgrading failure

Thanks a lot for adding some upgrade information.

It's probably better to put detailed information like this for OVN
upgrades in Documentation/intro/install/ovn-upgrades.rst.  Then NEWS
could point to that advice.

Thanks,

Ben.
0-day Robot Nov. 15, 2018, 7:57 p.m. UTC | #2 
Bleep bloop.  Greetings Han Zhou, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 82 characters long (recommended limit is 79)
#38 FILE: NEWS:13:
       user can delete the entries using command "ovn-sbctl chassis-del <chassis>"

Lines checked: 70, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Han Zhou Nov. 16, 2018, 1:23 a.m. UTC | #3 
On Thu, Nov 15, 2018 at 11:31 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Thu, Nov 15, 2018 at 11:07:11AM -0800, Han Zhou wrote:
> > From: Han Zhou <hzhou8@ebay.com>
> >
> > When adding a new chassis, if there is an old chassis with same IP
> > existed in Encap table, it is allowed to be added today. However,
> > allowing it to be added results in problems:
> >
> > 1. The new chassis cannot work because none of the other chassises
> >    are able to create tunnel to it, because of the IP confliction
> >    with already existed tunnel to the old chassis.
> >
> > 2. All the other chassises will continuously retry creating the tunnel
> >    and complaining about the error.
> >
> > So, instead of hiding the problem, it is better to expose it while
> > trying to add the second chassis with duplicated IP. This patch
> > ensures it from the ovsdb schema.
> >
> > Signed-off-by: Han Zhou <hzhou8@ebay.com>
> > ---
> > v2 -> v3: update NEWS for the potential DB upgrading failure
>
> Thanks a lot for adding some upgrade information.
>
> It's probably better to put detailed information like this for OVN
> upgrades in Documentation/intro/install/ovn-upgrades.rst.  Then NEWS
> could point to that advice.
>
> Thanks,
>
> Ben.

Makes sense! Please check v4:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-November/353981.html
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 02402d1..b53978d 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,11 @@  Post-v2.10.0
    - The environment variable OVS_CTL_TIMEOUT, if set, is now used
      as the default timeout for control utilities.
    - ovn:
+     * OVN-SB schema changed: duplicated IP with same Encapsulation type
+       is not allowed any more. DB upgrading from an earlier version
+       could fail if there were already duplicated entries. In such case,
+       user can delete the entries using command "ovn-sbctl chassis-del <chassis>"
+       to delete them before upgrading.
      * New support for IPSEC encrypted tunnels between hypervisors.
      * ovn-ctl: allow passing user:group ids to the OVN daemons.
    - DPDK:
diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
index 5b9537f..afa9859 100644
--- a/ovn/ovn-sb.ovsschema
+++ b/ovn/ovn-sb.ovsschema
@@ -1,7 +1,7 @@ 
 {
     "name": "OVN_Southbound",
-    "version": "1.17.0",
-    "cksum": "3217981733 15045",
+    "version": "1.18.0",
+    "cksum": "910560265 15086",
     "tables": {
         "SB_Global": {
             "columns": {
@@ -50,7 +50,8 @@ 
                                      "min": 0,
                                      "max": "unlimited"}},
                 "ip": {"type": "string"},
-                "chassis_name": {"type": "string"}}},
+                "chassis_name": {"type": "string"}},
+            "indexes": [["type", "ip"]]},
         "Address_Set": {
             "columns": {
                 "name": {"type": "string"},