-
Notifications
You must be signed in to change notification settings - Fork 22
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
Post-Processor: added vagrantfile_template_content
option to allow for dynamic Vagrantfile content
#112
base: main
Are you sure you want to change the base?
Post-Processor: added vagrantfile_template_content
option to allow for dynamic Vagrantfile content
#112
Conversation
…Vagrantfile content
581e9b0
to
9290c01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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'")) | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
templatefile
function for dynamic content inVagrantfile
templateExample:
A vagrant template file
A packer file with vagrant post processor
Resulting Vagrantfile inside Box file
Closes #104