-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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 Packer registry iteration_id to the contextual variables #11319
Conversation
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.
LGTM 👍🏼
Should we wait for @nywilken's review? Just in case there was a very specific use case that required the initialization after |
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.
This looks great. Just a couple of suggestions.
internal/registry/types.bucket.go
Outdated
return nil | ||
} | ||
|
||
// initializeIteration populates the bucket iteration with the details needed for tracking builds for a Packer run. |
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.
// initializeIteration populates the bucket iteration with the details needed for tracking builds for a Packer run. | |
// PopulateIteration populates the bucket iteration with the details needed for tracking builds for a Packer run. |
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.
nice catch!
hcl2template/types.packer_config.go
Outdated
@@ -109,7 +109,8 @@ func (cfg *PackerConfig) EvalContext(ctx BlockContext, variables map[string]cty. | |||
}), | |||
buildAccessor: cty.UnknownVal(cty.EmptyObject), | |||
packerAccessor: cty.ObjectVal(map[string]cty.Value{ | |||
"version": cty.StringVal(cfg.CorePackerVersionString), | |||
"version": cty.StringVal(cfg.CorePackerVersionString), | |||
"iteration_id": cty.UnknownVal(cty.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.
nit: We don't seem to use snake case naming for access variables. Should this be iterationID
or IterationID
?
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.
Yeah, good point. We're inconsistent with this but the build accessor mostly uses CamelCase
|
||
# HCP Packer Iteration ID | ||
|
||
If your build is pushing metadata to the HCP Packer reigstry, this variable is |
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.
What do you think about including an example with the manifest post-processor as well?
post-processor "manifest" {
output = "manifest.json"
strip_path = true
custom_data = {
iteration = "${packer.iteration_id}"
}
}
{
"name": "basic-example",
"builder_type": "file",
"build_time": 1633962545,
"files": [
{
"name": "compressed_artifact",
"size": 26
}
],
"artifact_id": "File",
"packer_run_uuid": "00821859-e924-1325-13c3-f1e62237875e",
"custom_data": {
"iteration": "01FHQW3H5J6F4FYSWQ7DX85N2C"
}
}
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.
will do!
TBH I can't seem to remember why I added it after the call the GetBuilds. I think I might've just thought that we didn't have all the information needed prior to that but that may have changes. I also recall thinking about the only or exclude flags being a thing. But I can't seem to remember. I tested uses cases for building a new image, updating an incomplete image, using the only flags and all works expected. Cancelling a build part way through seems to fail harder then I remember but it has been a while since I cancelled builds so I need to revisit older versions. I also doin't think this change is related. |
b0081dc
to
b763b79
Compare
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. |
Adds the contextual variable packer.iteration_id to HCL templates, as well as documentation for this value.
The biggest challenge was getting the iteration ID into the eval context for builds, since the initialization was happening after the builds were evaluated, so we could initialize builds into the iteration.
I did this by splitting the bucket.Initialize into two functions -- initialize(), which I moved before the GetBuilds call, and PopulateIteration, which I kept after the GetBuilds call. We create an empty iteration, then load the builds using an eval context that contains the iteration id, then load the placeholders for the newly-parsed builds into the placeholder iteration.
This doesn't implement a corollary for JSON templates; if there is demand for it, we can add it later.
Usage is shown in the docs.