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

enabling hcp multibuilds #13328

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

enabling hcp multibuilds #13328

wants to merge 3 commits into from

Conversation

mogrogan
Copy link
Contributor

@mogrogan mogrogan commented Mar 12, 2025

HCP Packer multibuilds

This Pr will remove the check that prevents user from using multiple build block in HCL2 configuration.

Example

// multi-build.pkr.hcl

packer {
  required_plugins {
    docker = {
      version = ">= 1.0.0"
      source  = "github.com/hashicorp/docker"
    }
  }
}

hcp_packer_registry {
  bucket_name = "my-bucket"
  description = "some packer test"
}

source "docker" "debian" {
  image  = "debian"
  commit = true
}

source "docker" "alpine" {
  image  = "alpine"
  commit = true
}

build {
  sources = ["source.docker.debian"]
}

build {
  sources = ["source.docker.alpine"]
}

Output:

$ packer build multi-build.pkr.hcl
Tracking build on HCP Packer with fingerprint "01JPB2F0B22PT3G7BSQCQ6RGC3"
docker.debian: output will be in this color.
docker.alpine: output will be in this color.

==> docker.debian: Creating a temporary directory for sharing data...
==> docker.debian: Pulling Docker image: debian
==> docker.alpine: Creating a temporary directory for sharing data...
==> docker.alpine: Pulling Docker image: alpine
    docker.debian: Using default tag: latest
    docker.alpine: Using default tag: latest
    docker.debian: latest: Pulling from library/debian
    docker.alpine: latest: Pulling from library/alpine
    docker.debian: Digest: sha256:35286826a88dc879b4f438b645ba574a55a14187b483d09213a024dc0c0a64ed
    docker.debian: Status: Image is up to date for debian:latest
    docker.debian: docker.io/library/debian:latest
    docker.alpine: Digest: sha256:a8560b36e8b8210634f77d9f7f9efd7ffa463e380b75e2e74aff4511df3ef88c
    docker.alpine: Status: Image is up to date for alpine:latest
    docker.alpine: docker.io/library/alpine:latest
==> docker.debian: Starting docker container...
    docker.debian: Run command: docker run -v /Users/martin.grogan/.config/packer/tmp29564845:/packer-files -d -i -t --entrypoint=/bin/sh -- debian
==> docker.alpine: Starting docker container...
    docker.alpine: Run command: docker run -v /Users/martin.grogan/.config/packer/tmp3773282027:/packer-files -d -i -t --entrypoint=/bin/sh -- alpine
    docker.debian: Container ID: 75b2aaf56a0acc1aadb291c95642ca73a224b225620bd7f8099929e424e39dc1
==> docker.debian: Using docker communicator to connect: 172.17.0.2
    docker.alpine: Container ID: 5d2267087a3c4a43c6be7df47153c731e031b28cc51a6920ec639a04ed4fe5dc
==> docker.alpine: Using docker communicator to connect: 172.17.0.3
==> docker.debian: Committing the container
==> docker.alpine: Committing the container
    docker.alpine: Image ID: sha256:1efbdd66f335465890f8e97e289facc2653e6983638f32891f1c9227008350cc
==> docker.alpine: Killing the container: 5d2267087a3c4a43c6be7df47153c731e031b28cc51a6920ec639a04ed4fe5dc
    docker.debian: Image ID: sha256:0350cdd87d4183a1106bd164ab8feb72da9cc3fcc71919239cf09d57b48ad9c3
==> docker.debian: Killing the container: 75b2aaf56a0acc1aadb291c95642ca73a224b225620bd7f8099929e424e39dc1
Build 'docker.alpine' finished after 1 second 236 milliseconds.
Build 'docker.debian' finished after 1 second 361 milliseconds.

==> Wait completed after 1 second 531 milliseconds

==> Builds finished. The artifacts of successful builds are:
--> docker.alpine: Imported Docker image: sha256:1efbdd66f335465890f8e97e289facc2653e6983638f32891f1c9227008350cc
--> docker.alpine: Published metadata to HCP Packer registry packer/my-bucket/versions/01JPB2F0PY3NYSP3FDNEDQZ9VG
--> docker.debian: Imported Docker image: sha256:0350cdd87d4183a1106bd164ab8feb72da9cc3fcc71919239cf09d57b48ad9c3
--> docker.debian: Published metadata to HCP Packer registry packer/my-bucket/versions/01JPB2F0PY3NYSP3FDNEDQZ9VG

On the HCP Packer portal:
image
(Note that it's version 4 since I tested some combination before)

@mogrogan mogrogan force-pushed the enabling_hcp_multibuilds branch from 7a6f07b to 8ffabdb Compare March 14, 2025 14:59
Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Left one small nit on a diagnostic message, but besides that, it looks good to me!

@mogrogan mogrogan force-pushed the enabling_hcp_multibuilds branch from 8ffabdb to 367ea52 Compare March 14, 2025 19:23
@mogrogan mogrogan marked this pull request as ready for review March 14, 2025 19:23
@mogrogan mogrogan requested a review from a team as a code owner March 14, 2025 19:23
@mogrogan mogrogan force-pushed the enabling_hcp_multibuilds branch from 367ea52 to 5e2809a Compare March 14, 2025 19:23
Copy link
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Some small adjustments on the diagnostics, another small nit on the test code, but besides those, LGTM!

expectErr: false,
expectedBuilds: []string{"docker.ubuntu", "build.docker.ubuntu"},
expectErr: true,
// expectedBuilds: []string{"docker.ubuntu", "build.docker.ubuntu"},
Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably remove this instead of commenting it

Detail: fmt.Sprintf("Two sources are used in the same build block, causing "+
"a conflict, there must only be one instance of %s", source.String()),
Detail: fmt.Sprintf("Two sources are used in the same build block or "+
"two build has the same name causing a conflict, there must only "+
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"two build has the same name causing a conflict, there must only "+
"two builds have the same name, causing a conflict, there must only "+

if build.Name == "" {
diags = append(diags, &hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Build name conflicts",
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest pointing out at the lack of a build name here instead as the summary

Suggested change
Summary: "Build name conflicts",
Summary: "Missing mandatory build name",

Severity: hcl.DiagError,
Summary: "Build name conflicts",
Subject: &build.HCL2Ref.DefRange,
Detail: "build name is required when using the same source in two different builds",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Detail: "build name is required when using the same source in two different builds",
Detail: "A build name is required when using the same source in two or more different builds",

}
block = build.HCPPackerRegistry
// build block leveled hcp registry is only tolerated when there is only one build block
if len(cfg.Builds) <= 1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Likely not an issue, but if we don't have a build block defined here, what happens? I'm not sure we even reach this code, but I wonder if the check here shouldn't be exactly len(cfg.Builds) == 1 instead of a <=, otherwise we'll crash on the following statement since cfg.Builds[0] will be out-of-bounds

@mogrogan mogrogan force-pushed the enabling_hcp_multibuilds branch 2 times, most recently from 721a0b8 to 33e593a Compare March 17, 2025 16:09
@mogrogan mogrogan force-pushed the enabling_hcp_multibuilds branch from 33e593a to 0794593 Compare March 17, 2025 19:14
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.

2 participants