T3: CLI enhancements and absent group buckets fix
- Print the packet in the H2H command
- Print Ip next to the hosts in pingall
- Fixed wrong output when there is an error due to group buckets missing

Change-Id: Iaf33fa6608565aa5e735e0fc7c8efc45c8a9bb31
(cherry picked from commit c108337ae0121b892c67f52bdd4ea1c15d20924c)
diff --git a/apps/t3/src/main/java/org/onosproject/t3/cli/T3CliUtils.java b/apps/t3/src/main/java/org/onosproject/t3/cli/T3CliUtils.java
index 30e1ca0..b0f2a81 100644
--- a/apps/t3/src/main/java/org/onosproject/t3/cli/T3CliUtils.java
+++ b/apps/t3/src/main/java/org/onosproject/t3/cli/T3CliUtils.java
@@ -16,6 +16,7 @@
 
 package org.onosproject.t3.cli;
 
+import org.apache.commons.lang.StringUtils;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.flow.TrafficTreatment;
 import org.onosproject.net.group.GroupBucket;
@@ -84,7 +85,6 @@
                 tracePrint.append("\n");
                 tracePrint = printFlows(trace, verbose, connectPoint, tracePrint);
                 tracePrint = printGroups(trace, verbose, connectPoint, tracePrint);
-                tracePrint.append("Output through " + connectPoint);
                 tracePrint.append("\n");
             } else {
                 for (ConnectPoint connectPoint : path) {
@@ -102,7 +102,7 @@
                     previous = connectPoint;
                 }
             }
-            tracePrint.append("---------------------------------------------------------------\n");
+            tracePrint.append(StringUtils.leftPad("\n", 100, '-'));
         }
         return tracePrint;
     }
diff --git a/apps/t3/src/main/java/org/onosproject/t3/cli/TroubleshootPingAllTraceCommand.java b/apps/t3/src/main/java/org/onosproject/t3/cli/TroubleshootPingAllTraceCommand.java
index 8cecc8d..4b262ca 100644
--- a/apps/t3/src/main/java/org/onosproject/t3/cli/TroubleshootPingAllTraceCommand.java
+++ b/apps/t3/src/main/java/org/onosproject/t3/cli/TroubleshootPingAllTraceCommand.java
@@ -16,10 +16,14 @@
 
 package org.onosproject.t3.cli;
 
+import org.apache.commons.lang.StringUtils;
 import org.apache.karaf.shell.commands.Command;
 import org.apache.karaf.shell.commands.Option;
+import org.onlab.packet.IpAddress;
 import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.net.Host;
+import org.onosproject.net.flow.criteria.Criterion;
+import org.onosproject.net.flow.criteria.IPCriterion;
 import org.onosproject.t3.api.StaticPacketTrace;
 import org.onosproject.t3.api.TroubleshootService;
 
@@ -54,36 +58,48 @@
         if (!type.equals(EtherType.IPV4) && !type.equals(EtherType.IPV6)) {
             print("Command only support IPv4 or IPv6");
         } else {
-            print("--------------------------------------------------------------------------");
+            print("%s", StringUtils.leftPad("", 100, '-'));
             //Obtain the list of traces
             List<StaticPacketTrace> traces = service.pingAll(type);
 
             if (traces.size() == 0) {
                 print("No traces were obtained, please check system configuration");
             }
-
+            boolean ipv4 = type.equals(EtherType.IPV4);
             traces.forEach(trace -> {
                 if (trace.getInitialPacket() != null) {
                     if (verbosity1) {
                         printVerbose(trace);
                     } else {
-                        printResultOnly(trace);
+                        printResultOnly(trace, ipv4);
                     }
                 } else {
                     print("Error in obtaining trace: %s", trace.resultMessage());
                 }
-                print("--------------------------------------------------------------------------");
+                print("%s", StringUtils.leftPad("", 100, '-'));
             });
         }
 
 
     }
 
-    private void printResultOnly(StaticPacketTrace trace) {
+    private void printResultOnly(StaticPacketTrace trace, boolean ipv4) {
         if (trace.getEndpointHosts().isPresent()) {
             Host source = trace.getEndpointHosts().get().getLeft();
             Host destination = trace.getEndpointHosts().get().getRight();
-            print("Source %s --> Destination %s", source.id(), destination.id());
+            IpAddress srcIP;
+            IpAddress dstIP;
+            if (ipv4 && trace.getInitialPacket().getCriterion(Criterion.Type.IPV4_SRC) != null) {
+                srcIP = ((IPCriterion) trace.getInitialPacket().getCriterion(Criterion.Type.IPV4_SRC)).ip().address();
+                dstIP = ((IPCriterion) trace.getInitialPacket().getCriterion(Criterion.Type.IPV4_DST)).ip().address();
+                print("Source %s (%s) --> Destination %s (%s)", source.id(), srcIP, destination.id(), dstIP);
+            } else if (trace.getInitialPacket().getCriterion(Criterion.Type.IPV6_SRC) != null) {
+                srcIP = ((IPCriterion) trace.getInitialPacket().getCriterion(Criterion.Type.IPV6_SRC)).ip().address();
+                dstIP = ((IPCriterion) trace.getInitialPacket().getCriterion(Criterion.Type.IPV6_DST)).ip().address();
+                print("Source %s (%s) --> Destination %s (%s)", source.id(), srcIP, destination.id(), dstIP);
+            } else {
+                print("Source %s --> Destination %s", source.id(), destination.id());
+            }
             print("%s", trace.resultMessage());
         } else {
             print("Can't gather host information from trace");
diff --git a/apps/t3/src/main/java/org/onosproject/t3/cli/TroubleshootSimpleTraceCommand.java b/apps/t3/src/main/java/org/onosproject/t3/cli/TroubleshootSimpleTraceCommand.java
index 3674e32..971163b 100644
--- a/apps/t3/src/main/java/org/onosproject/t3/cli/TroubleshootSimpleTraceCommand.java
+++ b/apps/t3/src/main/java/org/onosproject/t3/cli/TroubleshootSimpleTraceCommand.java
@@ -65,6 +65,7 @@
         //Build the trace
         StaticPacketTrace trace = service.trace(HostId.hostId(srcHost), HostId.hostId(dstHost), type);
         if (trace.getInitialPacket() != null) {
+            print("Tracing Packet: %s", trace.getInitialPacket());
             print("%s", T3CliUtils.printTrace(trace, verbosity1, verbosity2));
         } else {
             print("Cannot obtain trace between %s and %s", srcHost, dstHost);
diff --git a/apps/t3/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java b/apps/t3/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
index 2232210..7c4b2dd 100644
--- a/apps/t3/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
+++ b/apps/t3/src/main/java/org/onosproject/t3/impl/TroubleshootManager.java
@@ -968,6 +968,10 @@
                 trace.addResultMessage("Null group for Instruction " + instr);
                 break;
             }
+            if (group.buckets().buckets().size() == 0) {
+                trace.addResultMessage("Group " + group.id() + "has no buckets");
+                break;
+            }
 
             //Cycle in each of the group's buckets and add them to the groups for this Device.
             for (GroupBucket bucket : group.buckets().buckets()) {
diff --git a/apps/t3/src/test/java/org/onosproject/t3/impl/T3TestObjects.java b/apps/t3/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
index 0885653..aeffbb2 100644
--- a/apps/t3/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
+++ b/apps/t3/src/test/java/org/onosproject/t3/impl/T3TestObjects.java
@@ -593,6 +593,36 @@
 
     static final ConnectPoint LLDP_FLOW_CP = ConnectPoint.deviceConnectPoint(LLDP_FLOW_DEVICE + "/" + 1);
 
+    //No Buckets
+
+    static final DeviceId NO_BUCKET_DEVICE = DeviceId.deviceId("nobucket");
+
+    private static final TrafficSelector NO_BUCKET_SELECTOR = DefaultTrafficSelector.builder()
+            .matchInPort(PortNumber.portNumber(1))
+            .build();
+
+    private static final GroupId NO_BUCKET_GROUP_ID = GroupId.valueOf(1);
+
+    private static final TrafficTreatment NO_BUCKET_TREATMENT = DefaultTrafficTreatment.builder()
+            .group(NO_BUCKET_GROUP_ID)
+            .build();
+    private static final FlowRule NO_BUCKET_FLOW = DefaultFlowEntry.builder().forDevice(NO_BUCKET_DEVICE)
+            .forTable(0)
+            .withPriority(100)
+            .withSelector(NO_BUCKET_SELECTOR)
+            .withTreatment(NO_BUCKET_TREATMENT)
+            .fromApp(new DefaultApplicationId(0, "TestApp"))
+            .makePermanent()
+            .build();
+    static final FlowEntry NO_BUCKET_ENTRY = new DefaultFlowEntry(NO_BUCKET_FLOW);
+
+    private static final GroupBuckets NO_BUCKETS = new GroupBuckets(ImmutableList.of());
+
+    static final Group NO_BUCKET_GROUP =
+            new DefaultGroup(NO_BUCKET_GROUP_ID, NO_BUCKET_DEVICE, Group.Type.SELECT, NO_BUCKETS);
+
+    static final ConnectPoint NO_BUCKET_CP = ConnectPoint.deviceConnectPoint(NO_BUCKET_DEVICE + "/" + 1);
+
     //helper elements
 
     static final String MASTER_1 = "Master1";
diff --git a/apps/t3/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java b/apps/t3/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
index 6a80c12..fa19d22 100644
--- a/apps/t3/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
+++ b/apps/t3/src/test/java/org/onosproject/t3/impl/TroubleshootManagerTest.java
@@ -35,8 +35,8 @@
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Host;
 import org.onosproject.net.Link;
-import org.onosproject.net.PortNumber;
 import org.onosproject.net.Port;
+import org.onosproject.net.PortNumber;
 import org.onosproject.net.device.DeviceServiceAdapter;
 import org.onosproject.net.driver.DefaultDriver;
 import org.onosproject.net.driver.Driver;
@@ -160,6 +160,20 @@
     }
 
     /**
+     * Test group with no buckets.
+     */
+    @Test
+    public void noBucketsTest() throws Exception {
+
+        StaticPacketTrace traceFail = mngr.trace(PACKET_OK, NO_BUCKET_CP);
+        assertNotNull("Trace should not be null", traceFail);
+        assertTrue("Trace should be unsuccessful",
+                traceFail.resultMessage().contains("no buckets"));
+        log.info("trace {}", traceFail.resultMessage());
+
+    }
+
+    /**
      * Test a single flow rule that has output port in it.
      */
     @Test
@@ -416,6 +430,8 @@
                 return ImmutableList.of(LLDP_FLOW_ENTRY);
             } else if (deviceId.equals(MULTICAST_GROUP_FLOW_DEVICE)) {
                 return ImmutableList.of(MULTICAST_GROUP_FLOW_ENTRY);
+            } else if (deviceId.equals(NO_BUCKET_DEVICE)) {
+                return ImmutableList.of(NO_BUCKET_ENTRY);
             }
             return ImmutableList.of();
         }
@@ -444,6 +460,8 @@
                 return ImmutableList.of(DUAL_LINK_GROUP);
             } else if (deviceId.equals(MULTICAST_GROUP_FLOW_DEVICE)) {
                 return ImmutableList.of(MULTICAST_GROUP);
+            } else if (deviceId.equals(NO_BUCKET_DEVICE)) {
+                return ImmutableList.of(NO_BUCKET_GROUP);
             }
             return ImmutableList.of();
         }