Post-Processor: added vagrantfile_template_content option to allow for dynamic Vagrantfile content - #112
Conversation
…Vagrantfile content
581e9b0 to
9290c01
Compare
nywilken
left a comment
There was a problem hiding this comment.
Hi there apologies for the delayed review here. Our focus has been on Packer itself trying to resolve issues with plugin loading. Looking at the changes and reference issue I can understand the need for this feature.
However, I am a little unclear on the expected usage of vagrantfile_template and vagrantfile_template_content. Please take a look at my suggested changes and questions.
If you're no longer interested in making the change I do understand given the time to review. I'll label the PR and await your response.
| if err != nil { | ||
| return nil, false, err | ||
|
|
||
| var templateContent string |
There was a problem hiding this comment.
I believe this condition might be nested in a conflicting if statement.
// lets default to using VagrantTemplateContent
templateContent := config.VagrantfileTemplateContent
if templateContent == "" && config.VagrantfileTemplate != "" {
ui.Message(fmt.Sprintf("Using custom Vagrantfile: %s", config.VagrantfileTemplate))
customBytes, err := ioutil.ReadFile(config.VagrantfileTemplate)
if err != nil {
return nil, false, err
}
templateContent = string(customBytes)
}
if templateContent != "" {
customVagrantfile, err = interpolate.Render(templateContent, &config.ctx)
if err != nil {
return nil, false, err
}
}There was a problem hiding this comment.
@nywilken could you please see the below comment #112 (comment) and let me know if this is still needs to be changed !!
| if c.VagrantfileTemplateContent != "" && c.VagrantfileTemplate == "" { | ||
| errs = packersdk.MultiErrorAppend(errs, fmt.Errorf( | ||
| "'vagrantfile_template' should be provided to use 'vagrantfile_template_content'")) | ||
| } |
There was a problem hiding this comment.
I am of the understanding that you would want to specify just the content with no filepath needed. Is that not the case?
If they are mutually exclusive I would rewrite the logic to be as follows:
| if c.VagrantfileTemplateContent != "" && c.VagrantfileTemplate == "" { | |
| errs = packersdk.MultiErrorAppend(errs, fmt.Errorf( | |
| "'vagrantfile_template' should be provided to use 'vagrantfile_template_content'")) | |
| } | |
| if c.VagrantfileTemplateContent != "" && c.VagrantfileTemplate != "" { | |
| errs = packersdk.MultiErrorAppend(errs, fmt.Errorf( | |
| "'vagrantfile_template' can not be used with 'vagrantfile_template_content'")) | |
| } |
|
Hi @VishnuJin just want to follow up to see if you would like to continue with this change. Please let me know if you have time to continue pushing it forward and if you have any questions about the provided feedback. Thank you for your contributions to this plugin. |
As specified in the issue, users can now use
templatefilefunction for dynamic content inVagrantfiletemplateExample:
A vagrant template file
A packer file with vagrant post processor
Resulting Vagrantfile inside Box file
Closes #104