BUG 2185 : Disable all internal switching of behavior

Since we do not depend on Raft for changing behavior when elections
are disabled we need to disable all internal switching of behaviors.

Added specific Leader tests to check the following,
1. Do not switch to Follower when you receive an AppendEntriesReply
   from a Follower with a higher term
2. Do not switch to IsolatedLeader even when no Follower is sending
   AppendEntriesReply

Change-Id: Ic2b4f76813f35db190e108306a62af5397d31658
Signed-off-by: Moiz Raja <moraja@cisco.com>
(cherry picked from commit 9baf6a2a5e494f70af407f04631980857a26daf9)

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
index 134cde8720517ed447fb5e3275347667a4068c7f..85f2f153ab3046d6541225c95d3863be3435b6b0 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractLeader.java
@@ -333,7 +333,7 @@ public abstract class AbstractLeader extends AbstractRaftActorBehavior {
 
                 context.getTermInformation().updateAndPersist(rpc.getTerm(), null);
 
-                return switchBehavior(new Follower(context));
+                return internalSwitchBehavior(RaftState.Follower);
             }
         }
 

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java
index 6ef4914477bee8fc2f7a50a78f66fdfd8be2f1d0..7e6175d9332277c2e828c84812d8d5de754f2f08 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/AbstractRaftActorBehavior.java
@@ -427,16 +427,27 @@ public abstract class AbstractRaftActorBehavior implements RaftActorBehavior {
 
     @Override
     public RaftActorBehavior switchBehavior(RaftActorBehavior behavior) {
-        LOG.info("{} :- Switching from behavior {} to {}", logName(), this.state(), behavior.state());
+        return internalSwitchBehavior(behavior);
+    }
+
+    protected RaftActorBehavior internalSwitchBehavior(RaftState newState) {
+        if(context.getRaftPolicy().automaticElectionsEnabled()){
+            return internalSwitchBehavior(newState.createBehavior(context));
+        }
+        return this;
+    }
+
+    private RaftActorBehavior internalSwitchBehavior(RaftActorBehavior newBehavior) {
+        LOG.info("{} :- Switching from behavior {} to {}", logName(), this.state(), newBehavior.state());
         try {
             close();
         } catch (Exception e) {
             LOG.error("{}: Failed to close behavior : {}", logName(), this.state(), e);
         }
-
-        return behavior;
+        return newBehavior;
     }
 
+
     protected int getMajorityVoteCount(int numPeers) {
         // Votes are required from a majority of the peers including self.
         // The numMajority field therefore stores a calculated value

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java
index 196456440b468794b21b60380934d705bfbf3234..a59a02051a7a8e5e5563429b098de55284829871 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Candidate.java
@@ -103,7 +103,7 @@ public class Candidate extends AbstractRaftActorBehavior {
         }
 
         if (voteCount >= votesRequired) {
-            return switchBehavior(new Leader(context));
+            return internalSwitchBehavior(RaftState.Leader);
         }
 
         return this;
@@ -129,7 +129,7 @@ public class Candidate extends AbstractRaftActorBehavior {
             if (rpc.getTerm() > context.getTermInformation().getCurrentTerm()) {
                 context.getTermInformation().updateAndPersist(rpc.getTerm(), null);
 
-                return switchBehavior(new Follower(context));
+                return internalSwitchBehavior(RaftState.Follower);
             }
         }
 
@@ -143,7 +143,7 @@ public class Candidate extends AbstractRaftActorBehavior {
                 // who we do not know about (as a peer)
                 // to send a message to the candidate
 
-                return switchBehavior(new Leader(context));
+                return internalSwitchBehavior(RaftState.Leader);
             }
             startNewTerm();
             scheduleElection(electionDuration());

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
index d4755e63586f564cd71d106529cef910074b8cba..0cd6fbab52c61343ef1ef93a6a49ffba7efa9b06 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Follower.java
@@ -313,7 +313,7 @@ public class Follower extends AbstractRaftActorBehavior {
 
         if (message instanceof ElectionTimeout) {
             LOG.debug("{}: Received ElectionTimeout - switching to Candidate", logName());
-            return switchBehavior(new Candidate(context));
+            return internalSwitchBehavior(RaftState.Candidate);
 
         } else if (message instanceof InstallSnapshot) {
             InstallSnapshot installSnapshot = (InstallSnapshot) message;

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java
index 4f77711a4d5af7dcf57f119f026e6c40e9516531..8f544dc32ff636c64f04dd88d59627f08f9f18d4 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/IsolatedLeader.java
@@ -40,7 +40,7 @@ public class IsolatedLeader extends AbstractLeader {
         // changes its state to Follower, hence we only need to switch to Leader if the state is still Isolated
         if (ret.state() == RaftState.IsolatedLeader && !isLeaderIsolated()) {
             LOG.info("IsolatedLeader {} switching from IsolatedLeader to Leader", leaderId);
-            return switchBehavior(new Leader(context));
+            return internalSwitchBehavior(RaftState.Leader);
         }
         return ret;
     }

diff --git a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java
index ebcdcd40fb078ebcc16439ec2feaa87b6f62eca4..1e58fbe541835916ad1f309b7fa87eb848bdd73a 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/main/java/org/opendaylight/controller/cluster/raft/behaviors/Leader.java
@@ -13,6 +13,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 import java.util.concurrent.TimeUnit;
 import org.opendaylight.controller.cluster.raft.RaftActorContext;
+import org.opendaylight.controller.cluster.raft.RaftState;
 import org.opendaylight.controller.cluster.raft.base.messages.IsolatedLeaderCheck;
 
 /**
@@ -54,7 +55,7 @@ public class Leader extends AbstractLeader {
                 LOG.warn("{}: At least {} followers need to be active, Switching {} from Leader to IsolatedLeader",
                         context.getId(), minIsolatedLeaderPeerCount, leaderId);
 
-                return switchBehavior(new IsolatedLeader(context));
+                return internalSwitchBehavior(RaftState.IsolatedLeader);
             }
         }
 

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java
index 5179ddc4816ebfa19ca26800a7b84e1444cafde6..9b877a7cdc2b0b3d804a9a31697e7a1f044f7f3e 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/behaviors/LeaderTest.java
@@ -51,6 +51,8 @@ import org.opendaylight.controller.cluster.raft.messages.InstallSnapshot;
 import org.opendaylight.controller.cluster.raft.messages.InstallSnapshotReply;
 import org.opendaylight.controller.cluster.raft.messages.RaftRPC;
 import org.opendaylight.controller.cluster.raft.messages.RequestVoteReply;
+import org.opendaylight.controller.cluster.raft.policy.DefaultRaftPolicy;
+import org.opendaylight.controller.cluster.raft.policy.RaftPolicy;
 import org.opendaylight.controller.cluster.raft.utils.ForwardMessageToBehaviorActor;
 import org.opendaylight.controller.cluster.raft.utils.MessageCollectorActor;
 import scala.concurrent.duration.FiniteDuration;
@@ -1424,6 +1426,53 @@ public class LeaderTest extends AbstractLeaderTest {
         assertEquals("Follower's lastTerm", 2, followerActorContext.getReplicatedLog().lastTerm());
     }
 
+    @Test
+    public void testHandleAppendEntriesReplyWithNewerTerm(){
+        logStart("testHandleAppendEntriesReplyWithNewerTerm");
+
+        MockRaftActorContext leaderActorContext = createActorContext();
+        ((DefaultConfigParamsImpl)leaderActorContext.getConfigParams()).setHeartBeatInterval(
+                new FiniteDuration(10000, TimeUnit.SECONDS));
+
+        leaderActorContext.setReplicatedLog(
+                new MockRaftActorContext.MockReplicatedLogBuilder().createEntries(0, 2, 2).build());
+
+        leader = new Leader(leaderActorContext);
+        leaderActor.underlyingActor().setBehavior(leader);
+        leaderActor.tell(new AppendEntriesReply("foo", 20, false, 1000, 10, (short) 1), ActorRef.noSender());
+
+        AppendEntriesReply appendEntriesReply = MessageCollectorActor.expectFirstMatching(leaderActor, AppendEntriesReply.class);
+
+        assertEquals(false, appendEntriesReply.isSuccess());
+        assertEquals(RaftState.Follower, leaderActor.underlyingActor().getFirstBehaviorChange().state());
+
+        MessageCollectorActor.clearMessages(leaderActor);
+    }
+
+    @Test
+    public void testHandleAppendEntriesReplyWithNewerTermWhenElectionsAreDisabled(){
+        logStart("testHandleAppendEntriesReplyWithNewerTermWhenElectionsAreDisabled");
+
+        MockRaftActorContext leaderActorContext = createActorContext();
+        ((DefaultConfigParamsImpl)leaderActorContext.getConfigParams()).setHeartBeatInterval(
+                new FiniteDuration(10000, TimeUnit.SECONDS));
+
+        leaderActorContext.setReplicatedLog(
+                new MockRaftActorContext.MockReplicatedLogBuilder().createEntries(0, 2, 2).build());
+        leaderActorContext.setRaftPolicy(createRaftPolicy(false, false));
+
+        leader = new Leader(leaderActorContext);
+        leaderActor.underlyingActor().setBehavior(leader);
+        leaderActor.tell(new AppendEntriesReply("foo", 20, false, 1000, 10, (short) 1), ActorRef.noSender());
+
+        AppendEntriesReply appendEntriesReply = MessageCollectorActor.expectFirstMatching(leaderActor, AppendEntriesReply.class);
+
+        assertEquals(false, appendEntriesReply.isSuccess());
+        assertEquals(RaftState.Leader, leaderActor.underlyingActor().getFirstBehaviorChange().state());
+
+        MessageCollectorActor.clearMessages(leaderActor);
+    }
+
     @Test
     public void testHandleAppendEntriesReplySuccess() throws Exception {
         logStart("testHandleAppendEntriesReplySuccess");
@@ -1597,56 +1646,69 @@ public class LeaderTest extends AbstractLeaderTest {
         Assert.assertTrue(behavior instanceof Leader);
     }
 
-    @Test
-    public void testIsolatedLeaderCheckTwoFollowers() throws Exception {
-        logStart("testIsolatedLeaderCheckTwoFollowers");
-
-        new JavaTestKit(getSystem()) {{
-
-            ActorRef followerActor1 = getTestActor();
-            ActorRef followerActor2 = getTestActor();
+    private RaftActorBehavior setupIsolatedLeaderCheckTestWithTwoFollowers(RaftPolicy raftPolicy){
+        ActorRef followerActor1 = getSystem().actorOf(MessageCollectorActor.props(), "follower-1");
+        ActorRef followerActor2 = getSystem().actorOf(MessageCollectorActor.props(), "follower-2");
 
-            MockRaftActorContext leaderActorContext = createActorContext();
+        MockRaftActorContext leaderActorContext = createActorContext();
 
-            Map<String, String> peerAddresses = new HashMap<>();
-            peerAddresses.put("follower-1", followerActor1.path().toString());
-            peerAddresses.put("follower-2", followerActor2.path().toString());
+        Map<String, String> peerAddresses = new HashMap<>();
+        peerAddresses.put("follower-1", followerActor1.path().toString());
+        peerAddresses.put("follower-2", followerActor2.path().toString());
 
-            leaderActorContext.setPeerAddresses(peerAddresses);
+        leaderActorContext.setPeerAddresses(peerAddresses);
+        leaderActorContext.setRaftPolicy(raftPolicy);
 
-            leader = new Leader(leaderActorContext);
+        leader = new Leader(leaderActorContext);
 
-            leader.markFollowerActive("follower-1");
-            leader.markFollowerActive("follower-2");
-            RaftActorBehavior behavior = leader.handleMessage(leaderActor, new IsolatedLeaderCheck());
-            Assert.assertTrue("Behavior not instance of Leader when all followers are active",
+        leader.markFollowerActive("follower-1");
+        leader.markFollowerActive("follower-2");
+        RaftActorBehavior behavior = leader.handleMessage(leaderActor, new IsolatedLeaderCheck());
+        Assert.assertTrue("Behavior not instance of Leader when all followers are active",
                 behavior instanceof Leader);
 
-            // kill 1 follower and verify if that got killed
-            final JavaTestKit probe = new JavaTestKit(getSystem());
-            probe.watch(followerActor1);
-            followerActor1.tell(PoisonPill.getInstance(), ActorRef.noSender());
-            final Terminated termMsg1 = probe.expectMsgClass(Terminated.class);
-            assertEquals(termMsg1.getActor(), followerActor1);
-
-            leader.markFollowerInActive("follower-1");
-            leader.markFollowerActive("follower-2");
-            behavior = leader.handleMessage(leaderActor, new IsolatedLeaderCheck());
-            Assert.assertTrue("Behavior not instance of Leader when majority of followers are active",
+        // kill 1 follower and verify if that got killed
+        final JavaTestKit probe = new JavaTestKit(getSystem());
+        probe.watch(followerActor1);
+        followerActor1.tell(PoisonPill.getInstance(), ActorRef.noSender());
+        final Terminated termMsg1 = probe.expectMsgClass(Terminated.class);
+        assertEquals(termMsg1.getActor(), followerActor1);
+
+        leader.markFollowerInActive("follower-1");
+        leader.markFollowerActive("follower-2");
+        behavior = leader.handleMessage(leaderActor, new IsolatedLeaderCheck());
+        Assert.assertTrue("Behavior not instance of Leader when majority of followers are active",
                 behavior instanceof Leader);
 
-            // kill 2nd follower and leader should change to Isolated leader
-            followerActor2.tell(PoisonPill.getInstance(), null);
-            probe.watch(followerActor2);
-            followerActor2.tell(PoisonPill.getInstance(), ActorRef.noSender());
-            final Terminated termMsg2 = probe.expectMsgClass(Terminated.class);
-            assertEquals(termMsg2.getActor(), followerActor2);
-
-            leader.markFollowerInActive("follower-2");
-            behavior = leader.handleMessage(leaderActor, new IsolatedLeaderCheck());
-            Assert.assertTrue("Behavior not instance of IsolatedLeader when majority followers are inactive",
-                behavior instanceof IsolatedLeader);
-        }};
+        // kill 2nd follower and leader should change to Isolated leader
+        followerActor2.tell(PoisonPill.getInstance(), null);
+        probe.watch(followerActor2);
+        followerActor2.tell(PoisonPill.getInstance(), ActorRef.noSender());
+        final Terminated termMsg2 = probe.expectMsgClass(Terminated.class);
+        assertEquals(termMsg2.getActor(), followerActor2);
+
+        leader.markFollowerInActive("follower-2");
+        return leader.handleMessage(leaderActor, new IsolatedLeaderCheck());
+    }
+
+    @Test
+    public void testIsolatedLeaderCheckTwoFollowers() throws Exception {
+        logStart("testIsolatedLeaderCheckTwoFollowers");
+
+        RaftActorBehavior behavior = setupIsolatedLeaderCheckTestWithTwoFollowers(DefaultRaftPolicy.INSTANCE);
+
+        Assert.assertTrue("Behavior not instance of IsolatedLeader when majority followers are inactive",
+            behavior instanceof IsolatedLeader);
+    }
+
+    @Test
+    public void testIsolatedLeaderCheckTwoFollowersWhenElectionsAreDisabled() throws Exception {
+        logStart("testIsolatedLeaderCheckTwoFollowersWhenElectionsAreDisabled");
+
+        RaftActorBehavior behavior = setupIsolatedLeaderCheckTestWithTwoFollowers(createRaftPolicy(false, true));
+
+        Assert.assertTrue("Behavior should not switch to IsolatedLeader because elections are disabled",
+                behavior instanceof Leader);
     }
 
     @Test

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/utils/ForwardMessageToBehaviorActor.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/utils/ForwardMessageToBehaviorActor.java
index 9bcfcd9091281db235975bfbad3a9f0e04129c82..63810d8882a066dffc8d4d86fa5551574ebb555c 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/utils/ForwardMessageToBehaviorActor.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/utils/ForwardMessageToBehaviorActor.java
@@ -8,18 +8,21 @@
 
 package org.opendaylight.controller.cluster.raft.utils;
 
+import static org.junit.Assert.assertTrue;
 import akka.actor.Props;
+import java.util.ArrayList;
+import java.util.List;
 import org.opendaylight.controller.cluster.raft.behaviors.RaftActorBehavior;
 
 public class ForwardMessageToBehaviorActor extends MessageCollectorActor {
     private RaftActorBehavior behavior;
+    private List<RaftActorBehavior> behaviorChanges = new ArrayList<>();
 
     @Override
     public void onReceive(Object message) throws Exception {
         if(behavior != null) {
-            behavior.handleMessage(sender(), message);
+            behaviorChanges.add(behavior.handleMessage(sender(), message));
         }
-
         super.onReceive(message);
     }
 
@@ -30,5 +33,25 @@ public class ForwardMessageToBehaviorActor extends MessageCollectorActor {
     public void setBehavior(RaftActorBehavior behavior){
         this.behavior = behavior;
     }
+
+    public RaftActorBehavior getFirstBehaviorChange() {
+        assertTrue("no behavior changes present", behaviorChanges.size() > 0);
+        return behaviorChanges.get(0);
+    }
+
+    public RaftActorBehavior getLastBehaviorChange() {
+        assertTrue("no behavior changes present", behaviorChanges.size() > 0);
+        return behaviorChanges.get(behaviorChanges.size() - 1);
+    }
+
+    public List<RaftActorBehavior> getBehaviorChanges(){
+        return behaviorChanges;
+    }
+
+    @Override
+    public void clear() {
+        super.clear();
+        behaviorChanges.clear();
+    }
 }
 

diff --git a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/utils/MessageCollectorActor.java b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/utils/MessageCollectorActor.java
index b0773bf7ca1ddbe92b6749ca7ec94d7b5e3bb9e1..2c5b6cc34be95cd3bce5eb792c1499b51c5fa578 100644 (file)
--- a/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/utils/MessageCollectorActor.java
+++ b/opendaylight/md-sal/sal-akka-raft/src/test/java/org/opendaylight/controller/cluster/raft/utils/MessageCollectorActor.java
@@ -45,7 +45,7 @@ public class MessageCollectorActor extends UntypedActor {
         if(GET_ALL_MESSAGES.equals(message)) {
             getSender().tell(new ArrayList<>(messages), getSelf());
         } else if(CLEAR_MESSAGES.equals(message)) {
-            messages.clear();
+            clear();
         } else if(message != null) {
             messages.add(SerializationUtils.fromSerializable(message));
         }