Fetch reboot env var at run time and keep constant values constant

Keep the default duration as a constant and fetch the custom environment
variable at run time with a fallback to the default. Set the sleep duration
into a constant and add tests covering the expected behaviors when the
default duration is in use as well as the override value. Also match the
environment variable up with the constant just for consistency.

plugins/guests/linux/cap/reboot.rb

@@ -6,7 +6,9 @@ module VagrantPlugins
     module Cap
       class Reboot
         extend Vagrant::Util::GuestInspection::Linux
-        MAX_REBOOT_RETRY_DURATION = ENV.fetch('VAGRANT_MAX_REBOOT_RETRY_TIMEOUT', 120).to_i
+
+        DEFAULT_MAX_REBOOT_RETRY_DURATION = 120
+        WAIT_SLEEP_TIME = 5
 
         def self.reboot(machine)
           @logger = Log4r::Logger.new("vagrant::linux::reboot")
@@ -25,14 +27,17 @@ module VagrantPlugins
 
           @logger.debug("Waiting for machine to finish rebooting")
 
-          wait_remaining = MAX_REBOOT_RETRY_DURATION
+          wait_remaining = ENV.fetch("VAGRANT_MAX_REBOOT_RETRY_DURATION",
+            DEFAULT_MAX_REBOOT_RETRY_DURATION).to_i
+          wait_remaining = DEFAULT_MAX_REBOOT_RETRY_DURATION if wait_remaining < 1
+
           begin
             wait_for_reboot(machine)
-          rescue Vagrant::Errors::MachineGuestNotReady => e
+          rescue Vagrant::Errors::MachineGuestNotReady
             raise if wait_remaining < 0
             @logger.warn("Machine not ready, cannot start reboot yet. Trying again")
-            sleep(5)
-            wait_remaining -= 5
+            sleep(WAIT_SLEEP_TIME)
+            wait_remaining -= WAIT_SLEEP_TIME
             retry
           end
         end

plugins/guests/windows/cap/reboot.rb

@@ -4,7 +4,8 @@ module VagrantPlugins
   module GuestWindows
     module Cap
       class Reboot
-        MAX_REBOOT_RETRY_DURATION = ENV.fetch('VAGRANT_MAX_REBOOT_RETRY_TIMEOUT', 120).to_i
+        DEFAULT_MAX_REBOOT_RETRY_DURATION = 120
+        WAIT_SLEEP_TIME = 5
 
         def self.reboot(machine)
           @logger = Log4r::Logger.new("vagrant::windows::reboot")
@@ -25,15 +26,18 @@ module VagrantPlugins
 
           @logger.debug("Waiting for machine to finish rebooting")
 
-          wait_remaining = MAX_REBOOT_RETRY_DURATION
+          wait_remaining = ENV.fetch("VAGRANT_MAX_REBOOT_RETRY_DURATION",
+            DEFAULT_MAX_REBOOT_RETRY_DURATION).to_i
+          wait_remaining = DEFAULT_MAX_REBOOT_RETRY_DURATION if wait_remaining < 1
+
           begin
             wait_for_reboot(machine)
           rescue => err
             raise if wait_remaining < 0
             @logger.debug("Exception caught while waiting for reboot: #{err}")
             @logger.warn("Machine not ready, cannot start reboot yet. Trying again")
-            sleep(5)
-            wait_remaining -= 5
+            sleep(WAIT_SLEEP_TIME)
+            wait_remaining -= WAIT_SLEEP_TIME
             retry
           end
         end

test/unit/plugins/guests/linux/cap/reboot_test.rb

@@ -64,15 +64,13 @@ describe "VagrantPlugins::GuestLinux::Cap::Reboot" do
         communicator.verify_expectations!
       end
 
-      describe ".reboot" do
-        it "reboots the vm" do
-          allow(communicator).to receive(:execute)
+      it "reboots the vm" do
+        allow(communicator).to receive(:execute)
 
-          expect(communicator).to receive(:execute).with(/systemctl reboot/, nil).and_return(0)
-          expect(described_class).to receive(:wait_for_reboot)
+        expect(communicator).to receive(:execute).with(/systemctl reboot/, nil).and_return(0)
+        expect(described_class).to receive(:wait_for_reboot)
 
-          described_class.reboot(machine)
-        end
+        described_class.reboot(machine)
       end
     end
 
@@ -80,13 +78,28 @@ describe "VagrantPlugins::GuestLinux::Cap::Reboot" do
       before do
         allow(communicator).to receive(:execute)
         expect(communicator).to receive(:execute).with(/reboot/, nil).and_return(0)
-        allow(described_class).to receive(:sleep)
+        allow(described_class).to receive(:sleep).and_return(described_class::WAIT_SLEEP_TIME)
         allow(described_class).to receive(:wait_for_reboot).and_raise(Vagrant::Errors::MachineGuestNotReady)
       end
 
-      describe ".reboot default" do
-        it "allows setting a custom max reboot retry duration" do
-          max_retries = 26 # initial call + 25 retries since multiple of 5
+      context "default retry duration value" do
+        let(:max_retries) { (described_class::DEFAULT_MAX_REBOOT_RETRY_DURATION / described_class::WAIT_SLEEP_TIME) + 2 }
+
+        it "should receive expected number of wait_for_reboot calls" do
+          expect(described_class).to receive(:wait_for_reboot).exactly(max_retries).times
+          expect { described_class.reboot(machine) }.to raise_error(Vagrant::Errors::MachineGuestNotReady)
+        end
+      end
+
+      context "with custom retry duration value" do
+        let(:duration) { 10 }
+        let(:max_retries) { (duration / described_class::WAIT_SLEEP_TIME) + 2 }
+
+        before do
+          expect(ENV).to receive(:fetch).with("VAGRANT_MAX_REBOOT_RETRY_DURATION", anything).and_return(duration)
+        end
+
+        it "should receive expected number of wait_for_reboot calls" do
           expect(described_class).to receive(:wait_for_reboot).exactly(max_retries).times
           expect { described_class.reboot(machine) }.to raise_error(Vagrant::Errors::MachineGuestNotReady)
         end

test/unit/plugins/guests/windows/cap/reboot_test.rb

@@ -122,11 +122,26 @@ describe "VagrantPlugins::GuestWindows::Cap::Reboot" do
       allow(described_class).to receive(:wait_for_reboot).and_raise(StandardError)
     end
 
-    describe ".reboot default" do
-      it "allows setting a custom max reboot retry duration" do
-        max_retries = 26 # initial call + 25 retries since multiple of 5
+    context "default retry duration value" do
+      let(:max_retries) { (described_class::DEFAULT_MAX_REBOOT_RETRY_DURATION / described_class::WAIT_SLEEP_TIME) + 2 }
+
+      it "should receive expected number of wait_for_reboot calls" do
         expect(described_class).to receive(:wait_for_reboot).exactly(max_retries).times
-        expect { described_class.reboot(machine) }.to raise_error(Exception)
+        expect { described_class.reboot(machine) }.to raise_error(StandardError)
+      end
+    end
+
+    context "with custom retry duration value" do
+      let(:duration) { 10 }
+      let(:max_retries) { (duration / described_class::WAIT_SLEEP_TIME) + 2 }
+
+      before do
+        expect(ENV).to receive(:fetch).with("VAGRANT_MAX_REBOOT_RETRY_DURATION", anything).and_return(duration)
+      end
+
+      it "should receive expected number of wait_for_reboot calls" do
+        expect(described_class).to receive(:wait_for_reboot).exactly(max_retries).times
+        expect { described_class.reboot(machine) }.to raise_error(StandardError)
       end
     end
   end

website/pages/docs/other/environmental-variables.mdx

@@ -225,7 +225,7 @@ and can help identify certain issues.
 some knowledge of Vagrant internals. It is the best output to attach to
 a support request or bug report, however.
 
-## `VAGRANT_MAX_REBOOT_RETRY_TIMEOUT`
+## `VAGRANT_MAX_REBOOT_RETRY_DURATION`
 
 By default, Vagrant will wait up to 120 seconds for a machine to reboot.
 However, if you're finding your OS is taking longer than 120 seconds to