Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose default vagrantfile template as a template variable {{.DefaultTemplate}} #59

Merged
merged 2 commits into from
Oct 18, 2023

Conversation

dangra
Copy link
Contributor

@dangra dangra commented Aug 10, 2022

The whole point is to pass a vagrantfile template that can extend the default template.
As a consequence, this PR also includes tests for template source field not tested until now.

source "vagrant" "foo" {
  communicator         = "ssh"
  source_path          = "some/box"
  provider             = "virtualbox"
  template             = "Vagrantfile.tpl"
}
# Vagrantfile.tpl
{{ .DefaultTemplate }}

Vagrant.configure("2") do |config|
  config.vm.provider "virtualbox" do |vb|
    vb.customize ['modifyvm', :id, '--nested-hw-virt', 'on']
  end
end

@dangra dangra requested a review from a team as a code owner August 10, 2022 17:32
@hashicorp-cla
Copy link

hashicorp-cla commented Aug 10, 2022

CLA assistant check
All committers have signed the CLA.

@dangra
Copy link
Contributor Author

dangra commented Jan 10, 2023

I still find this useful and it comes with tests to validate it works as expected :)

@nywilken
Copy link
Contributor

Hi @dangra apologies for the longer then normal review process. We haven't had the chance to prioritize this review. We have however selected the PR and will provide feedback if you are still interested in working on the change. Thanks!

@dangra
Copy link
Contributor Author

dangra commented Sep 25, 2023

I still rely on a workaround so will be happy to discuss further improvements to get this merged

Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again @dangra the changes look good to me so far. From looking at the code and description you are essentially making the template variable {{.DefaultTemplate}} available for users. Which gives the option of generating a template file containing multiple version configuration blocks like the following:

Vagrant.configure("2") do |config|
  # Generated by the vagrant builder.... contents of {{.DefaultTemplate}}
end

Vagrant.configure("2") do |config|
  # some other pre-exisiting configuration 
end

Would you mind sharing details on your use case so I can better understand the why.

That said, If you could add a comment to the builder file here for the {{.DefaultTemplate}}. Maybe something like example below and run make generate that would update all the documentation files.

+       // Alternatively, the template variable `{{.DefaultTemplate}}` is available for
+       // use if you wish extend the default generated template.

I'm also happy to update the docs for you on this PR but I would like to understand the use case better to add a little more light as to why in the docs.

@dangra dangra force-pushed the default-template-var branch from b68f5c5 to d536988 Compare October 18, 2023 12:39
@dangra
Copy link
Contributor Author

dangra commented Oct 18, 2023

@nywilken that is exactly it.

My use case is the same I posted in the example at the top, I need to enable Nested Harward Virtualization when packing with Virtualbox provider, so this line:

vb.customize ['modifyvm', :id, '--nested-hw-virt', 'on']

I current workaround (without this patch) is to define a template that duplicates the default template plus above line:

# Vagrantfile.template
Vagrant.configure("2") do |config|
  config.vm.define "source", autostart: false do |source|
    source.vm.box = "{{.SourceBox}}"
    config.ssh.insert_key = {{.InsertKey}}
  end
  config.vm.define "output" do |output|
    output.vm.box = "{{.BoxName}}"
    output.vm.box_url = "file://package.box"
    config.ssh.insert_key = {{.InsertKey}}
  end
  {{ if ne .SyncedFolder "" -}}
  config.vm.synced_folder "{{.SyncedFolder}}", "/vagrant"
  {{- else -}}
  config.vm.synced_folder ".", "/vagrant", disabled: true
  {{- end}}

  config.vm.provider "virtualbox" do |vb|
    vb.customize ['modifyvm', :id, '--nested-hw-virt', 'on']
  end
end

but this is not ideal, I'd prefer to always use the default plus my customization for whatever is the default on that packer version.

So, ideally:

{{ .DefaultTemplate }}

Vagrant.configure("2") do |config|
  config.vm.provider "virtualbox" do |vb|
    vb.customize ['modifyvm', :id, '--nested-hw-virt', 'on']
  end
end

I hope that makes more sense. thanks!

@dangra
Copy link
Contributor Author

dangra commented Oct 18, 2023

That said, If you could add a comment to the builder file here for the {{.DefaultTemplate}}. Maybe something like example below and run make generate that would update all the documentation files.

+       // Alternatively, the template variable `{{.DefaultTemplate}}` is available for
+       // use if you wish extend the default generated template.

Done

@nywilken nywilken force-pushed the default-template-var branch from d536988 to 9116328 Compare October 18, 2023 14:09
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great thanks for the update. I rebased and pushed up the changes. I will merge and release once all goes green.

@nywilken nywilken changed the title Default vagrantfile template as template var Expose default vagrantfile template as a template variable {{.DefaultTemplate}} Oct 18, 2023
@nywilken nywilken merged commit eb3bba7 into hashicorp:main Oct 18, 2023
11 checks passed
@dangra dangra deleted the default-template-var branch October 18, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants