-
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
packer: add HCL2 support for telemetry #12319
Conversation
4d70d36
to
98af4ea
Compare
98af4ea
to
2c6f175
Compare
@@ -120,9 +120,11 @@ func (m *Meta) GetConfig(cla *MetaArgs) (packer.Handler, int) { | |||
|
|||
switch cfgType { | |||
case ConfigTypeHCL2: | |||
packer.CheckpointReporter.SetTemplateType(packer.HCL2Template) |
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
@@ -489,6 +497,8 @@ func (cfg *PackerConfig) getCoreBuildProvisioner(source SourceUseBlock, pb *Prov | |||
return packer.CoreBuildProvisioner{}, diags | |||
} | |||
|
|||
flatProvisionerCfg, _ := decodeHCL2Spec(pb.HCL2Ref.Rest, ectx, provisioner) |
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.
Lets move this initialization closer to where it is used which right about line 521.
That said, do we want to capture details about the provisioner being wrapped?
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.
You can disregard the question above about including the rapped provisioner info. The configuration should already contain details about the wrapped provisioner so the flattened config will already have all the information, correct?
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.
Theoretically it should yes! I'll test it to be sure though
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.
Oh I need to test this.
Packer's telemetry package captures the field names from a configuration on legacy JSON templates, but did not contain any code for handling HCL2 template configurations. This commit adds a routine to extract the field names from a HCL2 configuration once flattened, and adds some glue code to propagate the configurations to the telemetry structures.
2c6f175
to
6b6269b
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.
This looks good to me. Nicely done. We have an open conversation internally about the type/name but besides that I think we are good to go.
The template type for a builder, provisioner, post-processor or datasource was not tracked with telemetry data. This commit adds the infrastructure to track this, bumping the schema for non-crashes to `beta/packer/6' at the same time.
6b6269b
to
e7dbd4d
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. |
Packer's telemetry package captures the field names from a configuration on legacy JSON templates, but did not contain any code for handling HCL2 template configurations.
This commit adds a routine to extract the field names from a HCL2 configuration once flattened, and adds some glue code to propagate the configurations to the telemetry structures.