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

Makefile: make dev uses packer for install #184

Conversation

lbajolet-hashicorp
Copy link
Contributor

When building the development version of the plugin, we now use
packer plugins install for that instead of moving it to the plugins
directory manually.

This target will require Packer 1.10.2 and above to function, as prior
versions don't support instaling pre-release versions of a plugin with
that command.

tuxtof
tuxtof previously approved these changes Mar 18, 2024
wolfganghuse
wolfganghuse previously approved these changes Mar 19, 2024
Copy link
Contributor

@wolfganghuse wolfganghuse left a comment

Choose a reason for hiding this comment

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

LGTM

@lbajolet-hashicorp
Copy link
Contributor Author

lbajolet-hashicorp commented Mar 20, 2024

I see an E2E test failing because the updated dev target needs packer installed for the process to succeed. Given this is a build test I would think invoking make build would be an acceptable replacement, alternatively we could also include the setup-packer action to install packer in the workflow's env.

I'll be happy to amend the PR however you think is more relevant, please let me know which direction's your preferred!

@tuxtof
Copy link
Contributor

tuxtof commented Mar 26, 2024

Hello @lbajolet-hashicorp
for now external action is blocked on the org
please amend with the first solution

@lbajolet-hashicorp lbajolet-hashicorp force-pushed the makefile_dev_use_packer_install branch 2 times, most recently from fa8dc74 to c9e93b7 Compare March 26, 2024 14:01
When building the development version of the plugin, we now use
`packer plugins install` for that instead of moving it to the plugins
directory manually.

This target will require Packer 1.10.2 and above to function, as prior
versions don't support instaling pre-release versions of a plugin with
that command.
@lbajolet-hashicorp
Copy link
Contributor Author

Hi @tuxtof,

Should be good to go on the E2E front now! Had to change a few more things than just the target since there's some build tests later in the pipeline that do have Packer installed, so I updated the plugin installation to use the updated workflows.

Not sure if there's something to do for the black duck policy check to go green on my end, but please let me know if you need anything from me to fix that.

Thanks for the reviews!

@tuxtof
Copy link
Contributor

tuxtof commented Mar 26, 2024

@lbajolet-hashicorp i miss the fact that hashicorp/setup-packer was already authorized and used in some workflow
you can take this path , don't worry for the blackduck error , we have work in progress issue with

@lbajolet-hashicorp
Copy link
Contributor Author

lbajolet-hashicorp commented Mar 26, 2024

@tuxtof regarding make dev, since the workflows are split in two I'd think we don't need to add the setup-packer action to the build step, so long as the dependent step that does the tests is able to get the binary we're good to go.
I don't think installing Packer during the build step improves the workflow here, but please correct me if I'm wrong and I can amend the PR again if you prefer.

Copy link
Contributor

@tuxtof tuxtof left a comment

Choose a reason for hiding this comment

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

just check the entire PR more deeply :-D and that's OK for me

@tuxtof tuxtof merged commit ac84292 into nutanix-cloud-native:main Mar 26, 2024
9 of 10 checks passed
@lbajolet-hashicorp lbajolet-hashicorp deleted the makefile_dev_use_packer_install branch March 26, 2024 15:33
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.

3 participants