[NETVIRT-799] NeutronPortChangeListener: handleNeutronPortUpdated doesn't catch, that Subnet ID for neutron port was changed (port was updated) Created: 21/Jul/17  Updated: 30/Oct/17  Resolved: 24/Aug/17

Status: Resolved
Project: netvirt
Component/s: General
Affects Version/s: Carbon
Fix Version/s: None

Type: Bug
Reporter: Valentina Krasnobaeva Assignee: Unassigned
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Operating System: All
Platform: All

External issue ID: 8875
Priority: Normal

 Description   

Issue was introduced by this gerrit change:

https://git.opendaylight.org/gerrit/#/c/45969
https://bugs.opendaylight.org/show_bug.cgi?id=6770

at NeutronPortChangeListener.java, handleNeutronPortUpdated method:

445 + final DataStoreJobCoordinator portDataStoreCoordinator = DataStoreJobCoordinator.getInstance();
446 + portDataStoreCoordinator.enqueueJob("PORT- " + portupdate.getUuid().getValue(), new
447 + Callable<List<ListenableFuture<Void>>>() {
448 + @Override
449 + public List<ListenableFuture<Void>> call() throws Exception {
450 + WriteTransaction wrtConfigTxn = dataBroker.newWriteOnlyTransaction();
451 + List<ListenableFuture<Void>> futures = new ArrayList<>();
452 +
453 + Uuid vpnIdNew = null;
454 + final Uuid subnetIdOr = portupdate.getFixedIps().get(0).getSubnetId();
455 + final Uuid subnetIdUp = portupdate.getFixedIps().get(0).getSubnetId();
456 + // check if subnet UUID has changed upon change in fixedIP
457 + final Boolean subnetUpdated = subnetIdUp.equals(subnetIdOr) ? false : true;
458 +
459 + if (subnetUpdated) {
460 + Subnetmap subnetMapOld = nvpnManager.removePortsFromSubnetmapNode(subnetIdOr, portoriginal
461 + .getUuid(), null);
462 + Uuid vpnIdOld = (subnetMapOld != null) ? subnetMapOld.getVpnId() : null;
463 + if (vpnIdOld != null) {
464 + // send port removed from subnet notification
465 + // only sent when the port was part of a VPN
466 + String elanInstanceName = portoriginal.getNetworkId().getValue();
467 + InstanceIdentifier<ElanInstance> elanIdentifierId = InstanceIdentifier.builder(ElanInstances
468 + .class).child(ElanInstance.class, new ElanInstanceKey(elanInstanceName)).build();
469 + Optional<ElanInstance> elanInstance = NeutronvpnUtils.read(dataBroker,
470 + LogicalDatastoreType.CONFIGURATION, elanIdentifierId);
471 + long elanTag = elanInstance.get().getElanTag();
472 + try

{ 473 + checkAndPublishPortRemoveNotification(subnetMapOld.getSubnetIp(), subnetIdOr, 474 + portoriginal.getUuid(), elanTag); 475 + LOG.debug("Port removed from subnet notification sent"); 476 + }

catch (Exception e)

{ 477 + LOG.error("Port removed from subnet notification failed", e); 478 + }

479 + }
480 + Subnetmap subnetMapNew = nvpnManager.updateSubnetmapNodeWithPorts(subnetIdUp, portupdate.getUuid(),
481 + null);
482 + vpnIdNew = (subnetMapNew != null) ? subnetMapNew.getVpnId() : null;
483 + if (vpnIdNew != null) {
484 + // send port added to subnet notification
485 + // only sent when the port is part of a VPN
486 + String elanInstanceName = portupdate.getNetworkId().getValue();
487 + InstanceIdentifier<ElanInstance> elanIdentifierId = InstanceIdentifier.builder(ElanInstances
488 + .class).child(ElanInstance.class, new ElanInstanceKey(elanInstanceName)).build();
489 + Optional<ElanInstance> elanInstance = NeutronvpnUtils.read(dataBroker,
490 + LogicalDatastoreType
491 + .CONFIGURATION, elanIdentifierId);
492 + long elanTag = elanInstance.get().getElanTag();
493 + try

{ 494 + checkAndPublishPortAddNotification(subnetMapNew.getSubnetIp(), subnetIdUp, portupdate 495 + .getUuid(), elanTag); 496 + LOG.debug("Port added to subnet notification sent"); 497 + }

catch (Exception e)

{ 498 + LOG.error("Port added to subnet notification failed", e); 499 + }

500 + }
501 + }
...

At lines 454, 455 we set values for subnetIdOr and subnetIdUp, but these variables will always have the same value, because we get SubnetId for both two variables from updated neutron port (portupdate) => comparation at line 457 will always give False => condition in line 459 will be never executed.

So this makes handleNeutronPortUpdated unable to catch, that Subnet ID for neutron port was changed (port was updated), hence, to threat a case, when we add a second subnet to a neutron network.

See appropriate log with added debug traces in attachement

 Comments   
Comment by Valentina Krasnobaeva [ 21/Jul/17 ]

(In reply to Valentina Krasnobaeva from comment #0)
> Issue was introduced by this gerrit change:
>
> https://git.opendaylight.org/gerrit/#/c/45969
> https://bugs.opendaylight.org/show_bug.cgi?id=6770
>
> at NeutronPortChangeListener.java, handleNeutronPortUpdated method:
>
> 445 + final DataStoreJobCoordinator portDataStoreCoordinator =
> DataStoreJobCoordinator.getInstance();
> 446 + portDataStoreCoordinator.enqueueJob("PORT- " +
> portupdate.getUuid().getValue(), new
> 447 + Callable<List<ListenableFuture<Void>>>() {
> 448 + @Override
> 449 + public List<ListenableFuture<Void>> call() throws
> Exception {
> 450 + WriteTransaction wrtConfigTxn =
> dataBroker.newWriteOnlyTransaction();
> 451 + List<ListenableFuture<Void>> futures = new
> ArrayList<>();
> 452 +
> 453 + Uuid vpnIdNew = null;
> 454 + final Uuid subnetIdOr =
> portupdate.getFixedIps().get(0).getSubnetId();
> 455 + final Uuid subnetIdUp =
> portupdate.getFixedIps().get(0).getSubnetId();
> 456 + // check if subnet UUID has changed upon change
> in fixedIP
> 457 + final Boolean subnetUpdated =
> subnetIdUp.equals(subnetIdOr) ? false : true;
> 458 +
> 459 + if (subnetUpdated) {
> 460 + Subnetmap subnetMapOld =
> nvpnManager.removePortsFromSubnetmapNode(subnetIdOr, portoriginal
> 461 + .getUuid(), null);
> 462 + Uuid vpnIdOld = (subnetMapOld != null) ?
> subnetMapOld.getVpnId() : null;
> 463 + if (vpnIdOld != null) {
> 464 + // send port removed from subnet
> notification
> 465 + // only sent when the port was part of
> a VPN
> 466 + String elanInstanceName =
> portoriginal.getNetworkId().getValue();
> 467 + InstanceIdentifier<ElanInstance>
> elanIdentifierId = InstanceIdentifier.builder(ElanInstances
> 468 +
> .class).child(ElanInstance.class, new
> ElanInstanceKey(elanInstanceName)).build();
> 469 + Optional<ElanInstance> elanInstance =
> NeutronvpnUtils.read(dataBroker,
> 470 +
> LogicalDatastoreType.CONFIGURATION, elanIdentifierId);
> 471 + long elanTag =
> elanInstance.get().getElanTag();
> 472 + try

{ > 473 + > checkAndPublishPortRemoveNotification(subnetMapOld.getSubnetIp(), subnetIdOr, > 474 + portoriginal.getUuid(), > elanTag); > 475 + LOG.debug("Port removed from subnet > notification sent"); > 476 + }

catch (Exception e)

{ > 477 + LOG.error("Port removed from subnet > notification failed", e); > 478 + }

> 479 + }
> 480 + Subnetmap subnetMapNew =
> nvpnManager.updateSubnetmapNodeWithPorts(subnetIdUp, portupdate.getUuid(),
> 481 + null);
> 482 + vpnIdNew = (subnetMapNew != null) ?
> subnetMapNew.getVpnId() : null;
> 483 + if (vpnIdNew != null) {
> 484 + // send port added to subnet
> notification
> 485 + // only sent when the port is part of a
> VPN
> 486 + String elanInstanceName =
> portupdate.getNetworkId().getValue();
> 487 + InstanceIdentifier<ElanInstance>
> elanIdentifierId = InstanceIdentifier.builder(ElanInstances
> 488 +
> .class).child(ElanInstance.class, new
> ElanInstanceKey(elanInstanceName)).build();
> 489 + Optional<ElanInstance> elanInstance =
> NeutronvpnUtils.read(dataBroker,
> 490 + LogicalDatastoreType
> 491 + .CONFIGURATION,
> elanIdentifierId);
> 492 + long elanTag =
> elanInstance.get().getElanTag();
> 493 + try

{ > 494 + > checkAndPublishPortAddNotification(subnetMapNew.getSubnetIp(), subnetIdUp, > portupdate > 495 + .getUuid(), elanTag); > 496 + LOG.debug("Port added to subnet > notification sent"); > 497 + }

catch (Exception e)

{ > 498 + LOG.error("Port added to subnet > notification failed", e); > 499 + }

> 500 + }
> 501 + }
> ...
>
> At lines 454, 455 we set values for subnetIdOr and subnetIdUp, but these
> variables will always have the same value, because we get SubnetId for both
> two variables from updated neutron port (portupdate) => comparation at line
> 457 will always give False => condition in line 459 will be never executed.
>
> So this makes handleNeutronPortUpdated unable to catch, that Subnet ID for
> neutron port was changed (port was updated), hence, to threat a case, when
> we add a second subnet to a neutron network.
>
> See appropriate log with added debug traces in attachement

Comment by Hari Prasidh [ 18/Aug/17 ]

(In reply to Valentina Krasnobaeva from comment #0)
> Issue was introduced by this gerrit change:
>
> https://git.opendaylight.org/gerrit/#/c/45969
> https://bugs.opendaylight.org/show_bug.cgi?id=6770
>
> at NeutronPortChangeListener.java, handleNeutronPortUpdated method:
>
> 445 + final DataStoreJobCoordinator portDataStoreCoordinator =
> DataStoreJobCoordinator.getInstance();
> 446 + portDataStoreCoordinator.enqueueJob("PORT- " +
> portupdate.getUuid().getValue(), new
> 447 + Callable<List<ListenableFuture<Void>>>() {
> 448 + @Override
> 449 + public List<ListenableFuture<Void>> call() throws
> Exception {
> 450 + WriteTransaction wrtConfigTxn =
> dataBroker.newWriteOnlyTransaction();
> 451 + List<ListenableFuture<Void>> futures = new
> ArrayList<>();
> 452 +
> 453 + Uuid vpnIdNew = null;
> 454 + final Uuid subnetIdOr =
> portupdate.getFixedIps().get(0).getSubnetId();
> 455 + final Uuid subnetIdUp =
> portupdate.getFixedIps().get(0).getSubnetId();
> 456 + // check if subnet UUID has changed upon change
> in fixedIP
> 457 + final Boolean subnetUpdated =
> subnetIdUp.equals(subnetIdOr) ? false : true;
> 458 +
> 459 + if (subnetUpdated) {
> 460 + Subnetmap subnetMapOld =
> nvpnManager.removePortsFromSubnetmapNode(subnetIdOr, portoriginal
> 461 + .getUuid(), null);
> 462 + Uuid vpnIdOld = (subnetMapOld != null) ?
> subnetMapOld.getVpnId() : null;
> 463 + if (vpnIdOld != null) {
> 464 + // send port removed from subnet
> notification
> 465 + // only sent when the port was part of
> a VPN
> 466 + String elanInstanceName =
> portoriginal.getNetworkId().getValue();
> 467 + InstanceIdentifier<ElanInstance>
> elanIdentifierId = InstanceIdentifier.builder(ElanInstances
> 468 +
> .class).child(ElanInstance.class, new
> ElanInstanceKey(elanInstanceName)).build();
> 469 + Optional<ElanInstance> elanInstance =
> NeutronvpnUtils.read(dataBroker,
> 470 +
> LogicalDatastoreType.CONFIGURATION, elanIdentifierId);
> 471 + long elanTag =
> elanInstance.get().getElanTag();
> 472 + try

{ > 473 + > checkAndPublishPortRemoveNotification(subnetMapOld.getSubnetIp(), subnetIdOr, > 474 + portoriginal.getUuid(), > elanTag); > 475 + LOG.debug("Port removed from subnet > notification sent"); > 476 + }

catch (Exception e)

{ > 477 + LOG.error("Port removed from subnet > notification failed", e); > 478 + }

> 479 + }
> 480 + Subnetmap subnetMapNew =
> nvpnManager.updateSubnetmapNodeWithPorts(subnetIdUp, portupdate.getUuid(),
> 481 + null);
> 482 + vpnIdNew = (subnetMapNew != null) ?
> subnetMapNew.getVpnId() : null;
> 483 + if (vpnIdNew != null) {
> 484 + // send port added to subnet
> notification
> 485 + // only sent when the port is part of a
> VPN
> 486 + String elanInstanceName =
> portupdate.getNetworkId().getValue();
> 487 + InstanceIdentifier<ElanInstance>
> elanIdentifierId = InstanceIdentifier.builder(ElanInstances
> 488 +
> .class).child(ElanInstance.class, new
> ElanInstanceKey(elanInstanceName)).build();
> 489 + Optional<ElanInstance> elanInstance =
> NeutronvpnUtils.read(dataBroker,
> 490 + LogicalDatastoreType
> 491 + .CONFIGURATION,
> elanIdentifierId);
> 492 + long elanTag =
> elanInstance.get().getElanTag();
> 493 + try

{ > 494 + > checkAndPublishPortAddNotification(subnetMapNew.getSubnetIp(), subnetIdUp, > portupdate > 495 + .getUuid(), elanTag); > 496 + LOG.debug("Port added to subnet > notification sent"); > 497 + }

catch (Exception e)

{ > 498 + LOG.error("Port added to subnet > notification failed", e); > 499 + }

> 500 + }
> 501 + }
> ...
>
> At lines 454, 455 we set values for subnetIdOr and subnetIdUp, but these
> variables will always have the same value, because we get SubnetId for both
> two variables from updated neutron port (portupdate) => comparation at line
> 457 will always give False => condition in line 459 will be never executed.
>
> So this makes handleNeutronPortUpdated unable to catch, that Subnet ID for
> neutron port was changed (port was updated), hence, to threat a case, when
> we add a second subnet to a neutron network.
>
> See appropriate log with added debug traces in attachement

Hi Valentina,

Can you please provide me the clear procedure in which scenario you have observed this issue and please update the logs.

Comment by Hari Prasidh [ 22/Aug/17 ]

Am trying to see the issue for that I've tested below scenarios.

Test1 :
Created Network and subnet.
observed above code is not executed.

Test2 :
Created Network and 2 subnets on the same network.
observed above code is not executed.

Test3:
Created Network and 2 subnets on the same network.
created router and attached both subnets with router.
observed above code is not executed.

Valentina,
Will you please tell me in which scenario/test, the above code snippet can be execute?

Comment by Hari Prasidh [ 23/Aug/17 ]

With latest git pull for networking-odl, below mentioned code got triggered.

Comment by Hari Prasidh [ 24/Aug/17 ]

Bug has been fixed.

Patch pushed to Carbon & Master. Please find below,

Carbon : https://git.opendaylight.org/gerrit/#/c/60636/
Master : https://git.opendaylight.org/gerrit/#/c/60650/

Generated at Wed Feb 07 20:22:30 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.