-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Refactor Packer templates for Windows and Ubuntu images #12305
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
base: main
Are you sure you want to change the base?
Refactor Packer templates for Windows and Ubuntu images #12305
Conversation
@@ -0,0 +1,21 @@ | |||
locals { |
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.
Will it be better if we pass image_os
to packer instead of image_sku
? And we will choose image_sku
here based on image_os
. With this approach, we will be able to move hardcoded skus from build-image.ps1
to locals.ubuntu.pkr.hcl
and locals.windows.pkr.hcl
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.
It makes sense, will update this part
@@ -44,13 +54,17 @@ Write-Host "Download packer plugins" | |||
packer plugins install github.com/hashicorp/azure $pluginVersion | |||
|
|||
Write-Host "Validate packer template" | |||
packer validate -syntax-only $TemplatePath | |||
packer validate -syntax-only -only "$buildName*" $TemplatePath |
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.
nit: what is the $buildName
here? can we use full name?
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.
The $buildName
here is a second part of build packer template (e.g. if we targeting windows-2025 image with build.windows-2025.pkr.hcl
template, the build name will be windows-2025
)
We can name our build templates whatever we want. What name do you want?
} | ||
} | ||
|
||
// Define local variables for the ubuntu-minimal build |
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 comment is relevant for full locals
block or only first property?
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 is true for the entire block. I had to copy it from the ubuntu-minimal
template to make the refactoring work. If we're not using the ubuntu-minimal image, maybe we should remove it?
|
||
// Define local variables for the ubuntu-minimal build | ||
locals { | ||
ubuntu_minimal_image_os = "ubuntu22" |
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.
consider removing ubuntu-minimal
template and this variable
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.
+1
locals { | ||
ubuntu_minimal_image_os = "ubuntu22" | ||
|
||
toolset_file_name = "toolset-2204.json" |
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.
I think we should use different toolset for different images
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 local is also for ubuntu-minimal
image. Let's delete the ubuntu-minimal
image and related locals.
@@ -0,0 +1,12 @@ | |||
locals { | |||
image_properties = var.image_sku == "2019-Datacenter" ? { |
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.
Two suggestions:
- instead of using
if
conditiona == b ? c : d == e ? f :g
, consider using maps in HCL. - consider using
coalesce
foros_disk_size_gb
It should be something like this:
image_properties_map = {
"2019-Datacenter" = {
image_os = "win19"
os_disk_size_gb = coalesce(var.os_disk_size_gb, 256)
},
"2022-Datacenter" = {
image_os = "win22"Add commentMore actions
os_disk_size_gb = coalesce(var.os_disk_size_gb, 256)
}
}
image_properties = image_properties_map[var.image_sku]
Description
This pull request introduces significant updates to the image build pipeline, including parameter adjustments, template refactoring, and improvements to support multiple operating systems. The key changes involve adding new parameters, refactoring image templates for Ubuntu, and introducing logic to dynamically handle image properties based on the operating system.
Parameter Updates:
BuildTemplateName
parameter toimages.CI/linux-and-win/build-image.ps1
to specify the build template name dynamically.pluginVersion
parameter withUseAzureCliAuth
(default:"false"
) and updated thePluginVersion
default to"2.3.3"
.Template Refactoring:
build.ubuntu-22_04.pkr.hcl
,build.ubuntu-24_04.pkr.hcl
,build.ubuntu-minimal.pkr.hcl
) by removing hardcoded variables and replacing them with shared local variables for better maintainability. [1] [2] [3]Dynamic Image Handling:
switch
statement inbuild-image.ps1
to determine the appropriateimageURN
based on theBuildTemplateName
. This adds support for new OS versions like Windows 2025 and Ubuntu 24.04.locals.ubuntu.pkr.hcl
to dynamically set image properties (e.g., OS type, disk size) based on theimage_sku
.Packer Build Improvements:
packer validate
andpacker build
commands to use theBuildTemplateName
dynamically for better flexibility.These changes enhance the flexibility and scalability of the image build pipeline while improving code maintainability and support for new operating systems.
Related issue:
Check list