Refactor ReconciliationServiceImpl 38/108238/16
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 11 Feb 2024 22:58:44 +0000 (23:58 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Mon, 12 Feb 2024 10:23:48 +0000 (11:23 +0100)
This service implements a single RPC, Reconcile, and is injected into
its sole user.

Defile a separate interface, ReconcileService, which defines the two
possibilities the RPC can handle.

Rename ReconciliationServiceImpl to DefaultReconcileService and let it
register the RPC itself. While we are at it, convert it to OSGi DS.

JIRA: OPNFLWPLUG-1112
JIRA: OPNFLWPLUG-1125
Change-Id: Ifc9d3707191484b492ebf5a190f65d5b673a5bdc
Signed-off-by: lubos-cicut <lubos.cicut@pantheon.tech>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
applications/forwardingrules-manager/pom.xml
applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/DefaultReconcileService.java [moved from applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/ReconciliationServiceImpl.java with 77% similarity]
applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/ReconcileService.java [new file with mode: 0644]
applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/cli/Reconciliation.java
applications/southbound-cli/src/main/resources/OSGI-INF/blueprint/commands.xml
applications/southbound-cli/src/main/resources/OSGI-INF/blueprint/southbound-cli.xml [deleted file]
applications/southbound-cli/src/main/yang/reconciliation.yang

index dad42d742ea66c47853d6b4805bcc994f02cb713..7e0d981586ef167b5256607167de5c7b58d0cb1e 100644 (file)
     </dependency>
   </dependencies>
 
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.felix</groupId>
+        <artifactId>maven-bundle-plugin</artifactId>
+        <extensions>true</extensions>
+        <configuration>
+          <instructions>
+            <Provide-Capability>
+              osgi.service;objectClass:List&lt;String&gt;="org.opendaylight.openflowplugin.applications.frm.ForwardingRulesManager";uses:="org.opendaylight.openflowplugin.applications.frm"
+            </Provide-Capability>
+          </instructions>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+
   <scm>
     <connection>scm:git:ssh://git.opendaylight.org:29418/openflowplugin.git</connection>
     <developerConnection>scm:git:ssh://git.opendaylight.org:29418/openflowplugin.git</developerConnection>
@@ -14,7 +14,6 @@ import static org.opendaylight.openflowplugin.api.openflow.ReconciliationState.R
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.util.concurrent.ListenableFuture;
-import com.google.common.util.concurrent.SettableFuture;
 import java.lang.management.ManagementFactory;
 import java.text.SimpleDateFormat;
 import java.time.LocalDateTime;
@@ -27,6 +26,9 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.stream.Collectors;
+import javax.annotation.PreDestroy;
+import javax.inject.Inject;
+import javax.inject.Singleton;
 import javax.management.InstanceAlreadyExistsException;
 import javax.management.InstanceNotFoundException;
 import javax.management.MBeanRegistrationException;
@@ -49,11 +51,11 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.Reconcile;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconcileInput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconcileOutput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconcileOutputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconciliationCounter;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconciliationService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.reconciliation.counter.ReconcileCounter;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.reconciliation.counter.ReconcileCounterBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.reconciliation.counter.ReconcileCounterKey;
@@ -64,12 +66,18 @@ import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.opendaylight.yangtools.yang.common.RpcResultBuilder;
 import org.opendaylight.yangtools.yang.common.Uint32;
 import org.opendaylight.yangtools.yang.common.Uint64;
+import org.osgi.service.component.annotations.Activate;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
+import org.osgi.service.component.annotations.Reference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-// FIXME: this is not just a CLI component, it should live somewhere else
-public final class ReconciliationServiceImpl implements ReconciliationService, AutoCloseable {
-    private static final Logger LOG = LoggerFactory.getLogger(ReconciliationServiceImpl.class);
+@Singleton
+@Component(service = ReconcileService.class, immediate = true)
+// FIXME: this should probably live in FRM, but how does it integrate with its functionality?
+public final class DefaultReconcileService implements Reconcile, ReconcileService, AutoCloseable {
+    private static final Logger LOG = LoggerFactory.getLogger(DefaultReconcileService.class);
     private static final ObjectName ALARM_NAME;
 
     static {
@@ -87,16 +95,17 @@ public final class ReconciliationServiceImpl implements ReconciliationService, A
     private final DataBroker broker;
 
     private ExecutorService executor = Executors.newWorkStealingPool(10);
-    private boolean unregister;
+    private boolean unregister = false;
 
-    public ReconciliationServiceImpl(final DataBroker broker, final ForwardingRulesManager frm,
-            final DpnTracker dpnTracker, final FlowGroupCacheManager flowGroupCacheManager) {
+    @Inject
+    @Activate
+    public DefaultReconcileService(@Reference final DataBroker broker, @Reference final ForwardingRulesManager frm,
+            @Reference final DpnTracker dpnTracker, @Reference final FlowGroupCacheManager flowGroupCacheManager) {
         this.broker = requireNonNull(broker);
         flowNodeReconciliation = frm.getFlowNodeReconciliation();
         this.dpnTracker = requireNonNull(dpnTracker);
         reconciliationStates = flowGroupCacheManager.getReconciliationStates();
 
-        unregister = false;
         final var mbs = ManagementFactory.getPlatformMBeanServer();
         if (!mbs.isRegistered(ALARM_NAME)) {
             try {
@@ -109,6 +118,8 @@ public final class ReconciliationServiceImpl implements ReconciliationService, A
         }
     }
 
+    @PreDestroy
+    @Deactivate
     @Override
     public void close() {
         if (unregister) {
@@ -127,62 +138,72 @@ public final class ReconciliationServiceImpl implements ReconciliationService, A
     }
 
     @Override
-    public ListenableFuture<RpcResult<ReconcileOutput>> reconcile(final ReconcileInput input) {
-        boolean reconcileAllNodes = input.getReconcileAllNodes();
-        Set<Uint64> inputNodes = input.getNodes();
-        if (inputNodes == null) {
-            inputNodes = Set.of();
+    public ListenableFuture<RpcResult<ReconcileOutput>> reconcile(final Set<Uint64> nodes) {
+        if (nodes == null || nodes.isEmpty()) {
+            return buildErrorResponse("Error executing command reconcile. No Node information was specified.");
         }
-        if (reconcileAllNodes && inputNodes.size() > 0) {
+
+        final var allNodes = getAllNodes();
+        final var unresolvedNodes = nodes.stream().filter(node -> !allNodes.contains(node))
+            .collect(Collectors.toList());
+        if (!unresolvedNodes.isEmpty()) {
             return buildErrorResponse("Error executing command reconcile. "
-                    + "If 'all' option is enabled, no Node must be specified as input parameter.");
+                + "Node(s) not found: " + String.join(", ", unresolvedNodes.toString()));
         }
-        if (!reconcileAllNodes && inputNodes.size() == 0) {
-            return buildErrorResponse("Error executing command reconcile. No Node information was specified.");
+        return doReconcile(nodes.stream().map(Uint64::longValue).collect(Collectors.toList()));
+    }
+
+    @Override
+    public ListenableFuture<RpcResult<ReconcileOutput>> reconcileAll() {
+        return doReconcile(getAllNodes());
+    }
+
+    private @NonNull ListenableFuture<RpcResult<ReconcileOutput>> doReconcile(final List<Long> nodes) {
+        if (!nodes.isEmpty()) {
+            return buildErrorResponse(
+                "Error executing command reconcile. No node information is found for reconciliation");
         }
-        SettableFuture<RpcResult<ReconcileOutput>> result = SettableFuture.create();
-        List<Long> nodeList = getAllNodes();
-        List<Long> nodesToReconcile = reconcileAllNodes ? nodeList :
-                inputNodes.stream().distinct().map(Uint64::longValue).collect(Collectors.toList());
-        if (nodesToReconcile.size() > 0) {
-            List<Long> unresolvedNodes =
-                    nodesToReconcile.stream().filter(node -> !nodeList.contains(node)).collect(Collectors.toList());
-            if (!unresolvedNodes.isEmpty()) {
-                return buildErrorResponse("Error executing command reconcile. "
-                        + "Node(s) not found: " + String.join(", ", unresolvedNodes.toString()));
+        final var inprogressNodes = ImmutableSet.<Uint64>builder();
+        nodes.parallelStream().forEach(nodeId -> {
+            ReconciliationState state = getReconciliationState(nodeId);
+            if (state != null && state.getState().equals(STARTED)) {
+                inprogressNodes.add(Uint64.valueOf(nodeId));
+            } else {
+                final var alarmText = getAlarmText(nodeId,  " started reconciliation");
+                final var source = getSourceText(nodeId);
+                LOG.debug("Raising NodeReconciliationOperationOngoing alarm, alarmText {} source {}", alarmText,
+                    source);
+                alarm.raiseAlarm("NodeReconciliationOperationOngoing", alarmText, source);
+                LOG.info("Executing reconciliation for node {} with state ", nodeId);
+                NodeKey nodeKey = new NodeKey(new NodeId("openflow:" + nodeId));
+                executor.execute(new ReconciliationTask(Uint64.valueOf(nodeId), nodeKey));
             }
-            ImmutableSet.Builder<Uint64> inprogressNodes = ImmutableSet.builder();
-            nodesToReconcile.parallelStream().forEach(nodeId -> {
-                ReconciliationState state = getReconciliationState(nodeId);
-                if (state != null && state.getState().equals(STARTED)) {
-                    inprogressNodes.add(Uint64.valueOf(nodeId));
-                } else {
-                    final var alarmText = getAlarmText(nodeId,  " started reconciliation");
-                    final var source = getSourceText(nodeId);
-                    LOG.debug("Raising NodeReconciliationOperationOngoing alarm, alarmText {} source {}", alarmText,
-                        source);
-                    alarm.raiseAlarm("NodeReconciliationOperationOngoing", alarmText, source);
-                    LOG.info("Executing reconciliation for node {} with state ", nodeId);
-                    NodeKey nodeKey = new NodeKey(new NodeId("openflow:" + nodeId));
-                    executor.execute(new ReconciliationTask(Uint64.valueOf(nodeId), nodeKey));
-                }
-            });
-            ReconcileOutput reconcilingInProgress = new ReconcileOutputBuilder()
-                    .setInprogressNodes(inprogressNodes.build())
-                    .build();
-            result.set(RpcResultBuilder.success(reconcilingInProgress).build());
-            return result;
-        } else {
-            return buildErrorResponse("Error executing command reconcile. "
-                    + "No node information is found for reconciliation");
+        });
+        return RpcResultBuilder.success(new ReconcileOutputBuilder()
+            .setInprogressNodes(inprogressNodes.build())
+            .build())
+            .buildFuture();
+    }
+
+    @Override
+    public ListenableFuture<RpcResult<ReconcileOutput>> invoke(final ReconcileInput input) {
+        final var reconcileAllNodes = input.getReconcileAllNodes();
+        final var nodes = input.getNodes();
+        if (reconcileAllNodes != null && reconcileAllNodes) {
+            if (nodes != null && nodes.size() > 0) {
+                return buildErrorResponse("Error executing command reconcile. If 'all' option is enabled, no Node "
+                    + "must be specified as input parameter.");
+            }
+            return reconcileAll();
         }
+        return reconcile(nodes == null ? Set.of() : nodes);
     }
 
     private ReconciliationState getReconciliationState(final Long nodeId) {
         return reconciliationStates.get(nodeId.toString());
     }
 
-    private static ListenableFuture<RpcResult<ReconcileOutput>> buildErrorResponse(final String msg) {
+    private static @NonNull ListenableFuture<RpcResult<ReconcileOutput>> buildErrorResponse(final String msg) {
         LOG.error("Error {}", msg);
         return RpcResultBuilder.<ReconcileOutput>failed()
                 .withError(ErrorType.PROTOCOL, new ErrorTag("reconcile"), msg)
diff --git a/applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/ReconcileService.java b/applications/southbound-cli/src/main/java/org/opendaylight/openflowplugin/applications/southboundcli/ReconcileService.java
new file mode 100644 (file)
index 0000000..b2cb404
--- /dev/null
@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2024 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.openflowplugin.applications.southboundcli;
+
+import com.google.common.util.concurrent.ListenableFuture;
+import java.util.Set;
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconcileOutput;
+import org.opendaylight.yangtools.yang.common.RpcResult;
+import org.opendaylight.yangtools.yang.common.Uint64;
+
+@NonNullByDefault
+public interface ReconcileService {
+
+    ListenableFuture<RpcResult<ReconcileOutput>> reconcile(Set<Uint64> nodes);
+
+    ListenableFuture<RpcResult<ReconcileOutput>> reconcileAll();
+}
index de09ed3635939ef20efc45f95a9dcc62e790e991..7d4adbeae13fc0ebe158dc3bb5e535c60350af04 100644 (file)
@@ -11,17 +11,13 @@ import java.util.Formatter;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
 import java.util.stream.Collectors;
 import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
 import org.apache.karaf.shell.commands.Option;
 import org.apache.karaf.shell.console.OsgiCommandSupport;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconcileInput;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconcileInputBuilder;
+import org.opendaylight.openflowplugin.applications.southboundcli.ReconcileService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconcileOutput;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.openflowplugin.app.reconciliation.service.rev180227.ReconciliationService;
-import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.opendaylight.yangtools.yang.common.Uint64;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -30,9 +26,9 @@ import org.slf4j.LoggerFactory;
 public class Reconciliation extends OsgiCommandSupport {
     private static final Logger LOG = LoggerFactory.getLogger(Reconciliation.class);
 
-    private ReconciliationService reconciliationService = null;
+    private ReconcileService reconciliationService = null;
 
-    public void setReconciliationService(final ReconciliationService reconciliationService) {
+    public void setReconciliationService(final ReconcileService reconciliationService) {
         this.reconciliationService = reconciliationService;
     }
 
@@ -45,15 +41,13 @@ public class Reconciliation extends OsgiCommandSupport {
     @SuppressWarnings("checkstyle:RegexpSinglelineJava")
     @Override
     protected Object doExecute() throws Exception {
-        Set<Uint64> nodes = nodeIds == null
-                ? Set.of()
-                : nodeIds.stream().distinct().map(Uint64::valueOf).collect(Collectors.toUnmodifiableSet());
+        final var nodes = nodeIds == null ? Set.<Uint64>of()
+            : nodeIds.stream().map(Uint64::valueOf).collect(Collectors.toSet());
+        final var rpcOutput = reconcileAllNodes ? reconciliationService.reconcileAll()
+            : reconciliationService.reconcile(nodes);
         LOG.debug("Triggering reconciliation for nodes {}", nodes);
-        ReconcileInput rpcInput = new ReconcileInputBuilder().setNodes(nodes)
-                .setReconcileAllNodes(reconcileAllNodes).build();
-        Future<RpcResult<ReconcileOutput>> rpcOutput = reconciliationService.reconcile(rpcInput);
         try {
-            RpcResult<ReconcileOutput> rpcResult = rpcOutput.get();
+            final var rpcResult = rpcOutput.get();
             if (rpcResult.isSuccessful()) {
                 System.out.println("Reconciliation triggered for the node(s)");
                 printInProgressNodes(rpcResult.getResult());
@@ -68,23 +62,23 @@ public class Reconciliation extends OsgiCommandSupport {
 
     @SuppressWarnings("checkstyle:RegexpSinglelineJava")
     private static void printInProgressNodes(final ReconcileOutput reconcileOutput) {
-        Set<Uint64> inprogressNodes = reconcileOutput.getInprogressNodes();
+        final var inprogressNodes = reconcileOutput.getInprogressNodes();
         if (inprogressNodes.size() > 0) {
-            StringBuilder stringBuilder = new StringBuilder();
-            final Formatter formatter = new Formatter(stringBuilder);
-            System.out.println(getReconcileHeaderOutput());
-            System.out.println("----------------------------------------------------");
-            for (Uint64 node : inprogressNodes) {
-                System.out.println(formatter.format("%-15s %n",node).toString());
-                stringBuilder.setLength(0);
+            final var stringBuilder = new StringBuilder();
+            try (var formatter = new Formatter(stringBuilder)) {
+                System.out.println(getReconcileHeaderOutput());
+                System.out.println("----------------------------------------------------");
+                for (Uint64 node : inprogressNodes) {
+                    System.out.println(formatter.format("%-15s %n",node).toString());
+                    stringBuilder.setLength(0);
+                }
             }
         }
     }
 
     private static String getReconcileHeaderOutput() {
-        final Formatter formatter = new Formatter();
-        String header = formatter.format("%-15s %n", "Reconciliation already InProgress for below node(s)").toString();
-        formatter.close();
-        return header;
+        try (var formatter = new Formatter()) {
+            return formatter.format("%-15s %n", "Reconciliation already InProgress for below node(s)").toString();
+        }
     }
 }
index 331c6d500cb49fc9c9c711b91aa1aec3d2b05b26..ab5e08badd4c25c7799f806e57254ee5285bd74c 100644 (file)
@@ -1,7 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <blueprint xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0"
            xmlns:cm="http://aries.apache.org/blueprint/xmlns/blueprint-cm/v1.2.0">
-
+    <reference id="dataBroker"
+               interface="org.opendaylight.mdsal.binding.api.DataBroker"/>
     <reference id="reconciliationJMXServiceMBean"
                interface="org.opendaylight.openflowplugin.applications.frm.ReconciliationJMXServiceMBean"
                availability="optional"/>
                availability="optional"/>
     <reference id="flowGroupHistories"
                interface="org.opendaylight.openflowplugin.api.openflow.FlowGroupInfoHistories"/>
+    <reference id="reconciliationService"
+               interface="org.opendaylight.openflowplugin.applications.southboundcli.ReconcileService"/>
+    <reference id="dpnTracker"
+               interface="org.opendaylight.openflowplugin.applications.southboundcli.DpnTracker"/>
 
     <cm:property-placeholder persistent-id="org.ops4j.pax.web" update-strategy="none">
         <cm:default-properties>
@@ -51,5 +56,4 @@
             </action>
         </command>
     </command-bundle>
-
 </blueprint>
diff --git a/applications/southbound-cli/src/main/resources/OSGI-INF/blueprint/southbound-cli.xml b/applications/southbound-cli/src/main/resources/OSGI-INF/blueprint/southbound-cli.xml
deleted file mode 100644 (file)
index fa3b5bb..0000000
+++ /dev/null
@@ -1,24 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<blueprint xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0"
-           xmlns:cm="http://aries.apache.org/blueprint/xmlns/blueprint-cm/v1.1.0"
-           xmlns:odl="http://opendaylight.org/xmlns/blueprint/v1.0.0">
-    <reference id="dataBroker"
-               interface="org.opendaylight.mdsal.binding.api.DataBroker"/>
-    <reference id="flowCacheManager"
-               interface="org.opendaylight.openflowplugin.api.openflow.FlowGroupCacheManager"/>
-    <reference id="frm"
-               interface="org.opendaylight.openflowplugin.applications.frm.ForwardingRulesManager"/>
-    <reference id="dpnTracker"
-               interface="org.opendaylight.openflowplugin.applications.southboundcli.DpnTracker"/>
-
-    <bean id="reconciliationService"
-          class="org.opendaylight.openflowplugin.applications.southboundcli.ReconciliationServiceImpl"
-          destroy-method="close">
-        <argument ref="dataBroker"/>
-        <argument ref="frm"/>
-        <argument ref="dpnTracker"/>
-        <argument ref="flowCacheManager"/>
-    </bean>
-
-    <odl:rpc-implementation ref="reconciliationService"/>
-</blueprint>
index 335f4f918efc45219a0b7e3fda329b1f7cd9f716..720e703d2839c41e1911223cb689a3446824f673 100644 (file)
@@ -46,7 +46,6 @@ module reconciliation {
             leaf reconcile-all-nodes {
                 description "Flag to indicate that all nodes to be reconciled";
                 type boolean;
-                mandatory false;
                 default false;
             }
         }