From 941a0858be834736ae56ab397a98f6c5b6ed2a5d Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Sat, 20 Sep 2014 05:36:17 -0700 Subject: [PATCH] Bug 1998 - Fixing Bridge Set-Controller operation failure If Controller-Target already exist in the Controller table, the existing logic in setOFController fails badly due to a very basic bug of updating a wrong table. If the Target exist in the Controller table, the updateRow must happen only on the Bridge Table and Not on Controller Table. That is the root-cause of the issue seen recently. While reviewing this bug, I felt that the logic sorrounding this is unncessarily complex and I took this opportunity to clean it a bit as well. Change-Id: I14cd38759c16f8b444d2d14bd0b83d1df5a3b45d Signed-off-by: Madhu Venugopal --- .../openflow13/PipelineOrchestratorImpl.java | 1 - .../plugin/impl/ConfigurationServiceImpl.java | 74 +++++++------------ 2 files changed, 25 insertions(+), 50 deletions(-) diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/PipelineOrchestratorImpl.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/PipelineOrchestratorImpl.java index 826a4bd9a..c4a5aa370 100644 --- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/PipelineOrchestratorImpl.java +++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/PipelineOrchestratorImpl.java @@ -116,7 +116,6 @@ public class PipelineOrchestratorImpl implements PipelineOrchestrator, Opendayli } } catch (Exception e) { logger.warn("Processing interrupted, terminating ", e); - e.printStackTrace(); } while (!queue.isEmpty()) { diff --git a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/impl/ConfigurationServiceImpl.java b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/impl/ConfigurationServiceImpl.java index 394233b65..d29f77477 100644 --- a/plugin/src/main/java/org/opendaylight/ovsdb/plugin/impl/ConfigurationServiceImpl.java +++ b/plugin/src/main/java/org/opendaylight/ovsdb/plugin/impl/ConfigurationServiceImpl.java @@ -74,6 +74,7 @@ import org.slf4j.LoggerFactory; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import com.google.common.util.concurrent.ListenableFuture; public class ConfigurationServiceImpl implements IPluginInBridgeDomainConfigService, @@ -372,47 +373,30 @@ public class ConfigurationServiceImpl implements IPluginInBridgeDomainConfigServ return this.getTables(node, OvsVswitchdSchemaConstants.DATABASE_NAME); } - private List getControllerIPAddresses(Connection connection) { + private InetAddress getControllerIPAddress(Connection connection) { List controllers = null; InetAddress controllerIP = null; - controllers = new ArrayList(); String addressString = System.getProperty("ovsdb.controller.address"); if (addressString != null) { try { controllerIP = InetAddress.getByName(addressString); if (controllerIP != null) { - controllers.add(controllerIP); - return controllers; + return controllerIP; } } catch (UnknownHostException e) { logger.error("Host {} is invalid", addressString); } } - if (clusterServices != null) { - controllers = clusterServices.getClusteredControllers(); - if (controllers != null && controllers.size() > 0) { - if (controllers.size() == 1) { - InetAddress controller = controllers.get(0); - if (!controller.equals(InetAddress.getLoopbackAddress())) { - return controllers; - } - } else { - return controllers; - } - } - } - addressString = System.getProperty("of.address"); if (addressString != null) { try { controllerIP = InetAddress.getByName(addressString); if (controllerIP != null) { - controllers.add(controllerIP); - return controllers; + return controllerIP; } } catch (UnknownHostException e) { logger.error("Host {} is invalid", addressString); @@ -421,12 +405,11 @@ public class ConfigurationServiceImpl implements IPluginInBridgeDomainConfigServ try { controllerIP = connection.getClient().getConnectionInfo().getLocalAddress(); - controllers.add(controllerIP); - return controllers; + return controllerIP; } catch (Exception e) { logger.debug("Invalid connection provided to getControllerIPAddresses", e); } - return controllers; + return controllerIP; } private short getControllerOFPort() { @@ -444,8 +427,7 @@ public class ConfigurationServiceImpl implements IPluginInBridgeDomainConfigServ return openFlowPort; } - private Set getCurrentControllerTargets(Node node, final String controllerTableName) { - Set currentControllerTargets = new HashSet<>(); + private UUID getCurrentControllerUuid(Node node, final String controllerTableName, final String target) { ConcurrentMap rows = this.getRows(node, controllerTableName); if (rows != null) { @@ -453,12 +435,12 @@ public class ConfigurationServiceImpl implements IPluginInBridgeDomainConfigServ Controller currController = this.getTypedRow(node, Controller.class, entry.getValue()); Column column = currController.getTargetColumn(); String currTarget = column.getData(); - if (currTarget == null) continue; - currentControllerTargets.add(currTarget); + if (currTarget != null && currTarget.equalsIgnoreCase(target)) { + return currController.getUuid(); + } } } - - return currentControllerTargets; + return null; } @Override @@ -501,28 +483,22 @@ public class ConfigurationServiceImpl implements IPluginInBridgeDomainConfigServ } Status status = null; - Set currControllerTargets = null; - List ofControllerAddrs = this.getControllerIPAddresses(connection); + UUID currControllerUuid = null; + InetAddress ofControllerAddr = this.getControllerIPAddress(connection); short ofControllerPort = getControllerOFPort(); - for (InetAddress ofControllerAddress : ofControllerAddrs) { - String newControllerTarget = "tcp:"+ofControllerAddress.getHostAddress()+":"+ofControllerPort; - Controller newController = connection.getClient().createTypedRowWrapper(Controller.class); - newController.setTarget(newControllerTarget); - final String controllerTableName = newController.getSchema().getName(); - - // get current controller targets, iff this is the first iteration - if (currControllerTargets == null) { - currControllerTargets = getCurrentControllerTargets(node, controllerTableName); - } + String newControllerTarget = "tcp:"+ofControllerAddr.getHostAddress()+":"+ofControllerPort; + Controller newController = connection.getClient().createTypedRowWrapper(Controller.class); + newController.setTarget(newControllerTarget); + final String controllerTableName = newController.getSchema().getName(); - if (currControllerTargets.contains(newControllerTarget)) { - //ToDo: Status gets overwritten on each iteration. If any operation other than the last fails it's ignored. - status = this.updateRow(node, controllerTableName, null, bridgeUUID, newController.getRow()); - } else { - //ToDo: Status gets overwritten on each iteration. If any operation other than the last fails it's ignored. - status = this.insertRow(node, controllerTableName, bridgeUUID, newController.getRow()); - if (status.isSuccess()) currControllerTargets.add(newControllerTarget); - } + currControllerUuid = getCurrentControllerUuid(node, controllerTableName, newControllerTarget); + + if (currControllerUuid != null) { + bridge = connection.getClient().createTypedRowWrapper(Bridge.class); + bridge.setController(Sets.newHashSet(currControllerUuid)); + status = this.updateRow(node, bridge.getSchema().getName(), null, bridgeUUID, bridge.getRow()); + } else { + status = this.insertRow(node, controllerTableName, bridgeUUID, newController.getRow()); } if (status != null) { -- 2.36.6