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

Add user_data_file to arm builder #123

Merged
merged 1 commit into from
Jul 27, 2021
Merged

Conversation

Itxaka
Copy link
Contributor

@Itxaka Itxaka commented Jul 15, 2021

There is the possibilityof using user_data when launching a vm, which
provides user data during the whole lifecycle, its easily accessible by
all apps (so no cloud-init or waagent required) and can even be modified
(not applicable here but its nice)

This PR adds an user_data_file that mimicks the custom_data_file option
and basically does the same. This causes the API version of the schema
to be 2021-03-01 if there is user data provided.

https://docs.microsoft.com/en-us/azure/virtual-machines/user-data

Closes #121

Signed-off-by: Itxaka [email protected]

There is the possibilityof using user_data when launching a vm, which
provides user data during the whole lifecycle, its easily accessible by
all apps (so no cloud-init or waagent required) and can even be modified
(not applicable here but its nice)

This PR adds an user_data_file that mimicks the custom_data_file option
and basically does the same. This causes the API version of the schema
to be 2021-03-01 if there is user data provided.

https://docs.microsoft.com/en-us/azure/virtual-machines/user-data

Signed-off-by: Itxaka <[email protected]>
@Itxaka Itxaka requested a review from a team as a code owner July 15, 2021 11:24
Copy link
Contributor

@paulmey paulmey left a comment

Choose a reason for hiding this comment

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

LGTM

@mudler
Copy link
Contributor

mudler commented Jul 26, 2021

Hi!

Is there any plan into merging/tackling this? it looks like a really useful feature and I'd be happy to help if there are any changes required in order to accept this

cc: @nywilken @azr could you please have a look? Thanks a lot 🙂

Cheers

@nywilken
Copy link
Contributor

Hi @mudler thanks for bubbling this up. The Packer team has been a bit heads down working on some Packer core related items so we are working through plugins changes slowly. That being said the changes here look good to me as well.

The only change I would suggest is adding an acceptance test to this file for the new user_data_file. Is this a change you would be able to make?

It involves adding a new case with a configuration template that leverages the user_data_file option.

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.

This looks good to me. I'm going to let the change sit for a day to see if we can get an acceptance test added to the change. Otherwise this is good to go.

@mudler
Copy link
Contributor

mudler commented Jul 27, 2021

Hi @mudler thanks for bubbling this up. The Packer team has been a bit heads down working on some Packer core related items so we are working through plugins changes slowly. That being said the changes here look good to me as well.

The only change I would suggest is adding an acceptance test to this file for the new user_data_file. Is this a change you would be able to make?

It involves adding a new case with a configuration template that leverages the user_data_file option.

Thanks for the fast response! Totally understendable, just wanted to check if the direction looks good 👍 Sure, I'll have a look!

@mudler
Copy link
Contributor

mudler commented Jul 27, 2021

I cannot push to the same branch of this PR, shall I open a new one?

Otherwise the patch is here mudler@bc9ae66 - which is good for a first 👀

I couldn't test it locally as it seemed required access to Azure, is there a way to test it with the CI?

@nywilken
Copy link
Contributor

I cannot push to the same branch of this PR, shall I open a new one?

Otherwise the patch is here mudler@bc9ae66 - which is good for a first 👀

I couldn't test it locally as it seemed required access to Azure, is there a way to test it with the CI?

Thanks for taking this on @mudler. I went ahead and reviewed your changes as they currently and left a few comments. The test is off to a good start.

I'm going to merge this PR. Please feel free to open a new one with your test changes.

@nywilken nywilken merged commit 95bb647 into hashicorp:main Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow passing user data
4 participants