improvements to initial member handling

components/butterfly/src/server.rs

@@ -310,6 +310,8 @@ pub struct Server {
     gossip_rounds:            Arc<AtomicIsize>,
     block_list:               Arc<Lock<HashSet<String>>>,
     election_timers:          Arc<Mutex<HashMap<String, ElectionTimer>>>,
+    /// Flag to indicate that new initial members have been added and should be pinged
+    new_initial_members:      Arc<AtomicBool>,
 }
 
 impl Clone for Server {
@@ -338,7 +340,8 @@ impl Clone for Server {
                  gossip_rounds:        self.gossip_rounds.clone(),
                  block_list:           self.block_list.clone(),
                  socket:               None,
-                 election_timers:      self.election_timers.clone(), }
+                 election_timers:      self.election_timers.clone(),
+                 new_initial_members:  self.new_initial_members.clone(), }
     }
 }
 
@@ -401,7 +404,8 @@ impl Server {
                             gossip_rounds: Arc::new(AtomicIsize::new(0)),
                             block_list: Arc::new(Lock::new(HashSet::new())),
                             socket: None,
-                            election_timers: Arc::new(Mutex::new(HashMap::new())) })
+                            election_timers: Arc::new(Mutex::new(HashMap::new())),
+                            new_initial_members: Arc::new(AtomicBool::new(false)) })
             }
             (Err(e), _) | (_, Err(e)) => Err(Error::CannotBind(e)),
             (Ok(None), _) | (_, Ok(None)) => {
@@ -545,11 +549,6 @@ impl Server {
         Ok(())
     }
 
-    /// # Locking (see locking.md)
-    /// * `MemberList::entries` (read) Additionally `with_closure` is called with this lock held, so
-    ///   the closure must not call any functions which take this lock.
-    pub fn need_peer_seeding_mlr(&self) -> bool { self.member_list.is_empty_mlr() }
-
     /// Persistently block a given address, causing no traffic to be seen.
     ///
     /// # Locking (see locking.md)
@@ -1359,6 +1358,13 @@ impl Server {
         }
     }
 
+    /// Signal that new initial members have been added and should be pinged on the next outbound
+    /// cycle
+    pub fn signal_new_initial_members(&self) {
+        self.new_initial_members.store(true, Ordering::Release);
+        debug!("Signaled outbound thread that new initial members were added");
+    }
+
     #[allow(dead_code)]
     pub fn is_departed(&self) -> bool { self.departed.load(Ordering::Relaxed) }
 }

components/butterfly/src/server/outbound.rs

@@ -33,7 +33,8 @@ use std::{collections::HashSet,
           iter::FromIterator,
           net::{SocketAddr,
                 UdpSocket},
-          sync::mpsc,
+          sync::{atomic::Ordering,
+                 mpsc},
           thread,
           time::{Duration,
                  Instant}};
@@ -94,27 +95,63 @@ pub fn spawn_thread(name: String,
 /// before starting the next probe.
 fn run_loop(server: &Server, socket: &UdpSocket, rx_inbound: &AckReceiver, timing: &Timing) -> ! {
     let mut have_members = false;
+
     loop {
         liveliness_checker::mark_thread_alive().and_divergent();
 
-        if !have_members {
+        // Check if new initial members have been added via peer watch file
+        let new_initial_members_added = server.new_initial_members.load(Ordering::Acquire);
+        if new_initial_members_added {
+            // Reset the flag and force re-evaluation of initial member pinging
+            server.new_initial_members.store(false, Ordering::Relaxed);
+            debug!("New initial members detected, re-evaluating initial member ping logic");
+            // We don't set have_members = false here because we want to respect the existing
+            // min_to_start threshold logic, but we will allow the initial member ping to run again
+        }
+
+        if !have_members || new_initial_members_added {
             let num_initial = server.member_list.len_initial_members_imlr();
             if num_initial != 0 {
                 // The minimum that's strictly more than half
                 #[allow(clippy::integer_division)]
                 let min_to_start = num_initial / 2 + 1;
 
-                if server.member_list.len_mlr() >= min_to_start {
-                    have_members = true;
-                } else {
-                    server.member_list.with_initial_members_imlr(|member| {
+                // Only ping initial members that are not already marked as alive
+                server.member_list.with_initial_members_imlr(|member| {
+                                      // Check if this member is already alive in the member list
+                                      let member_health = server.member_list.health_of_mlr(member);
+                                      let should_ping = match member_health {
+                                          Some(Health::Alive) => {
+                                              trace!("Skipping ping to initial member {} - \
+                                                      already marked as Alive",
+                                                     member.id);
+                                              false
+                                          }
+                                          _ => {
+                                              trace!("Pinging initial member {} - health status: \
+                                                      {:?}",
+                                                     member.id, member_health);
+                                              true
+                                          }
+                                      };
+
+                                      if should_ping {
                                           ping_mlr_smr_rhw(server,
                                                            socket,
                                                            member,
                                                            member.swim_socket_address(),
                                                            None,
                                                            false);
-                                      });
+                                      }
+                                  });
+
+                // Only set have_members = true if we're not already in steady state
+                // and we've reached the threshold
+                if !have_members && server.member_list.len_mlr() >= min_to_start {
+                    debug!("Reached min_to_start threshold ({} members), stopping initial member \
+                            pings",
+                           min_to_start);
+                    have_members = true;
                 }
             }
         }

components/sup/src/manager.rs

@@ -2035,15 +2035,14 @@ impl Manager {
     /// * `MemberList::entries` (read)
     /// * `MemberList::initial_members` (write)
     fn update_peers_from_watch_file_mlr_imlw(&mut self) -> Result<()> {
-        if !self.butterfly.need_peer_seeding_mlr() {
-            return Ok(());
-        }
         match self.peer_watcher {
             None => Ok(()),
             Some(ref watcher) => {
                 if watcher.has_fs_events() {
                     let members = watcher.get_members()?;
                     self.butterfly.member_list.set_initial_members_imlw(members);
+                    // Signal the outbound thread to re-evaluate initial member pinging
+                    self.butterfly.signal_new_initial_members();
                 }
                 Ok(())
             }

test/end-to-end/multi-supervisor/testcases/peer_watcher/docker-compose.override.yml

@@ -16,9 +16,6 @@ services:
       - run
       - --listen-ctl=0.0.0.0:9632
       - --peer-watch-file=/hab/PEERS
-    volumes:
-      - ./testcases/peer_watcher/PEERS:/hab/PEERS
-
 
   tester:
     image: ${TESTCASE}_image
@@ -30,6 +27,9 @@ services:
       COMPOSE_PROJECT_NAME: ${COMPOSE_PROJECT_NAME}
       HAB_AUTH_TOKEN: ${HAB_AUTH_TOKEN}
     volumes:
+      - source: ./testcases/peer_watcher/PEERS
+        target: /hab/PEERS
+        type: bind
       - source: ./CTL_SECRET
         target: /hab/sup/default/CTL_SECRET
         type: bind

test/end-to-end/multi-supervisor/testcases/peer_watcher/run_test.ps1

@@ -4,6 +4,13 @@ Describe "Finding peers from watch file" {
         $timeoutScript = { Write-Error "Timed out waiting 45 seconds for all members to reach bastion" }
         Wait-True -TestScript $testScript -TimeoutScript $timeoutScript -Timeout 45
         $json = (Invoke-WebRequest "http://bastion.habitat.dev:9631/census" | ConvertFrom-Json)
+        $json.last_membership_counter | Should -Be 2
+    }
+
+    It "adds beta to the peer watch file and finds it as a peer" {
+        Add-Content -Path "/hab/PEERS" -Value "beta.habitat.dev"
+        Start-Sleep -Seconds 5 # give butterfly some time to detect the change and update the census
+        $json = (Invoke-WebRequest "http://bastion.habitat.dev:9631/census" | ConvertFrom-Json)
         $json.last_membership_counter | Should -Be 3
     }
-}
+}