Skip to content

Attempt leafcloud build on local disk to compare speed #632

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

Closed
wants to merge 8 commits into from

Conversation

JohnGarbutt
Copy link
Member

@JohnGarbutt JohnGarbutt commented Mar 25, 2025

Leafcloud image builds are volume-backed by default, which causes slow build times

Building images on disk should be quicker for the fatimage build, though volumes are still required for extra-builds

@JohnGarbutt JohnGarbutt requested a review from a team as a code owner March 25, 2025 18:13
@JohnGarbutt JohnGarbutt marked this pull request as draft March 25, 2025 18:13
@bertiethorpe
Copy link
Member

@bertiethorpe bertiethorpe marked this pull request as ready for review April 7, 2025 17:43
@bertiethorpe bertiethorpe requested a review from sjpb April 7, 2025 17:59
Copy link
Collaborator

@sjpb sjpb left a comment

Choose a reason for hiding this comment

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

Think it needs testing on SMS too TBH.

@@ -118,10 +119,16 @@ jobs:
echo $IMAGE_ID > image-id.txt
echo $IMAGE_NAME > image-name.txt

- name: Make image usable for further builds
- name: Make image usable for further builds (if signature_verified property exists)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need this check here, but not for the same in the fatimage workflow below?

@@ -101,6 +101,7 @@ jobs:
-var "source_image_name=${{ fromJSON(env.FAT_IMAGES)['cluster_image'][matrix.build.source_image_name_key] }}" \
-var "image_name=${{ matrix.build.image_name }}" \
-var "inventory_groups=${{ matrix.build.inventory_groups }}" \
-var "use_blockstorage_volume=true" \
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this will be for all clouds. Is that OK? If not, I would set it in e.g. environments/.stackhpc/LEAFCLOUD.pkrvars.hcl instead.

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/stackhpc/ansible-slurm-appliance/actions/runs/14333297338

Disk image build works for SMS, so probably okay to set for all clouds

@@ -109,7 +109,7 @@ variable "manifest_output_path" {

variable "use_blockstorage_volume" {
type = bool
default = true
default = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we're doing this, can you modify docs/image-build.md b/c it currently says vol backed is default. Or, you could just set it to false in CI.

@sjpb
Copy link
Collaborator

sjpb commented May 6, 2025

Replaced by #650

@sjpb sjpb closed this May 6, 2025
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.

3 participants