hashicorp / packer-plugin-azure

Packer plugin for Azure Virtual Machine Image Builders
https://www.packer.io/docs/builders/azure
Mozilla Public License 2.0
51 stars 80 forks source link

Docs for the `target_region` block are confusing and do not provide an easy to find example #444

Open jsloan117 opened 1 month ago

jsloan117 commented 1 month ago

Overview of the Issue

After reviewing the documentation, it appears that replication_regions is deprecated in favor of target_region(s).

Reproduction Steps

Replace replication_regions with target_regions block.

shared_image_gallery_destination {
    gallery_name                         = var.gallery_name
    image_name                           = var.img_name
    image_version                        = var.img_ver
    target_regions                       = [
      {
        name = "centralus"
        replicas = 1
      }
    ]
    resource_group                       = var.resource_group
  }

Plugin and Packer version

Packer version: 1.11.2 Plugin version: v2.1.8

Simplified Packer Buildfile

NA

Operating system and Environment details

Building RHEL 8 image, the build server is Ubuntu 22.04 both are x64.

Log Fragments and crash.log files

I followed https://developer.hashicorp.com/packer/integrations/hashicorp/azure/latest/components/builder/arm#target-regions and searched the repo for references like https://github.com/hashicorp/packer-plugin-azure/blob/93909bcb17b16d207ee60207ff50d7cf8809b47d/.web-docs/components/builder/chroot/README.md?plain=1#L228.

image

JenGoldstrich commented 1 month ago

Hey @jsloan117 I think it's just poorly documented on our part rather than an actual bug. We should probably add an example to the TargetRegion section to make this clearer, but if you check the shared_image_gallery_destination arguments in the ARM docs you can find this example which shows how to define the target_region blocks. Screenshot 2024-10-01 at 4 37 14 PM

Each target_region is its own block, defined without an = operator. So for your code example this would be

shared_image_gallery_destination {
    gallery_name                         = var.gallery_name
    image_name                           = var.img_name
    image_version                        = var.img_ver
    target_region  {
        name = "centralus"
        replicas = 1
      }
    resource_group                       = var.resource_group
  }

I think us calling the previous replicated_regions field deprecated was a bit hasty, and we have no immediate plans to remove it.

I will keep this ticket open to indicate we should improve these docs, but can you confirm for me that you're able to set the target_region block following that example?

jsloan117 commented 1 month ago

@JenGoldstrich - thanks for the quick response! That worked!

The section you linked to was fairly straightforward, in my opinion, and it was going to be my next thing to try when I had time.

My confusion came from the section below.

target-regions The target_regions block is available inside the shared_image_gallery_destination block for setting replica regions and the replica community )

I don't think I fully understand the difference between target_region and target_regions — unless target_regions itself is a typo.

With the above-linked text, I concluded that it was a "cleaner/neater" way to write the syntax, which I drew the wrong conclusion about.

Out of curiosity and a little help from Copilot - would either of the snippets work?

...
source "azure-arm" "example" {
  target_regions = ["East US", "West US", "Central US"]
  # other configuration options
}
...
...
source "azure-arm" "example" {
  target_regions = ["East US", "West US", "Central US"]
  replicas       = 3
  # other configuration options
}
...
JenGoldstrich commented 1 month ago

Yeah I guess target_regions in that block is a typo, I think we should fix that and include an example in that block to make it clearer I agree, I will leave this ticket open to track fixing this in the docs.

Neither of those code snippets will work as I understand it, target_region is not an array of strings, its a block definition, the only way to define them is how I did above, if you want to pass in a list of regions you can use replicated_regions, as I mentioned us calling it deprecated was a bit hasty