On 12/18/23 15:25, Mohammad Heib wrote:
> When the user updates the NB_GLOBAL.name after registering
> to IC Databases if the user already has defined chassis as a gateway
> that will cause ovn-ic instance to run in an infinity loop trying
> to update the gateways and insert the current gateway to the SB.chassis
> tables as a remote chassis (we match on the new AZ ) which will fail since
> we already have this chassis with is-interconn in local SB.
>
> This patch aims to fix the above issues by updating the AZ.name only
> when the user updates the NB.name locally.
>
> Signed-off-by: Mohammad Heib <[email protected]>
> ---
Thanks for the patch, Mohammad, and sorry for the delay in reviewing it.
> ic/ovn-ic.c | 10 +++++++---
> tests/ovn-ic.at | 23 +++++++++++++++++++++++
> 2 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 8ceb34d7c..12e2729ce 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -132,14 +132,18 @@ az_run(struct ic_context *ctx)
> return NULL;
> }
>
> - /* Delete old AZ if name changes. Note: if name changed when ovn-ic
> - * is not running, one has to manually delete the old AZ with:
> + /* Update old AZ if name changes. Note: if name changed when ovn-ic
> + * is not running, one has to manually delete/update the old AZ with:
> * "ovn-ic-sbctl destroy avail <az>". */
> static char *az_name;
> const struct icsbrec_availability_zone *az;
> if (az_name && strcmp(az_name, nb_global->name)) {
> ICSBREC_AVAILABILITY_ZONE_FOR_EACH (az, ctx->ovnisb_idl) {
> - if (!strcmp(az->name, az_name)) {
> + /* AZ name update locally need to update az in ISB. */
> + if (nb_global->name[0] && !strcmp(az->name, az_name)) {
> + icsbrec_availability_zone_set_name(az, nb_global->name);
> + break;
> + } else if (!nb_global->name[0] && !strcmp(az->name, az_name)) {
> icsbrec_availability_zone_delete(az);
> break;
> }
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index d4c436f84..5cc504e17 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -28,7 +28,30 @@ availability-zone az3
> ])
>
> OVN_CLEANUP_IC([az1], [az2])
> +AT_CLEANUP
> +])
> +
> +
We're missing a "OVN_FOR_EACH_NORTHD([" here.
> +AT_SETUP([ovn-ic -- AZ update in GW])
> +ovn_init_ic_db
> +net_add n1
>
> +ovn_start az1
> +sim_add gw-az1
> +as gw-az1
> +
> +check ovs-vsctl add-br br-phys
> +ovn_az_attach az1 n1 br-phys 192.168.1.1
> +check ovs-vsctl set open . external-ids:ovn-is-interconn=true
> +
> +az_uuid=$(fetch_column ic-sb:availability-zone _uuid name="az1")
> +ovn_as az1 ovn-nbctl set NB_Global . name="az2"
Nit: missing "check".
> +wait_column "$az_uuid" ic-sb:availability-zone _uuid name="az2"
> +
> +# make sure that gateway still point to the same AZ with new name
> +wait_column "$az_uuid" ic-sb:gateway availability_zone name="gw-az1"
> +
> +OVN_CLEANUP_IC([az1])
> AT_CLEANUP
> ])
>
The rest looks correct to me, feel free to add my ack to v2 if you
address the two comments above.
Acked-by: Dumitru Ceara <[email protected]>
Thanks,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
