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

Support named builds in HCL2 templates #9245

Merged
merged 6 commits into from Jun 2, 2020
Merged

Support named builds in HCL2 templates #9245

merged 6 commits into from Jun 2, 2020

Conversation

AdrienneCohea
Copy link
Contributor

@AdrienneCohea AdrienneCohea commented May 18, 2020

I believe that the legacy JSON format supports named builds, but I found that HCL2 does not, and that's a feature I would really like to have. Here is how I propose to do this:

build {
  name = "foo"

  sources = [
    ...
  ]

 ...
}

Name is an optional string field. If the value is empty, then Packer logs will show the type of the build (for example, googlecompute or amazon-ebs). If the value is populated, then log lines relating to each separate build will be labeled as the user requested. For example:

compute: output will be in this color.
configuration-store: output will be in this color.
scheduler: output will be in this color.
vault_standalone: output will be in this color.

==> scheduler: Checking image does not exist...
==> configuration-store: Checking image does not exist...
==> vault_standalone: Checking image does not exist...
==> compute: Checking image does not exist...
==> scheduler: Using existing SSH private key
==> configuration-store: Using existing SSH private key
==> compute: Using existing SSH private key

I added a test. After analysis of the code, I have a good faith belief it is not in the legacy JSON code path, so I didn't check that.

Closes #9246

@AdrienneCohea AdrienneCohea requested a review from a team as a code owner May 18, 2020 05:20
@codecov
Copy link

codecov bot commented May 18, 2020

Codecov Report

Merging #9245 into master will decrease coverage by 0.00%.
The diff coverage is 57.14%.

Impacted Files Coverage Δ
packer/build.go 69.64% <0.00%> (-0.95%) ⬇️
hcl2template/types.build.go 96.49% <100.00%> (+0.19%) ⬆️
hcl2template/types.packer_config.go 69.37% <100.00%> (+0.11%) ⬆️
builder/azure/arm/tempname.go 80.55% <0.00%> (-5.56%) ⬇️
packer/rpc/server.go 90.90% <0.00%> (-4.55%) ⬇️
packer/rpc/mux_broker.go 61.32% <0.00%> (-3.78%) ⬇️
packer/communicator.go 75.53% <0.00%> (ø)
packer/plugin/client.go 80.35% <0.00%> (+0.44%) ⬆️

Copy link
Contributor

@azr azr left a comment

Choose a reason for hiding this comment

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

Hello @AdrienneCohea ! Super nice PR, thanks I like this idea.

Please note that currently a build will be named with it's source type and label: so for example amazon-ebs.ubuntu-1604 and currently you can use the only and except flags using a glob pattern to match builders; so say packer build --only amazon-ebs.* will only run everything of type amazon-ebs.

⚠️ Note that we have to make that field visible in the logs.

Now I think it's also nice to optionally allow to label a group of builders like you are suggesting but I suggest we improve it a little by moving that field to the label section, for example:

build "somebuild" {

What do you think ?

Note that I don't think naming all the builds of a build section similarly would be a good idea and I think a good idea here would be to name the builder with the label of the build section, for example:

somebuild.amazon-ebs.ubuntu-1604 ( and that, only when that field is set ).

@sylviamoss
Copy link
Member

Just to keep track, this will close #9167 as well.

@azr
Copy link
Contributor

azr commented May 18, 2020

Hello there again ! I wanted to give pointers as to how to do this, did some research and realised it is not possible to make block naming optional. So - sorry ! - scratch that part from my last comment.

Edit: this requires a bit more thinking from our side just to be sure, hang in there !

@azr azr added the stage/thinking Flagged for internal discussions about possible enhancements label May 19, 2020
@azr
Copy link
Contributor

azr commented May 19, 2020

Hello again, sorry for spamming a bit here; I had to think a bit more. I just opened #9257 that uses source type and name as an identifier. I think this will solve your initial issue, the only thing with your implemented solution is that if a builder is using two different sources you will see:

compute: output will be in this color.
compute: output will be in this color.
compute: output will be in this color.

And this will still be not the best; I recommend we update the builders to have a mandatory label at the top:

build "mandatory-label" {
  sources = [
    "foo.bar"
    "baz.bar"
  ]
}

build "other-mandatory-label" {
  sources = [
    "foo.bar"
  ]
}

Will show:

mandatory-label.foo.bar: output will be in this color.
mandatory-label.baz.bar: output will be in this color.
other-mandatory-label.foo.bar: output will be in this color.

@azr azr removed the stage/thinking Flagged for internal discussions about possible enhancements label May 19, 2020
@azr
Copy link
Contributor

azr commented Jun 2, 2020

Hello there, I've fixed a conflict here and just tweaked the name output so now:

source "null" "test" {
    communicator = "none"
}

source "null" "potato" {
    communicator = "none"
}

build {
    name = "a"

    sources = [
        "sources.null.test",
        "sources.null.potato",
    ]

}


build {
    sources = [
        "sources.null.potato",
    ]
}

will display:

> packer build ./null/
Build 'a.null.test' finished.
Build 'a.null.potato' finished.
Build 'null.potato' finished.

==> Builds finished. The artifacts of successful builds are:
--> a.null.test: Did not export anything. This is the null builder
--> a.null.potato: Did not export anything. This is the null builder
--> null.potato: Did not export anything. This is the null builder

@azr azr self-requested a review June 2, 2020 09:47
Copy link
Contributor

@azr azr left a comment

Choose a reason for hiding this comment

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

This LGTM

@SwampDragons SwampDragons merged commit 19ae0ec into hashicorp:master Jun 2, 2020
@azr azr mentioned this pull request Jun 4, 2020
34 tasks
@SwampDragons SwampDragons added this to the 1.6.0 milestone Jun 4, 2020
@ghost
Copy link

ghost commented Jul 3, 2020

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@hashicorp hashicorp locked and limited conversation to collaborators Jul 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Named Builds in HCL2
4 participants