From c999e7c3d43411ad4690d2c61c0e3adc3b6ee62f Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 7 Dec 2018 16:17:44 -0800 Subject: [PATCH 1/2] Update behavior of `snapshot restore` and `snapshot pop` Both of these commands failed to default the options disabling the provisioning from ignoring the sentinel file. This resulted in different behavior than what was seen with the `up` and `resume` commands which would only provision items with run set to "always". This defaults the options to proper match the behavior of `up` and `resume` to be consistent. This also adds an extra `--no-start` flag to allow users to restore a snapshot but not start the restored guest immediately. Fixes #6752 --- plugins/commands/snapshot/command/pop.rb | 6 ++++ plugins/commands/snapshot/command/restore.rb | 6 ++++ plugins/providers/virtualbox/action.rb | 6 +++- .../commands/snapshot/command/pop_test.rb | 34 +++++++++++++++++++ .../commands/snapshot/command/restore_test.rb | 33 ++++++++++++++++++ 5 files changed, 84 insertions(+), 1 deletion(-) diff --git a/plugins/commands/snapshot/command/pop.rb b/plugins/commands/snapshot/command/pop.rb index 3f11dc24a..cba57ef17 100644 --- a/plugins/commands/snapshot/command/pop.rb +++ b/plugins/commands/snapshot/command/pop.rb @@ -17,6 +17,9 @@ module VagrantPlugins def execute options = {} options[:snapshot_delete] = true + options[:provision_ignore_sentinel] = false + options[:snapshot_start] = true + opts = OptionParser.new do |o| o.banner = "Usage: vagrant snapshot pop [options] [vm-name]" o.separator "" @@ -26,6 +29,9 @@ module VagrantPlugins o.on("--no-delete", "Don't delete the snapshot after the restore") do options[:snapshot_delete] = false end + o.on("--no-start", "Don't start the snapshot after the restore") do + options[:snapshot_start] = false + end end # Parse the options diff --git a/plugins/commands/snapshot/command/restore.rb b/plugins/commands/snapshot/command/restore.rb index 1d8df2ca3..0788212bf 100644 --- a/plugins/commands/snapshot/command/restore.rb +++ b/plugins/commands/snapshot/command/restore.rb @@ -13,12 +13,18 @@ module VagrantPlugins def execute options = {} + options[:provision_ignore_sentinel] = false + options[:snapshot_start] = true opts = OptionParser.new do |o| o.banner = "Usage: vagrant snapshot restore [options] [vm-name] " o.separator "" build_start_options(o, options) o.separator "Restore a snapshot taken previously with snapshot save." + + o.on("--no-start", "Don't start the snapshot after the restore") do + options[:snapshot_start] = false + end end # Parse the options diff --git a/plugins/providers/virtualbox/action.rb b/plugins/providers/virtualbox/action.rb index 113be2b03..bb4e677af 100644 --- a/plugins/providers/virtualbox/action.rb +++ b/plugins/providers/virtualbox/action.rb @@ -267,7 +267,11 @@ module VagrantPlugins end end - b2.use action_start + b2.use Call, IsEnvSet, :snapshot_start do |env2, b3| + if env2[:result] + b3.use action_start + end + end end end end diff --git a/test/unit/plugins/commands/snapshot/command/pop_test.rb b/test/unit/plugins/commands/snapshot/command/pop_test.rb index 66687868f..1ec6fd635 100644 --- a/test/unit/plugins/commands/snapshot/command/pop_test.rb +++ b/test/unit/plugins/commands/snapshot/command/pop_test.rb @@ -25,6 +25,12 @@ describe VagrantPlugins::CommandSnapshot::Command::Pop do end describe "execute" do + before do + machine.id = "mach" + allow(machine.provider).to receive(:capability). + with(:snapshot_list).and_return(["push_2_0"]) + end + it "calls snapshot_restore with the last pushed snapshot" do machine.id = "foo" @@ -48,5 +54,33 @@ describe VagrantPlugins::CommandSnapshot::Command::Pop do expect(machine).to_not receive(:action) expect(subject.execute).to eq(0) end + + it "should disable ignoring sentinel file for provisioning" do + expect(machine).to receive(:action) do |name, opts| + expect(name).to eq(:snapshot_restore) + expect(opts[:provision_ignore_sentinel]).to eq(false) + end + subject.execute + end + + it "should start the snapshot" do + expect(machine).to receive(:action) do |name, opts| + expect(name).to eq(:snapshot_restore) + expect(opts[:snapshot_start]).to eq(true) + end + subject.execute + end + + context "when --no-start flag is provided" do + let(:argv) { ["--no-start"] } + + it "should not start the snapshot" do + expect(machine).to receive(:action) do |name, opts| + expect(name).to eq(:snapshot_restore) + expect(opts[:snapshot_start]).to eq(false) + end + subject.execute + end + end end end diff --git a/test/unit/plugins/commands/snapshot/command/restore_test.rb b/test/unit/plugins/commands/snapshot/command/restore_test.rb index 8747998f1..dbbb74b49 100644 --- a/test/unit/plugins/commands/snapshot/command/restore_test.rb +++ b/test/unit/plugins/commands/snapshot/command/restore_test.rb @@ -12,6 +12,7 @@ describe VagrantPlugins::CommandSnapshot::Command::Restore do env.create_vagrant_env end + let(:snapshot_name) { "snapshot_name" } let(:guest) { double("guest") } let(:host) { double("host") } let(:machine) { iso_env.machine(iso_env.machine_names[0], :dummy) } @@ -31,6 +32,11 @@ describe VagrantPlugins::CommandSnapshot::Command::Restore do end describe "execute" do + before do + allow(machine.provider).to receive(:capability). + with(:snapshot_list).and_return([snapshot_name]) + end + context "with no arguments" do it "shows help" do expect { subject.execute }. @@ -93,5 +99,32 @@ describe VagrantPlugins::CommandSnapshot::Command::Restore do to raise_error(Vagrant::Errors::SnapshotNotFound) end end + + it "should disable ignoring sentinel file for provisioning" do + argv << snapshot_name + expect(machine).to receive(:action) do |_, opts| + expect(opts[:provision_ignore_sentinel]).to eq(false) + end + subject.execute + end + + it "should start the snapshot" do + argv << snapshot_name + expect(machine).to receive(:action) do |_, opts| + expect(opts[:snapshot_start]).to eq(true) + end + subject.execute + end + + context "when --no-start flag is provided" do + let(:argv) { [snapshot_name, "--no-start"] } + + it "should not start the snapshot" do + expect(machine).to receive(:action) do |_, opts| + expect(opts[:snapshot_start]).to eq(false) + end + subject.execute + end + end end end From 4696943ae6a51dc2b1b9bbe6aae739d3b9118aaa Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 7 Dec 2018 16:26:13 -0800 Subject: [PATCH 2/2] Add new flag to cli docs --- website/source/docs/cli/snapshot.html.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/website/source/docs/cli/snapshot.html.md b/website/source/docs/cli/snapshot.html.md index 7ce3b1341..6d079162f 100644 --- a/website/source/docs/cli/snapshot.html.md +++ b/website/source/docs/cli/snapshot.html.md @@ -56,6 +56,8 @@ the pushed state. * `--no-delete` - Prevents deletion of the snapshot after restoring (so that you can restore to the same point again later). +* `--no-start` - Prevents the guest from being started after restore + # Snapshot Save **Command: `vagrant snapshot save [vm-name] NAME`** @@ -72,6 +74,8 @@ This command restores the named snapshot. * `--[no-]provision` - Force the provisioners to run (or prevent them from doing so). +* `--no-start` - Prevents the guest from being started after restore + # Snapshot List **Command: `vagrant snapshot list`**