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

hcp: fix hcp artifact extraction method #12854

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

lbajolet-hashicorp
Copy link
Contributor

With Packer 1.10.1 we started warning when a build failed to complete because of a potential incompatibility with the builder being used.

This led to cases in which the build failed for other reasons, and Packer would still warn of potential incompatibilities, even if the builder was in effect HCP compatible.

We attempted to fix this issue by introducing a new error type, and checks when we read the artifacts linked to a build, however this loop would fail when any one of the artifacts is not compatible with HCP Packer, leading to false failures.

To avoid this problem, we log incompatibilities to the verbose logger, and only signal the problem if all the artifacts could not be used to upload data to HCP Packer, in which case it's almost certain that if the build succeeded and no artifacts are registered to the build, that all the components used are not compatible with HCP, and should be reported as such to users.

@lbajolet-hashicorp lbajolet-hashicorp marked this pull request as ready for review February 26, 2024 22:02
@lbajolet-hashicorp lbajolet-hashicorp requested a review from a team as a code owner February 26, 2024 22:02
@lbajolet-hashicorp lbajolet-hashicorp added the backport/1.10.x Backport PR changes to `release/1.10.x` label Feb 26, 2024
Copy link
Contributor

@sylviamoss sylviamoss left a comment

Choose a reason for hiding this comment

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

Nice catch! 🚀

@sylviamoss
Copy link
Contributor

Is it possible to catch that in a unit test?

@lbajolet-hashicorp
Copy link
Contributor Author

Is it possible to catch that in a unit test?

That seems very probable, I'll take a look at this, good call!

@nywilken nywilken added this to the 1.10.2 milestone Feb 27, 2024
With Packer 1.10.1 we started warning when a build failed to complete
because of a potential incompatibility with the builder being used.

This led to cases in which the build failed for other reasons, and
Packer would still warn of potential incompatibilities, even if the
builder was in effect HCP compatible.

We attempted to fix this issue by introducing a new error type, and
checks when we read the artifacts linked to a build, however this loop
would fail when any one of the artifacts is not compatible with HCP
Packer, leading to false failures.

To avoid this problem, we log incompatibilities to the verbose logger,
and only signal the problem if all the artifacts could not be used to
upload data to HCP Packer, in which case it's almost certain that if the
build succeeded and no artifacts are registered to the build, that all
the components used are not compatible with HCP, and should be reported
as such to users.
@lbajolet-hashicorp lbajolet-hashicorp merged commit 32f8901 into main Feb 27, 2024
11 checks passed
@lbajolet-hashicorp lbajolet-hashicorp deleted the fix_hcp_compat_detection branch February 27, 2024 16:40
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.10.x Backport PR changes to `release/1.10.x` bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants