Do not propagate PCEPSession in onSessionUp

We have only a single implementation and that implementation is just
filling the builder. Make sure do not allow it to do anything with
the incoming session -- allowing us to better reason about state
transitions here.

JIRA: BGPCEP-1005
Change-Id: I784120c4232036dc846e2e3c60dedffe13731b3a
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
index d076894c97a91a0045da2aaec487dc2d9da9e3fd..f16b79d331005f1ad36ef67bc6c7782abd57fc20 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
@@ -33,6 +33,7 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.checkerframework.checker.lock.qual.Holding;
+import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.bgpcep.pcep.topology.provider.session.stats.SessionStateImpl;
 import org.opendaylight.bgpcep.pcep.topology.provider.session.stats.TopologySessionStats;
 import org.opendaylight.mdsal.binding.api.WriteTransaction;
@@ -50,6 +51,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.typ
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev181109.MessageHeader;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev181109.Object;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev181109.ProtocolVersion;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.types.rev181109.open.object.open.Tlvs;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev200120.LspId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev200120.Node1;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev200120.Node1Builder;
@@ -151,8 +153,7 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
                 }
 
                 if (this.session != null || this.nodeState != null) {
-                    LOG.error("PCEP session is already up with {}. Closing session {}", psession.getRemoteAddress(),
-                            psession);
+                    LOG.error("PCEP session is already up with {}. Closing session {}", peerAddress, psession);
                     psession.close(TerminationReason.UNKNOWN);
                     this.onSessionTerminated(psession, new PCEPCloseTermination(TerminationReason.UNKNOWN));
                     return;
@@ -163,12 +164,14 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
                 LOG.trace("Peer {} resolved to topology node {}", peerAddress, state.getNodeId());
 
                 // Our augmentation in the topology node
-                final PathComputationClientBuilder pccBuilder = new PathComputationClientBuilder();
+                final PathComputationClientBuilder pccBuilder = new PathComputationClientBuilder()
+                    .setIpAddress(IetfInetUtil.INSTANCE.ipAddressNoZoneFor(peerAddress));
+
+                // Let subclass fill the details
+                onSessionUp(pccBuilder, peerAddress, psession.getRemoteTlvs());
 
-                onSessionUp(psession, pccBuilder);
                 this.synced.set(isSynchronized());
 
-                pccBuilder.setIpAddress(IetfInetUtil.INSTANCE.ipAddressNoZoneFor(peerAddress));
                 final InstanceIdentifier<Node1> topologyAugment = state.getNodeId().augmentation(Node1.class);
                 this.pccIdentifier = topologyAugment.child(PathComputationClient.class);
 
@@ -185,13 +188,13 @@ public abstract class AbstractTopologySessionListener<S, L> implements TopologyS
 
                 this.listenerState = new SessionStateImpl(this, psession);
                 this.serverSessionManager.bind(state.getNodeId(), this.listenerState);
-                LOG.info("Session with {} attached to topology node {}", psession.getRemoteAddress(),
-                        state.getNodeId());
+                LOG.info("Session with {} attached to topology node {}", peerAddress, state.getNodeId());
             }
         }
     }
 
-    protected abstract void onSessionUp(PCEPSession session, PathComputationClientBuilder pccBuilder);
+    protected abstract void onSessionUp(PathComputationClientBuilder pccBuilder, InetAddress peerAddress,
+            @Nullable Tlvs remoteTlvs);
 
     synchronized void updatePccState(final PccSyncState pccSyncState) {
         if (this.nodeState == null) {

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologySessionListener.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologySessionListener.java
index 2f8d6d262930d8b78eadaa1ddedad0db29ef34d3..0eacf16fc9b92df5d4dd66ab38ed6f7d9f5ea3e2 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologySessionListener.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/PCEPTopologySessionListener.java
@@ -21,7 +21,6 @@ import java.net.InetAddress;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -32,7 +31,6 @@ import org.checkerframework.checker.lock.qual.Holding;
 import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.bgpcep.pcep.server.PathComputation;
 import org.opendaylight.bgpcep.pcep.server.PceServerProvider;
-import org.opendaylight.protocol.pcep.PCEPSession;
 import org.opendaylight.protocol.pcep.spi.PCEPErrors;
 import org.opendaylight.protocol.pcep.spi.PSTUtil;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.pcep.sync.optimizations.rev200720.PathComputationClient1Builder;
@@ -147,32 +145,32 @@ class PCEPTopologySessionListener extends AbstractTopologySessionListener<SrpIdN
     }
 
     @Override
-    protected void onSessionUp(final PCEPSession session, final PathComputationClientBuilder pccBuilder) {
-        final InetAddress peerAddress = session.getRemoteAddress();
-
-        final Tlvs tlvs = session.getRemoteTlvs();
-        if (tlvs != null && tlvs.augmentation(Tlvs1.class) != null) {
-            final Stateful stateful = tlvs.augmentation(Tlvs1.class).getStateful();
-            if (stateful != null) {
-                setStatefulCapabilities(stateful);
-                pccBuilder.setReportedLsp(Collections.emptyMap());
-                if (isSynchronized()) {
-                    pccBuilder.setStateSync(PccSyncState.Synchronized);
-                } else if (isTriggeredInitialSynchro()) {
-                    pccBuilder.setStateSync(PccSyncState.TriggeredInitialSync);
-                } else if (isIncrementalSynchro()) {
-                    pccBuilder.setStateSync(PccSyncState.IncrementalSync);
-                } else {
-                    pccBuilder.setStateSync(PccSyncState.InitialResync);
+    protected void onSessionUp(final PathComputationClientBuilder pccBuilder, final InetAddress peerAddress,
+            final Tlvs remoteTlvs) {
+        if (remoteTlvs != null) {
+            final Tlvs1 statefulTlvs = remoteTlvs.augmentation(Tlvs1.class);
+            if (statefulTlvs != null) {
+                final Stateful stateful = statefulTlvs.getStateful();
+                if (stateful != null) {
+                    setStatefulCapabilities(stateful);
+                    pccBuilder.setReportedLsp(Map.of());
+                    if (isSynchronized()) {
+                        pccBuilder.setStateSync(PccSyncState.Synchronized);
+                    } else if (isTriggeredInitialSynchro()) {
+                        pccBuilder.setStateSync(PccSyncState.TriggeredInitialSync);
+                    } else if (isIncrementalSynchro()) {
+                        pccBuilder.setStateSync(PccSyncState.IncrementalSync);
+                    } else {
+                        pccBuilder.setStateSync(PccSyncState.InitialResync);
+                    }
+                    pccBuilder.setStatefulTlv(new StatefulTlvBuilder()
+                        .addAugmentation(new StatefulTlv1Builder(statefulTlvs).build())
+                        .build());
+                    return;
                 }
-                pccBuilder.setStatefulTlv(new StatefulTlvBuilder().addAugmentation(
-                        new StatefulTlv1Builder(tlvs.augmentation(Tlvs1.class)).build()).build());
-            } else {
-                LOG.debug("Peer {} does not advertise stateful TLV", peerAddress);
             }
-        } else {
-            LOG.debug("Peer {} does not advertise stateful TLV", peerAddress);
         }
+        LOG.debug("Peer {} does not advertise stateful TLV", peerAddress);
     }
 
     @Override
@@ -232,7 +230,7 @@ class PCEPTopologySessionListener extends AbstractTopologySessionListener<SrpIdN
         updBuilder.setPath(pb.build());
         updBuilder.setLsp(lsp).setSrp(srp).setPath(pb.build());
 
-        pcupdMessageBuilder.setUpdates(Collections.singletonList(updBuilder.build()));
+        pcupdMessageBuilder.setUpdates(List.of(updBuilder.build()));
         return srp.getOperationId();
     }
 
@@ -475,7 +473,7 @@ class PCEPTopologySessionListener extends AbstractTopologySessionListener<SrpIdN
                     }
                     final PcinitiateMessageBuilder ib = new PcinitiateMessageBuilder(MESSAGE_HEADER);
                     final Requests rb = buildRequest(rep, reportedLsp);
-                    ib.setRequests(Collections.singletonList(rb));
+                    ib.setRequests(List.of(rb));
                     return sendMessage(new PcinitiateBuilder().setPcinitiateMessage(ib.build()).build(),
                         rb.getSrp().getOperationId(), null);
                 }, MoreExecutors.directExecutor());
@@ -511,7 +509,7 @@ class PCEPTopologySessionListener extends AbstractTopologySessionListener<SrpIdN
             pb.fieldsFrom(input.getArguments());
             rb.setPath(pb.build());
             final PcupdMessageBuilder ub = new PcupdMessageBuilder(MESSAGE_HEADER);
-            ub.setUpdates(Collections.singletonList(rb.build()));
+            ub.setUpdates(List.of(rb.build()));
             msg = new PcupdBuilder().setPcupdMessage(ub.build()).build();
         } else {
             final Lsp1 lspCreateFlag = reportedLsp.augmentation(Lsp1.class);
@@ -741,7 +739,7 @@ class PCEPTopologySessionListener extends AbstractTopologySessionListener<SrpIdN
             final PathBuilder pb = new PathBuilder();
             rb.setPath(pb.build());
             final PcupdMessageBuilder ub = new PcupdMessageBuilder(MESSAGE_HEADER);
-            ub.setUpdates(Collections.singletonList(rb.build()));
+            ub.setUpdates(List.of(rb.build()));
             return new PcupdBuilder().setPcupdMessage(ub.build()).build();
         }
     }
@@ -829,7 +827,7 @@ class PCEPTopologySessionListener extends AbstractTopologySessionListener<SrpIdN
             // Send the message
             return sendMessage(new PcinitiateBuilder()
                 .setPcinitiateMessage(new PcinitiateMessageBuilder(MESSAGE_HEADER)
-                    .setRequests(Collections.singletonList(rb.build()))
+                    .setRequests(List.of(rb.build()))
                     .build())
                 .build(),
                 rb.getSrp().getOperationId(), input.getArguments().getMetadata());
@@ -888,7 +886,7 @@ class PCEPTopologySessionListener extends AbstractTopologySessionListener<SrpIdN
                 .setPcerrMessage(new PcerrMessageBuilder()
                     .setErrorType(new RequestCaseBuilder()
                         .setRequest(new RequestBuilder()
-                            .setRps(Collections.singletonList(new RpsBuilder()
+                            .setRps(List.of(new RpsBuilder()
                                 .setRp(new RpBuilder()
                                     .setProcessingRule(false)
                                     .setIgnore(false)
@@ -897,7 +895,7 @@ class PCEPTopologySessionListener extends AbstractTopologySessionListener<SrpIdN
                                 .build()))
                             .build())
                         .build())
-                    .setErrors(Collections.singletonList(new ErrorsBuilder()
+                    .setErrors(List.of(new ErrorsBuilder()
                         .setErrorObject(new ErrorObjectBuilder()
                             .setType(pcepErrors.getErrorType())
                             .setValue(pcepErrors.getErrorValue())