Reduce build image components templates size #6910

hanwen-cluster · 2025-07-09T17:24:26Z

Description of changes

ImageBuilder has a limit of 16000 with create component API https://docs.aws.amazon.com/imagebuilder/latest/APIReference/API_CreateComponent.html. This commit reduces the size and add a unit test to prevent the template hitting the limit when parameters with long values are provided

With this PR
Component UpdateOSComponent has size 9251
Component ParallelClusterComponent has size 9699
Component ParallelClusterTagComponent has size 8073
Component ParallelClusterTestComponent has size 13547

Tests

Build image on all OSes has passed

Checklist

Make sure you are pointing to the right branch.
If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
Check all commits' messages are clear, describing what and why vs how.
Make sure to have added unit tests or integration tests to cover the new/modified code.
Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

himani2411 · 2025-07-09T18:55:42Z

cli/tests/pcluster/templates/test_imagebuilder_stack.py

+                )
+            )
+            assert_that(
+                len(str(imagebuilder_resources.get(component_name).get("Properties").get("Data")))


Here the length of the component will always remain the same.
Because this component has not replaced the parameters that we pass and which are replaced by Cfn Function Sub.So we need to test that by replacing these paramters in the Data.

Can we just keep it as is. Since the test limits the length to under 13600, it leave 2400 for all parameters

himani2411 · 2025-07-09T19:30:52Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

@@ -13,205 +13,91 @@ constants:
 phases:
  - name: build
    steps:


Could you describe the changes made in parallelcluster.yaml its difficult to follow through and compare without guidance.

I added more comments to describe the changes and hope they will help

himani2411 · 2025-07-09T19:33:31Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-      # Get Cookbook Url
-      - name: CookbookUrl
+      # Initialize system information and URLs
+      - name: SystemInfo


This is really smart!!

Thanks! AI did it

himani2411 · 2025-07-09T19:35:17Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-      # Get Cookbook name
-      - name: PClusterCookbookVersionName
+      # Get operating system name for variable chaining
+      - name: OperatingSystemName


Why not combine OperatingSystemName and SystemInfo?

OperatingSystemName is used also in ‎cli/src/pcluster/config/imagebuilder_config.py as part of the parameter

Why do we set this OS in the image_dna.json when Cookbook could essentially set that for us?
These defaults are used for image_dna.json

Ohh thats to validate OS in cookbook!

himani2411 · 2025-07-09T19:47:31Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-               echo ${!PLATFORM}
-
-      # Get input base AMI Architecture
-      - name: OperatingSystemArchitecture


Can we not remove this check?

IDK. This commit aims at retaining the logic. So I didn't dig into it

we dont retain this though or did i miss something?

We retain it here https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-b012645dfa9db7a854d5437348c00c5460f9c96572a97492c741e97a083e89a2R54-R56

hanwen-cluster · 2025-07-09T19:48:22Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-      # Get AWS Region
-      - name: AWSRegion
-        action: ExecuteBash
-        inputs:
-          commands:
-            - |
-              set -v
-              echo ${AWS::Region}


This is no longer needed after this change https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-64aaeae41ccc76ceed59ed96a79d7754dbb96617d7a7768b6a7cb1c8ad7e2cc0L343-R343

hanwen-cluster · 2025-07-09T19:49:34Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-              RELEASE='{{ build.OperatingSystemRelease.outputs.stdout }}'
-
-              if [ `echo "${!RELEASE}" | grep -w '^amzn\.2'` ]; then
-                OS='alinux2'
-              elif [ `echo "${!RELEASE}" | grep -w '^amzn\.2023'` ]; then
-                OS='alinux2023'
-              elif [ `echo "${!RELEASE}" | grep '^ubuntu\.22'` ]; then
-                OS='ubuntu2204'
-              elif [ `echo "${!RELEASE}" | grep '^ubuntu\.24'` ]; then
-                OS='ubuntu2404'
-              elif [ `echo "${!RELEASE}" | grep '^rhel\.8'` ]; then
-                OS='rhel8'
-              elif [ `echo "${!RELEASE}" | grep '^rocky\.8'` ]; then
-                OS='rocky8'
-              elif [ `echo "${!RELEASE}" | grep '^rhel\.9'` ]; then
-                OS='rhel9'
-              elif [ `echo "${!RELEASE}" | grep '^rocky\.9'` ]; then
-                OS='rocky9'
-              else
-                echo "Operating System '${!RELEASE}' is not supported. Failing build."
-                exit {{ FailExitCode }}
-              fi


Replaced by https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-365de0075f4659870e5ed9878016b28a29065c60bc7bb763da54670e5b626eb6R23-R35

hanwen-cluster · 2025-07-09T19:49:50Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

+              . /etc/os-release
+              RELEASE="${!ID}${!VERSION_ID:+.${!VERSION_ID}}"
+              case "$RELEASE" in
+                amzn.2) echo 'alinux2' ;;
+                amzn.2023) echo 'alinux2023' ;;
+                ubuntu.22*) echo 'ubuntu2204' ;;
+                ubuntu.24*) echo 'ubuntu2404' ;;
+                rhel.8*) echo 'rhel8' ;;
+                rocky.8*) echo 'rocky8' ;;
+                rhel.9*) echo 'rhel9' ;;
+                rocky.9*) echo 'rocky9' ;;
+                *) echo "Unsupported OS: $RELEASE" && exit {{ FailExitCode }} ;;
+              esac


Replaces https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-365de0075f4659870e5ed9878016b28a29065c60bc7bb763da54670e5b626eb6L80-L101

hanwen-cluster · 2025-07-09T19:50:55Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-               echo ${!PLATFORM}
-
-      # Get input base AMI Architecture
-      - name: OperatingSystemArchitecture


IDK. This commit aims at retaining the logic. So I didn't dig into it

hanwen-cluster · 2025-07-09T19:51:47Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-      - name: IsOperatingSystemSupported
-        action: ExecuteBash
-        inputs:
-          commands:
-            - |
-              set -v
-              if [ ${CfnParamUpdateOsAndReboot} == false ]; then
-                RELEASE='{{ build.OperatingSystemRelease.outputs.stdout }}'
-                if [ `echo "${!RELEASE}" | grep -Ev '^(amzn|ubuntu|rhel|rocky)'` ]; then
-                  echo "This component does not support '${!RELEASE}'. Failing build."
-                  exit {{ FailExitCode }}
-                fi
-
-                # This component only supports aarch64 CPUs on Amazon Linux 2, Ubuntu2204, RHEL8, Rocky8, RHEL9 and Rocky9
-                ARCH=$(uname -m)
-                if [[ `echo ${!ARCH}` == 'aarch64' ]]; then
-                  if [ `echo "${!RELEASE}" | grep -Ev '^(amzn\.2|amzn\.2023|ubuntu\.20\.04|ubuntu\.22\.04|ubuntu\.24\.04|rhel\.8|rocky\.8|rhel\.9|rocky\.9)'` ]; then
-                    echo "This component does not support '${!RELEASE}' on ARM64 CPUs. Failing build."
-                    exit {{ FailExitCode }}
-                  fi
-                fi
-              fi


Removed because the check is redundant with https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-365de0075f4659870e5ed9878016b28a29065c60bc7bb763da54670e5b626eb6R23-R35

himani2411 · 2025-07-09T19:57:12Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-                # This component only supports aarch64 CPUs on Amazon Linux 2, Ubuntu2204, RHEL8, Rocky8, RHEL9 and Rocky9
-                ARCH=$(uname -m)
-                if [[ `echo ${!ARCH}` == 'aarch64' ]]; then
-                  if [ `echo "${!RELEASE}" | grep -Ev '^(amzn\.2|amzn\.2023|ubuntu\.20\.04|ubuntu\.22\.04|ubuntu\.24\.04|rhel\.8|rocky\.8|rhel\.9|rocky\.9)'` ]; then


why remove these checks?

why were these checks added when previous steps already checked the OS?

was it centos?

Explained here https://github.com/aws/aws-parallelcluster/pull/6910/files#r2195855408

hanwen-cluster · 2025-07-09T19:55:30Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-      - name: OperatingSystemRelease
-        action: ExecuteBash
-        inputs:
-          commands:
-            - |
-              set -v
-              FILE=/etc/os-release
-              if [ -e ${!FILE} ]; then
-                . ${!FILE}
-                echo "${!ID}${!VERSION_ID:+.${!VERSION_ID}}"
-              else
-                echo "The file '${!FILE}' does not exist. Failing build."
-                exit {{ FailExitCode }}
-              fi


Replaced by https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-365de0075f4659870e5ed9878016b28a29065c60bc7bb763da54670e5b626eb6R24

hanwen-cluster · 2025-07-09T19:56:13Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-      - name: OperatingSystemVersion
-        action: ExecuteBash
-        inputs:
-          commands:
-            - |
-              set -v
-              FILE=/etc/os-release
-              if [ -e ${!FILE} ]; then
-                . ${!FILE}
-                echo "${!VERSION_ID}"
-              else
-                echo "The file '${!FILE}' does not exist. Failing build."
-                exit {{ FailExitCode }}
-              fi


Replaced by https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-365de0075f4659870e5ed9878016b28a29065c60bc7bb763da54670e5b626eb6R68

hanwen-cluster · 2025-07-09T19:57:31Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-      - name: PlatformName
-        action: ExecuteBash
-        inputs:
-          commands:
-            - |
-               set -v
-               OS='{{ build.OperatingSystemName.outputs.stdout }}'
-
-               if [ `echo "${!OS}" | grep -E '^(alinux|rhel|rocky)'` ]; then
-                 PLATFORM='RHEL'
-               elif [ `echo "${!OS}" | grep -E '^ubuntu'` ]; then
-                 PLATFORM='DEBIAN'
-               fi
-
-               echo ${!PLATFORM}


Replaced by https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-365de0075f4659870e5ed9878016b28a29065c60bc7bb763da54670e5b626eb6R52-R53

gmarciani · 2025-07-09T20:07:19Z

I like the approach of sourcing system info from a single file rather than retrieving every time from step outputs.
If we need to further reduce the size we can leverage this approach and move the steps to retrieve all the info into a dedicated component to be executed before parallelcluster main component.

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

himani2411 · 2025-07-11T17:22:26Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

                apt-get clean
                apt-get -y update
                apt-get -y install build-essential curl wget jq
              fi
+              exit 0


why is this exit 0 needed?

It is not strictly needed. It is the logic is complex. We had a failure from installing curl on al2023. Adding exit 0 makes sure the step pass even if some package installation fail. I know this will cover up some issue. But I don't see serious problem when commands here fail to run

himani2411 · 2025-07-11T17:31:20Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

+              
+              curl --retry 3 -L $CINC_URL | bash -s -- -v {{ ChefVersion }}
+              
+              [[ -e $CA_CERTS_FILE ]] && {


Here we are changing the indirect expansion to directly check for the file. IIRC with indirect expansion we check for the existence of the symlink file

I didn't understand this comment. Can you elaborate?

himani2411 · 2025-07-11T17:32:49Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

              cd /etc/chef && tar -xzf /etc/chef/aws-parallelcluster-cookbook.tgz --strip-components 1 && rm -f aws-parallelcluster-cookbook.tgz

-      - name: CreatingChefClientFile
+      - name: CreateFiles


NitPick: Can we name it CreateChefFiles, I dont want the name to be same as action

himani2411 · 2025-07-11T17:36:29Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-              PLATFORM='{{ build.PlatformName.outputs.stdout }}'
+              . /opt/parallelcluster/system_info
+
+              # Disable Nouveau driver
              /bin/sed -r -i -e 's/GRUB_CMDLINE_LINUX="(.*)"/GRUB_CMDLINE_LINUX="\1 rd.driver.blacklist=nouveau nouveau.modeset=0"/' /etc/default/grub
              if [[ ${!PLATFORM} == RHEL ]]; then


we do not need the indirect expansion here as we are no longer keeping PLATFORM='{{ build.PlatformName.outputs.stdout }}'

I see we still use indirect expansion in multiple places, please correct the same

PLATFORM is defined here https://github.com/aws/aws-parallelcluster/pull/6910/files#diff-365de0075f4659870e5ed9878016b28a29065c60bc7bb763da54670e5b626eb6R66

In Bash this syntax ${!PLATFORM} leads to indirect expansion where the value of PLATFORM for example if RHEL, then RHEL will be treated as a variable and any value it holds will be used here.
Below is what it will do

PLATFORM='RHEL' RHEL='OTHER VALUE' echo "$PLATFORM" # Prints RHEL echo "${!PLATFORM}" # Prints OTHER VALUE

The syntax ${PLATFORM} or ``$PLATFORM` is same and is direct expansion where it only value of PLATFORM for example if RHEL.

Plz address the use of indirect expansion in these bash scripts as we not longer use {{ build.AWSRegion.outputs.stdout }} for PLATFORM

I thought {{ build.AWSRegion.outputs.stdout }} was replaced by jinja but now i am not sure

The behavior in bash is different from CloudFormation

In CloudFormation:

To write a dollar sign and curly braces (${}) literally, add an exclamation point (!) after the open curly brace, such as ${!Literal}. CloudFormation resolves this text as ${Literal}.

https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/intrinsic-function-reference-sub.html

Then I would suggest that you make these changes in all the parts of the YAML as I see in some places we are see if [[ ${!PLATFORM} == RHEL ]]; then or if [[ $PLATFORM == RHEL ]]; then

himani2411 · 2025-07-11T17:42:03Z

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml

-            overwrite: true
-
-      - name: RemoveKernelPin
+      - name: Cleanup


NitPick: Can you keep a name like RemoveKernelPinAndCleanup

ImageBuilder has a limit of 16000. This commit reduces the size and add a unit test to prevent the template hitting the limit when parameters with long values are provided Co-authored-by: Himani Anil Deshpande <himanidp@amazon.com>

hanwen-cluster requested review from a team as code owners July 9, 2025 17:24

hanwen-cluster force-pushed the developjul9 branch 2 times, most recently from cc68643 to 863bf90 Compare July 9, 2025 17:29

hanwen-cluster assigned himani2411 and unassigned himani2411 Jul 9, 2025

hanwen-cluster added the skip-changelog-update Disables the check that enforces changelog updates in PRs label Jul 9, 2025

hanwen-cluster force-pushed the developjul9 branch 3 times, most recently from 1769061 to 7209724 Compare July 9, 2025 17:48

himani2411 reviewed Jul 9, 2025

View reviewed changes

hanwen-cluster commented Jul 9, 2025

View reviewed changes

himani2411 reviewed Jul 9, 2025

View reviewed changes

hanwen-cluster commented Jul 9, 2025

View reviewed changes

himani2411 reviewed Jul 11, 2025

View reviewed changes

cli/src/pcluster/resources/imagebuilder/parallelcluster.yaml Show resolved Hide resolved

himani2411 reviewed Jul 11, 2025

View reviewed changes

Reduce build image components templates size

19d7235

ImageBuilder has a limit of 16000. This commit reduces the size and add a unit test to prevent the template hitting the limit when parameters with long values are provided Co-authored-by: Himani Anil Deshpande <himanidp@amazon.com>

hanwen-cluster force-pushed the developjul9 branch from 7209724 to 19d7235 Compare July 11, 2025 18:38


		curl --retry 3 -L $CINC_URL \| bash -s -- -v {{ ChefVersion }}

		[[ -e $CA_CERTS_FILE ]] && {

Reduce build image components templates size #6910

Are you sure you want to change the base?

Reduce build image components templates size #6910

Uh oh!

Conversation

hanwen-cluster commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Tests

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanwen-cluster Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

himani2411 Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmarciani commented Jul 9, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanwen-cluster commented Jul 9, 2025 •

edited

Loading

hanwen-cluster Jul 9, 2025 •

edited

Loading

himani2411 Jul 11, 2025 •

edited

Loading

himani2411 Jul 11, 2025 •

edited

Loading