[OVSDB-410] TerminationPoint reconciliation fails if one port is already present Created: 24/Apr/17  Updated: 30/Oct/17  Resolved: 26/Apr/17

Status: Resolved
Project: ovsdb
Component/s: Southbound.Open_vSwitch
Affects Version/s: unspecified
Fix Version/s: None

Type: Bug
Reporter: Vishal Thapar Assignee: Vishal Thapar
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: 8280

 Description   

TerminationPoint reconciliation has a bug where reconciliation can fail if even one port doesn't require reconciliation. The issue is with TerminationPointCreateCommand, which checks if TerminationPoint already exists in operational or not. It seems to be failing for ports that already exist and we end up reconciling ports that are already present.

However, trying to create a port that already exists results in constraint violation error:

{ "error": "constraint violation", "details": "Transaction causes multiple rows in \"Interface\" table to have identical values (\"tun21435b8e5dd\") for index on column \"name\". First row, with UUID fdb4afd2-9877-42bb-9dac-c35e96b3de6d, existed in the database before this transaction and was not modified by the transaction. Second row, with UUID 63d879a2-77fb-4401-9ee0-d2861791c192, was inserted by this transaction." }

A failure in any of operations in a transaction results in all other operations also failing. This means operations to add rest of the ports fail and they never get reconciled.

Steps to reproduce:

1. Create two tunnel interfaces through OVSDB plugin.
2. Note OFPort number to determine which one was second to be added (higher OF Port number).
3. Disconnect the switch from ODL (del-manager)
4. Delete the port with higher OF Port number from switch through CLI.
5. Reconnect the switch using set-manager

Expected behavior:
Port deleted in step 4 should show back up.

Actual behavior:
Port is never added.

If first port is deleted, it will not throw constraint violation, but second one will.

 Comments   
Comment by Vishal Thapar [ 24/Apr/17 ]

The issue is in TerminationPointConfigReconciliationTask.java, Line 57:

@Override
public boolean reconcileConfiguration(final OvsdbConnectionManager connectionManager) {
LOG.debug("Reconcile Termination Point Configuration for node {}", ((Node) configData).getNodeId());
final Map<InstanceIdentifier<?>, DataObject> changes = new HashMap<>();
changes.putAll(SouthboundMapper.extractTerminationPointConfigurationChanges((Node) configData));
BridgeOperationalState bridgeOperationalState =
new BridgeOperationalState(reconciliationManager.getDb(), Collections.EMPTY_LIST) {
@Override
public Optional<TerminationPoint> getBridgeTerminationPoint(InstanceIdentifier<?> iid)

{ return Optional.absent(); }

};

We're initializing BridgeOperationalState always as empty. In TerminationPointCreate when we check if TP exists in operational or not:
InstanceIdentifier terminationPointIid = entry.getKey();
Optional<TerminationPoint> terminationPointOptional =
state.getBridgeTerminationPoint(terminationPointIid);
if (!terminationPointOptional.isPresent()) {

it always returns absent, so we end up reconciling ports that are already present in operational datastore.

This looks like an artifact of earlier version of the patch [1] where Bridge reconciliation was taking are of TPs too, or probably some pending code that never got in. If testing was done by deleting all ports, it this issue won't be hit.

I'm working on a fix, will be pushing soon.

Question: Should this be termed critical? As it stands, reconciliation of ports is currently broken in all branches.

[1] https://git.opendaylight.org/gerrit/#/c/40506/]

Comment by Vishal Thapar [ 24/Apr/17 ]

Marking it as blocker as per discussion with Sam in IRC, as I see this issue even on single node.

Patches
-------
master: https://git.opendaylight.org/gerrit/55901

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