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

Add ability to set individual replica count per region #387

Merged
merged 6 commits into from
Apr 8, 2024

Conversation

nywilken
Copy link
Contributor

@nywilken nywilken commented Mar 8, 2024

This changes allows a user to set different replica counts for each
target region defined with a shared_image_gallery_destination block.

Closes: #386

@nywilken
Copy link
Contributor Author

nywilken commented Mar 11, 2024

It looks like the Azure SDK allows for there to be a global replica count as well as a regional replica count. I need to verify what happens when both values are set.

@JenGoldstrich do you have insight on this by chance?

@nywilken nywilken force-pushed the nywilken/target-region-replica-count branch from 90f50f1 to 5281a8c Compare April 8, 2024 19:30
@nywilken nywilken marked this pull request as ready for review April 8, 2024 19:31
@nywilken nywilken requested a review from a team as a code owner April 8, 2024 19:31
This changes allows a user to set different replica counts for each
target region defined with a shared_image_gallery_destination block.

Closes: #386
For backwards compatibility setting shared_image_gallery_replica_count
with replication_regions, will set the replica count value for each
target region that is generated internally.

When setting replica counts using a target block the global attribute
for setting shared image gallery replica counts is not allowed.

```
~>  packer validate
example/ubuntu_managed_image_sig.targetregions.pkr.hcl
Error: 1 error(s) occurred:

* `shared_image_gallery_replica_count` can not be defined alongside
  `target_region`; you can specify the number of replicas per region
  within a single target_region block.

    on example/ubuntu_managed_image_sig.targetregions.pkr.hcl line 21:
      (source code not available)
```
The replica count for the build location is set at the gallery level not
the target region level in the API. To ensure the replica is set
correctly, when using target_region blocks the replica count value for
the build location is updated to match the value in target_region if
set. If not set the global region count defaults to 1.

When using replicated_regions the global replica count value is set as
the value for the target region so that it matches the global count. In
this situation the replica count will be honored by all target regions.
@JenGoldstrich JenGoldstrich force-pushed the nywilken/target-region-replica-count branch from 5281a8c to 90e756f Compare April 8, 2024 20:07
Copy link
Contributor

@JenGoldstrich JenGoldstrich left a comment

Choose a reason for hiding this comment

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

I rebased this and there was one missing test case and an error message I felt was a bit confusingly worded so I changed that, otherwise this LGTM,

if len(b.config.SharedGalleryDestination.SigDestinationTargetRegions) > 0 {
normalizedRegions := make([]TargetRegion, 0, len(b.config.SharedGalleryDestination.SigDestinationTargetRegions))
for _, tr := range b.config.SharedGalleryDestination.SigDestinationTargetRegions {
tr.Name = normalizeAzureRegion(tr.Name)
normalizedRegions = append(normalizedRegions, tr)
if strings.EqualFold(tr.Name, buildLocation) && tr.ReplicaCount != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea!

}

if c.SharedGalleryDestination.SigDestinationUseShallowReplicationMode {
if err := c.SharedGalleryDestination.ValidateShallowReplicationRegion(); err != nil {
errs = packersdk.MultiErrorAppend(errs, err)
}

if c.SharedGalleryImageVersionReplicaCount == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice refactoring!

}

if (len(c.SharedGalleryDestination.SigDestinationTargetRegions) > 0) && c.SharedGalleryImageVersionReplicaCount != 0 {
errs = packersdk.MultiErrorAppend(errs, errors.New("`shared_image_gallery_replica_count` can not be defined alongside `target_region`; you can specify the number of replicas per region within target_region blocks."))
Copy link
Contributor

Choose a reason for hiding this comment

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

Two notes on this I found the error message a bit confusing when talking about single target_region blocks so I reworded that

worth noting this is a breaking change since users would have had to set the replica count with shared_image_gallery_replica_count before, even when using a target_region block, so its worth making the next release a minor version bump, and calling this out in the release notes I think

@JenGoldstrich JenGoldstrich merged commit 8539248 into main Apr 8, 2024
12 checks passed
@JenGoldstrich JenGoldstrich deleted the nywilken/target-region-replica-count branch April 8, 2024 23:47
@dhananjaipai
Copy link

getting the error

An argument named "ReplicaCount" is not expected here

when trying to use

dynamic "target_region" {
      for_each = split(",", var.target_regions)
      content {
        name                 = target_region.value
        ReplicaCount         = 1
      }
    }

I tried different combinations of regional_replica_count, regionalReplicaCount, ReplicaCount and others, can you improve the documentation so that the target_region spec is more clear?

currently is shows only

target_region ([]TargetRegion) - A target region to store the image version in.

you then expect to see something like this but nothing works?

@dhananjaipai
Copy link

Reading through the commits found this

apparently the key was modified to be replicas ?
and I can see zero references to that in the docs

I can create a separate issue on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to set replica count per region for each shared image gallery destination region
3 participants