Redoing NatRule usage and some fixes based on issues found while writing

unittests

plugins/network-elements/nicira-nvp/src/com/cloud/network/nicira/DestinationNatRule.java

@@ -1,24 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements.  See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership.  The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
-//
-//   http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
-// under the License.
-package com.cloud.network.nicira;
-
-public class DestinationNatRule extends NatRule {
-	{
-		type = "DestinationNatRule";
-	}
-	
-}

plugins/network-elements/nicira-nvp/src/com/cloud/network/nicira/Match.java

@@ -128,6 +128,126 @@ public class Match {
 	public void setDestinationIpAddressesNot(Boolean destination_ip_addresses_not) {
 		this.destination_ip_addresses_not = destination_ip_addresses_not;
 	}
+
+	@Override
+	public int hashCode() {
+		final int prime = 31;
+		int result = 1;
+		result = prime
+				* result
+				+ ((destination_ip_addresses == null) ? 0
+						: destination_ip_addresses.hashCode());
+		result = prime
+				* result
+				+ ((destination_ip_addresses_not == null) ? 0
+						: destination_ip_addresses_not.hashCode());
+		result = prime
+				* result
+				+ ((destination_port_max == null) ? 0 : destination_port_max
+						.hashCode());
+		result = prime
+				* result
+				+ ((destination_port_min == null) ? 0 : destination_port_min
+						.hashCode());
+		result = prime
+				* result
+				+ ((destination_port_not == null) ? 0 : destination_port_not
+						.hashCode());
+		result = prime * result
+				+ ((ethertype == null) ? 0 : ethertype.hashCode());
+		result = prime * result
+				+ ((protocol == null) ? 0 : protocol.hashCode());
+		result = prime
+				* result
+				+ ((source_ip_addresses == null) ? 0 : source_ip_addresses
+						.hashCode());
+		result = prime
+				* result
+				+ ((source_ip_addresses_not == null) ? 0
+						: source_ip_addresses_not.hashCode());
+		result = prime * result
+				+ ((source_port_max == null) ? 0 : source_port_max.hashCode());
+		result = prime * result
+				+ ((source_port_min == null) ? 0 : source_port_min.hashCode());
+		result = prime * result
+				+ ((source_port_not == null) ? 0 : source_port_not.hashCode());
+		return result;
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (obj == null)
+			return false;
+		if (getClass() != obj.getClass())
+			return false;
+		Match other = (Match) obj;
+		if (destination_ip_addresses == null) {
+			if (other.destination_ip_addresses != null)
+				return false;
+		} else if (!destination_ip_addresses
+				.equals(other.destination_ip_addresses))
+			return false;
+		if (destination_ip_addresses_not == null) {
+			if (other.destination_ip_addresses_not != null)
+				return false;
+		} else if (!destination_ip_addresses_not
+				.equals(other.destination_ip_addresses_not))
+			return false;
+		if (destination_port_max == null) {
+			if (other.destination_port_max != null)
+				return false;
+		} else if (!destination_port_max.equals(other.destination_port_max))
+			return false;
+		if (destination_port_min == null) {
+			if (other.destination_port_min != null)
+				return false;
+		} else if (!destination_port_min.equals(other.destination_port_min))
+			return false;
+		if (destination_port_not == null) {
+			if (other.destination_port_not != null)
+				return false;
+		} else if (!destination_port_not.equals(other.destination_port_not))
+			return false;
+		if (ethertype == null) {
+			if (other.ethertype != null)
+				return false;
+		} else if (!ethertype.equals(other.ethertype))
+			return false;
+		if (protocol == null) {
+			if (other.protocol != null)
+				return false;
+		} else if (!protocol.equals(other.protocol))
+			return false;
+		if (source_ip_addresses == null) {
+			if (other.source_ip_addresses != null)
+				return false;
+		} else if (!source_ip_addresses.equals(other.source_ip_addresses))
+			return false;
+		if (source_ip_addresses_not == null) {
+			if (other.source_ip_addresses_not != null)
+				return false;
+		} else if (!source_ip_addresses_not
+				.equals(other.source_ip_addresses_not))
+			return false;
+		if (source_port_max == null) {
+			if (other.source_port_max != null)
+				return false;
+		} else if (!source_port_max.equals(other.source_port_max))
+			return false;
+		if (source_port_min == null) {
+			if (other.source_port_min != null)
+				return false;
+		} else if (!source_port_min.equals(other.source_port_min))
+			return false;
+		if (source_port_not == null) {
+			if (other.source_port_not != null)
+				return false;
+		} else if (!source_port_not.equals(other.source_port_not))
+			return false;
+		return true;
+	}
 	
 	
 }

plugins/network-elements/nicira-nvp/src/com/cloud/network/nicira/NatRule.java

@@ -110,4 +110,114 @@ public class NatRule {
 	public String getType() {
 		return type;
 	}
+	
+	public void setType(String type) {
+		this.type = type;
+	}
+
+	@Override
+	public int hashCode() {
+		final int prime = 42;
+		int result = 1;
+		result = prime * result + ((match == null) ? 0 : match.hashCode());
+		result = prime
+				* result
+				+ ((to_destination_ip_address_max == null) ? 0
+						: to_destination_ip_address_max.hashCode());
+		result = prime
+				* result
+				+ ((to_destination_ip_address_min == null) ? 0
+						: to_destination_ip_address_min.hashCode());
+		result = prime
+				* result
+				+ ((to_destination_port == null) ? 0 : to_destination_port
+						.hashCode());
+		result = prime
+				* result
+				+ ((to_source_ip_address_max == null) ? 0
+						: to_source_ip_address_max.hashCode());
+		result = prime
+				* result
+				+ ((to_source_ip_address_min == null) ? 0
+						: to_source_ip_address_min.hashCode());
+		result = prime
+				* result
+				+ ((to_source_port_max == null) ? 0 : to_source_port_max
+						.hashCode());
+		result = prime
+				* result
+				+ ((to_source_port_min == null) ? 0 : to_source_port_min
+						.hashCode());
+		result = prime * result + ((type == null) ? 0 : type.hashCode());
+		result = prime * result + ((uuid == null) ? 0 : uuid.hashCode());
+		return result;
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (this == obj)
+			return true;
+		if (obj == null)
+			return false;
+		if (getClass() != obj.getClass())
+			return false;
+		NatRule other = (NatRule) obj;
+		if (match == null) {
+			if (other.match != null)
+				return false;
+		} else if (!match.equals(other.match))
+			return false;
+		if (to_destination_ip_address_max == null) {
+			if (other.to_destination_ip_address_max != null)
+				return false;
+		} else if (!to_destination_ip_address_max
+				.equals(other.to_destination_ip_address_max))
+			return false;
+		if (to_destination_ip_address_min == null) {
+			if (other.to_destination_ip_address_min != null)
+				return false;
+		} else if (!to_destination_ip_address_min
+				.equals(other.to_destination_ip_address_min))
+			return false;
+		if (to_destination_port == null) {
+			if (other.to_destination_port != null)
+				return false;
+		} else if (!to_destination_port.equals(other.to_destination_port))
+			return false;
+		if (to_source_ip_address_max == null) {
+			if (other.to_source_ip_address_max != null)
+				return false;
+		} else if (!to_source_ip_address_max
+				.equals(other.to_source_ip_address_max))
+			return false;
+		if (to_source_ip_address_min == null) {
+			if (other.to_source_ip_address_min != null)
+				return false;
+		} else if (!to_source_ip_address_min
+				.equals(other.to_source_ip_address_min))
+			return false;
+		if (to_source_port_max == null) {
+			if (other.to_source_port_max != null)
+				return false;
+		} else if (!to_source_port_max.equals(other.to_source_port_max))
+			return false;
+		if (to_source_port_min == null) {
+			if (other.to_source_port_min != null)
+				return false;
+		} else if (!to_source_port_min.equals(other.to_source_port_min))
+			return false;
+		if (type == null) {
+			if (other.type != null)
+				return false;
+		} else if (!type.equals(other.type))
+			return false;
+		if (uuid == null) {
+			if (other.uuid != null)
+				return false;
+		} else if (!uuid.equals(other.uuid))
+			return false;
+		return true;
+	}
+	
+	
 }

plugins/network-elements/nicira-nvp/src/com/cloud/network/nicira/NiciraNvpApi.java

@@ -240,14 +240,7 @@ public class NiciraNvpApi {
     public NatRule createLogicalRouterNatRule(String logicalRouterUuid, NatRule natRule) throws NiciraNvpApiException {
     	String uri = "/ws.v1/lrouter/" + logicalRouterUuid + "/nat";
     	
-    	if (natRule instanceof SourceNatRule) {
-    		return executeCreateObject(natRule, new TypeToken<SourceNatRule>(){}.getType(), uri, Collections.<String,String>emptyMap());
-    	}
-    	else if (natRule instanceof DestinationNatRule) {
-    		return executeCreateObject(natRule, new TypeToken<DestinationNatRule>(){}.getType(), uri, Collections.<String,String>emptyMap());
-    	}
-    	
-    	throw new NiciraNvpApiException("Unknown NatRule type");
+    	return executeCreateObject(natRule, new TypeToken<NatRule>(){}.getType(), uri, Collections.<String,String>emptyMap());
     }
     
     public void modifyLogicalRouterNatRule(String logicalRouterUuid, NatRule natRule) throws NiciraNvpApiException {

plugins/network-elements/nicira-nvp/src/com/cloud/network/nicira/SourceNatRule.java

@@ -1,27 +0,0 @@
-// Licensed to the Apache Software Foundation (ASF) under one
-// or more contributor license agreements.  See the NOTICE file
-// distributed with this work for additional information
-// regarding copyright ownership.  The ASF licenses this file
-// to you under the Apache License, Version 2.0 (the
-// "License"); you may not use this file except in compliance
-// with the License.  You may obtain a copy of the License at
-// 
-//   http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing,
-// software distributed under the License is distributed on an
-// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-// KIND, either express or implied.  See the License for the
-// specific language governing permissions and limitations
-// under the License.
-package com.cloud.network.nicira;
-
-/**
- * 
- */
-public class SourceNatRule extends NatRule {
-	{
-		type = "SourceNatRule";
-	}	
-	
-}

plugins/network-elements/nicira-nvp/src/com/cloud/network/resource/NiciraNvpResource.java

@@ -60,9 +60,7 @@ import com.cloud.agent.api.to.PortForwardingRuleTO;
 import com.cloud.agent.api.to.StaticNatRuleTO;
 import com.cloud.host.Host;
 import com.cloud.host.Host.Type;
-import com.cloud.network.nicira.Attachment;
 import com.cloud.network.nicira.ControlClusterStatus;
-import com.cloud.network.nicira.DestinationNatRule;
 import com.cloud.network.nicira.L3GatewayAttachment;
 import com.cloud.network.nicira.LogicalRouterConfig;
 import com.cloud.network.nicira.LogicalRouterPort;
@@ -77,13 +75,10 @@ import com.cloud.network.nicira.NiciraNvpTag;
 import com.cloud.network.nicira.PatchAttachment;
 import com.cloud.network.nicira.RouterNextHop;
 import com.cloud.network.nicira.SingleDefaultRouteImplictRoutingConfig;
-import com.cloud.network.nicira.SourceNatRule;
 import com.cloud.network.nicira.TransportZoneBinding;
 import com.cloud.network.nicira.VifAttachment;
 import com.cloud.resource.ServerResource;
 
-import edu.emory.mathcs.backport.java.util.Arrays;
-
 public class NiciraNvpResource implements ServerResource {
     private static final Logger s_logger = Logger.getLogger(NiciraNvpResource.class);
     
@@ -307,7 +302,13 @@ public class NiciraNvpResource implements ServerResource {
 
             LogicalSwitchPort logicalSwitchPort = new LogicalSwitchPort(attachmentUuid, tags, true);
             LogicalSwitchPort newPort = _niciraNvpApi.createLogicalSwitchPort(logicalSwitchUuid, logicalSwitchPort);
-            _niciraNvpApi.modifyLogicalSwitchPortAttachment(cmd.getLogicalSwitchUuid(), newPort.getUuid(), new VifAttachment(attachmentUuid));
+            try {
+            	_niciraNvpApi.modifyLogicalSwitchPortAttachment(cmd.getLogicalSwitchUuid(), newPort.getUuid(), new VifAttachment(attachmentUuid));
+            } catch (NiciraNvpApiException ex) {
+            	s_logger.warn("modifyLogicalSwitchPort failed after switchport was created, removing switchport");
+            	_niciraNvpApi.deleteLogicalSwitchPort(cmd.getLogicalSwitchUuid(), newPort.getUuid());
+            	throw (ex); // Rethrow the original exception
+            }
             return new CreateLogicalSwitchPortAnswer(cmd, true, "Logical switch port " + newPort.getUuid() + " created", newPort.getUuid());
         } catch (NiciraNvpApiException e) {
         	if (numRetries > 0) {
@@ -404,6 +405,9 @@ public class NiciraNvpResource implements ServerResource {
         			new RouterNextHop(publicNetworkNextHopIp)));
         	lrc = _niciraNvpApi.createLogicalRouter(lrc);
         	
+        	// store the switchport for rollback
+        	LogicalSwitchPort lsp = null;
+        	
         	try {
 	        	// Create the outside port for the router
 	        	LogicalRouterPort lrpo = new LogicalRouterPort();
@@ -433,7 +437,7 @@ public class NiciraNvpResource implements ServerResource {
 	        	lrpi = _niciraNvpApi.createLogicalRouterPort(lrc.getUuid(),lrpi);
 	        	
 	        	// Create the inside port on the lswitch
-	            LogicalSwitchPort lsp = new LogicalSwitchPort(truncate(routerName + "-inside-port", 40), tags, true);
+	            lsp = new LogicalSwitchPort(truncate(routerName + "-inside-port", 40), tags, true);
 	            lsp = _niciraNvpApi.createLogicalSwitchPort(logicalSwitchUuid, lsp);
 	       	
 	        	// Attach the inside router port to the lswitch port with a PatchAttachment
@@ -445,7 +449,8 @@ public class NiciraNvpResource implements ServerResource {
 	            		new PatchAttachment(lrpi.getUuid()));
 	            
 	            // Setup the source nat rule
-	            SourceNatRule snr = new SourceNatRule();
+	            NatRule snr = new NatRule();
+	            snr.setType("SourceNatRule");
 	            snr.setToSourceIpAddressMin(publicNetworkIpAddress.split("/")[0]);
 	            snr.setToSourceIpAddressMax(publicNetworkIpAddress.split("/")[0]);
 	            Match match = new Match();
@@ -454,10 +459,12 @@ public class NiciraNvpResource implements ServerResource {
 	            _niciraNvpApi.createLogicalRouterNatRule(lrc.getUuid(), snr);
         	} catch (NiciraNvpApiException e) {
         		// We need to destroy the router if we already created it
-        		// this will also take care of any router ports
-        		// TODO Clean up the switchport
+        		// this will also take care of any router ports and rules
         		try {
         			_niciraNvpApi.deleteLogicalRouter(lrc.getUuid());
+        			if (lsp != null) {
+        				_niciraNvpApi.deleteLogicalSwitchPort(logicalSwitchUuid, lsp.getUuid());
+        			}
         		} catch (NiciraNvpApiException ex) {}
         		
         		throw e;
@@ -498,7 +505,8 @@ public class NiciraNvpResource implements ServerResource {
     		lrp.setIpAddresses(cmd.getPublicCidrs());
     		_niciraNvpApi.modifyLogicalRouterPort(cmd.getLogicalRouterUuid(), lrp);
     		
-    		return new ConfigurePublicIpsOnLogicalRouterAnswer(cmd, true, "Logical Router deleted (uuid " + cmd.getLogicalRouterUuid() + ")");
+    		return new ConfigurePublicIpsOnLogicalRouterAnswer(cmd, true, "Configured " + cmd.getPublicCidrs().size() + 
+    				" ip addresses on logical router uuid " + cmd.getLogicalRouterUuid());
         } catch (NiciraNvpApiException e) {
         	if (numRetries > 0) {
         		return retry(cmd, --numRetries);
@@ -581,21 +589,23 @@ public class NiciraNvpResource implements ServerResource {
 					// create the dnat rule
 					Match m = new Match();
 					m.setDestinationIpAddresses(outsideCidr);
-					DestinationNatRule newDnatRule = new DestinationNatRule();
+					NatRule newDnatRule = new NatRule();
+					newDnatRule.setType("DestinationNatRule");
 					newDnatRule.setMatch(m);
 					newDnatRule.setToDestinationIpAddressMin(insideIp);
 					newDnatRule.setToDestinationIpAddressMax(insideIp);
-					newDnatRule = (DestinationNatRule) _niciraNvpApi.createLogicalRouterNatRule(cmd.getLogicalRouterUuid(), newDnatRule);
+					newDnatRule = _niciraNvpApi.createLogicalRouterNatRule(cmd.getLogicalRouterUuid(), newDnatRule);
 					s_logger.debug("Created " + natRuleToString(newDnatRule));
 
 					// create matching snat rule
 					m = new Match();
 					m.setSourceIpAddresses(insideIp + "/32");
-					SourceNatRule newSnatRule = new SourceNatRule();
+					NatRule newSnatRule = new NatRule();
+					newSnatRule.setType("SourceNatRule");
 					newSnatRule.setMatch(m);
 					newSnatRule.setToSourceIpAddressMin(outsideIp);
 					newSnatRule.setToSourceIpAddressMax(outsideIp);
-					newSnatRule = (SourceNatRule) _niciraNvpApi.createLogicalRouterNatRule(cmd.getLogicalRouterUuid(), newSnatRule);
+					newSnatRule = _niciraNvpApi.createLogicalRouterNatRule(cmd.getLogicalRouterUuid(), newSnatRule);
 					s_logger.debug("Created " + natRuleToString(newSnatRule));
 					
 				}
@@ -698,12 +708,13 @@ public class NiciraNvpResource implements ServerResource {
 					}
 					m.setDestinationPortMin(rule.getSrcPortRange()[0]);
 					m.setDestinationPortMax(rule.getSrcPortRange()[1]);
-					DestinationNatRule newDnatRule = new DestinationNatRule();
+					NatRule newDnatRule = new NatRule();
+					newDnatRule.setType("DestinationNatRule");
 					newDnatRule.setMatch(m);
 					newDnatRule.setToDestinationIpAddressMin(insideIp);
 					newDnatRule.setToDestinationIpAddressMax(insideIp);
 					newDnatRule.setToDestinationPort(rule.getDstPortRange()[0]);
-					newDnatRule = (DestinationNatRule) _niciraNvpApi.createLogicalRouterNatRule(cmd.getLogicalRouterUuid(), newDnatRule);
+					newDnatRule = _niciraNvpApi.createLogicalRouterNatRule(cmd.getLogicalRouterUuid(), newDnatRule);
 					s_logger.debug("Created " + natRuleToString(newDnatRule));
 					
 					// create matching snat rule
@@ -717,13 +728,14 @@ public class NiciraNvpResource implements ServerResource {
 					}
 					m.setSourcePortMin(rule.getDstPortRange()[0]);
 					m.setSourcePortMax(rule.getDstPortRange()[1]);
-					SourceNatRule newSnatRule = new SourceNatRule();
+					NatRule newSnatRule = new NatRule();
+					newSnatRule.setType("SourceNatRule");
 					newSnatRule.setMatch(m);
 					newSnatRule.setToSourceIpAddressMin(outsideIp);
 					newSnatRule.setToSourceIpAddressMax(outsideIp);
 					newSnatRule.setToSourcePortMin(rule.getSrcPortRange()[0]);
 					newSnatRule.setToSourcePortMax(rule.getSrcPortRange()[1]);
-					newSnatRule = (SourceNatRule) _niciraNvpApi.createLogicalRouterNatRule(cmd.getLogicalRouterUuid(), newSnatRule);
+					newSnatRule = _niciraNvpApi.createLogicalRouterNatRule(cmd.getLogicalRouterUuid(), newSnatRule);
 					s_logger.debug("Created " + natRuleToString(newSnatRule));
 					
 				}
@@ -749,9 +761,8 @@ public class NiciraNvpResource implements ServerResource {
     }    
 
     private Answer retry(Command cmd, int numRetries) {
-        int numRetriesRemaining = numRetries - 1;
-        s_logger.warn("Retrying " + cmd.getClass().getSimpleName() + ". Number of retries remaining: " + numRetriesRemaining);
-        return executeRequest(cmd, numRetriesRemaining);
+        s_logger.warn("Retrying " + cmd.getClass().getSimpleName() + ". Number of retries remaining: " + numRetries);
+        return executeRequest(cmd, numRetries);
     }
     
     private String natRuleToString(NatRule rule) {
@@ -808,4 +819,17 @@ public class NiciraNvpResource implements ServerResource {
     	}
     }
     
+    private NatRule[] generateStaticNatRulePair(String insideIp, String outsideIp) {
+    	NatRule[] rulepair = new NatRule[2];
+    	rulepair[0] = new NatRule();
+    	rulepair[0].setType("DestinationNatRule");
+    	rulepair[1] = new NatRule();
+    	rulepair[1].setType("SourceNatRule");
+    	
+    	//FIXME Implement
+    	
+    	return rulepair;
+    	
+    }
+    
 }