Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ CHANGELOG
- Improve cluster update resiliency on login nodes by reusing the head-node-driven orchestration already in place on compute nodes,
removing the dependency on cfn-hup and cfn-init.
- Move all ParallelCluster-managed bootstrap files off `/tmp` into a dedicated `/opt/parallelcluster/tmp`
directory. Therefore, Image builds, cluster creations and updates work on custom AMIs that mount `/tmp` with `noexec`.
directory. Therefore, cluster creations and updates work on custom AMIs that mount `/tmp` with `noexec`. Image builds work on the custom AMIs only if GDRcopy installation is skipped.

**CHANGES**
- The validator `ClusterNameValidator` now enforces cluster names to be limited to 40 characters when using `ExternalSlurmdbd`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,12 +296,3 @@ phases:
# Final cleanup
rm -f /opt/parallelcluster/system_info
/usr/local/sbin/ami_cleanup.sh "${CfnParamIsOfficialAmiBuild}"

- name: validate
steps:
- name: PClusterValidate
action: ExecuteBash
inputs:
commands:
- |
echo "Check ParallelCluster software stack has been installed"
Empty file.
2 changes: 2 additions & 0 deletions cli/src/pcluster/templates/imagebuilder_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
PCLUSTER_IMAGE_NAME_TAG,
PCLUSTER_S3_BUCKET_TAG,
PCLUSTER_S3_IMAGE_DIR_TAG,
PCLUSTER_TMP_DIR,
PCLUSTER_VERSION_TAG,
)
from pcluster.imagebuilder_utils import (
Expand Down Expand Up @@ -347,6 +348,7 @@ def _add_imagebuilder_image_recipe(self, build_tags, components):
version=utils.get_installed_version(base_version_only=True),
tags=build_tags,
parent_image=self.config.build.parent_image,
working_directory=PCLUSTER_TMP_DIR,
components=components,
block_device_mappings=[
imagebuilder.CfnImageRecipe.InstanceBlockDeviceMappingProperty(
Expand Down
132 changes: 101 additions & 31 deletions cloudformation/patching/ami-patching.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Description: >-

Parameters:
ParentImage:
Description: The ParallelCluster AMI to patch.
Description: The AMI to patch.
Type: String
InstanceType:
Description: Instance type used by Image Builder to build the patched AMI.
Expand All @@ -18,6 +18,7 @@ Parameters:
PatchScriptS3Uri:
Description: S3 URI (s3://bucket/key) of the patching script to run on the build instance.
Type: String
Default: ""
PatchFlavour:
Description: >-
Patching flavour passed to the patch script. 'minimal' applies only security
Expand All @@ -31,6 +32,17 @@ Parameters:
- full
- minimal-capped
- full-capped
EnableNoExecTmp:
Description: Build an AMI with noexec, nosuid, and nodev mount options on /tmp instead of applying patches.
Type: String
Default: "false"
AllowedValues:
- "true"
- "false"

Conditions:
BuildNoExecTmpImage: !Equals [!Ref EnableNoExecTmp, "true"]
BuildPatchedImage: !Equals [!Ref EnableNoExecTmp, "false"]

Resources:

Expand Down Expand Up @@ -85,19 +97,14 @@ Resources:
urllib.request.urlopen(req)

def cleanup(stack_name):
# Deregister the patched AMI(s) built by this stack and delete their
# snapshots. The snapshots are tagged first so DeleteSnapshot is allowed
# by the (tag-scoped) IAM policy.
# Deregister the patched AMI(s) built by this stack and delete their snapshots.
if not stack_name:
return
images = ec2.describe_images(Owners=["self"], Filters=[
{"Name": "tag:parallelcluster:ami-patching-stack", "Values": [stack_name]}]).get("Images", [])
for img in images:
snaps = [m["Ebs"]["SnapshotId"] for m in img.get("BlockDeviceMappings", [])
if m.get("Ebs", {}).get("SnapshotId")]
if snaps:
ec2.create_tags(Resources=snaps, Tags=[
{"Key": "parallelcluster:ami-patching-stack", "Value": stack_name}])
ec2.deregister_image(ImageId=img["ImageId"])
for snap in snaps:
ec2.delete_snapshot(SnapshotId=snap)
Expand Down Expand Up @@ -142,12 +149,6 @@ Resources:
- Effect: Allow
Action: ec2:DescribeImages
Resource: "*"
- Effect: Allow
Action: ec2:CreateTags
Resource: !Sub arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:snapshot/*
Condition:
StringEquals:
aws:RequestTag/parallelcluster:ami-patching-stack: !Ref AWS::StackName
- Effect: Allow
Action: ec2:DeregisterImage
Resource: !Sub arn:${AWS::Partition}:ec2:${AWS::Region}::image/*
Expand All @@ -156,16 +157,18 @@ Resources:
aws:ResourceTag/parallelcluster:ami-patching-stack: !Ref AWS::StackName
- Effect: Allow
Action: ec2:DeleteSnapshot
Resource: !Sub arn:${AWS::Partition}:ec2:${AWS::Region}:${AWS::AccountId}:snapshot/*
Resource: !Sub arn:${AWS::Partition}:ec2:${AWS::Region}::snapshot/*
Condition:
StringEquals:
aws:ResourceTag/parallelcluster:ami-patching-stack: !Ref AWS::StackName

# ===========================================================================
# Image Builder
#
# Builds the patched AMI: the build instance downloads and runs the patching
# script, reboots, executes the AMI cleanup and create the new AMI.
# Builds an AMI using the selected component: by default the build instance
# downloads and runs the patching script; when EnableNoExecTmp is true, it
# configures restrictive /tmp mount options instead. Both paths reboot, run
# the AMI cleanup, and create a new AMI.
# ===========================================================================

PatchedImage:
Expand All @@ -184,11 +187,13 @@ Resources:
Name: !Sub pcluster-ami-patching-recipe-${AWS::StackName}
Version: 1.0.0
ParentImage: !Ref ParentImage
WorkingDirectory: /opt/parallelcluster/tmp
Components:
- ComponentArn: !Ref PatchComponent
- ComponentArn: !If [BuildNoExecTmpImage, !Ref NoExecTmpComponent, !Ref PatchComponent]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need a separate gate? Can;t we just go with a patch component that does not make use of exec in tmp?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This template is reused for two purposes. One is to create an patched AMI, the other one is to create an AMI with noexec on tmp. Depending on the input parameters, we choose one component


PatchComponent:
Type: AWS::ImageBuilder::Component
Condition: BuildPatchedImage
Properties:
Name: !Sub pcluster-ami-patching-${AWS::StackName}
Platform: Linux
Expand Down Expand Up @@ -233,6 +238,66 @@ Resources:
commands:
- /usr/local/sbin/ami_cleanup.sh

NoExecTmpComponent:
Type: AWS::ImageBuilder::Component
Condition: BuildNoExecTmpImage
Properties:
Name: !Sub pcluster-tmp-noexec-${AWS::StackName}
Platform: Linux
Version: 1.0.0
Description: Persist and validate noexec, nosuid, and nodev mount options on /tmp.
Data: |
name: ConfigureNoExecTmpMount
description: Persist and validate restrictive mount options on /tmp.
schemaVersion: 1.0
phases:
- name: build
steps:
- name: ConfigureTmpMount
action: ExecuteBash
inputs:
commands:
- |
set -euxo pipefail
awk '
function add_option(options_list, option, item_count, i, items) {
item_count = split(options_list, items, ",")
for (i = 1; i <= item_count; i++) {
if (items[i] == option) return options_list
}
return options_list "," option
}
BEGIN { found = 0 }
$1 !~ /^#/ && $2 == "/tmp" {
$4 = add_option($4, "noexec")
$4 = add_option($4, "nosuid")
$4 = add_option($4, "nodev")
found = 1
}
{ print }
END {
if (!found) print "/tmp", "/tmp", "none", "bind,rw,nodev,nosuid,noexec", "0", "0"
}
' OFS='\t' /etc/fstab > /etc/fstab.pcluster-tmp-noexec
install -o root -g root -m 0644 /etc/fstab.pcluster-tmp-noexec /etc/fstab
rm -f /etc/fstab.pcluster-tmp-noexec
findmnt --verify --tab-file /etc/fstab
- name: Reboot
action: Reboot
- name: VerifyTmpMountAfterReboot
action: ExecuteBash
inputs:
commands:
- |
set -euxo pipefail
options_list=$(findmnt -no OPTIONS --target /tmp)
for option in noexec nosuid nodev; do
echo "$options_list" | tr ',' '\n' | grep -Fx "$option"
done
if [[ -x /usr/local/sbin/ami_cleanup.sh ]]; then
/usr/local/sbin/ami_cleanup.sh
fi

RecipeLogGroup:
Type: AWS::Logs::LogGroup
DeletionPolicy: Retain
Expand Down Expand Up @@ -262,13 +327,16 @@ Resources:
Distributions:
- Region: !Ref AWS::Region
AmiDistributionConfiguration:
# The distributed AMI name is "patched-<SourceName>-<buildDate>". The
# Patched AMIs use "patched-<SourceName>-<buildDate>". The
# "patched-" prefix keeps the name from matching the pcluster AMI name
# filter used to discover base AMIs, so the patched AMI is only consumed
# explicitly by id and never picked up as the "latest" AMI on updates.
Name: !Sub
- "patched-${SourceName}-{{ imagebuilder:buildDate }}"
- SourceName: !GetAtt AmiHelper.SourceName
Name: !If
- BuildNoExecTmpImage
- !Sub "tmp-noexec-${AWS::StackName}-{{ imagebuilder:buildDate }}"
- !Sub
- "patched-${SourceName}-{{ imagebuilder:buildDate }}"
- SourceName: !GetAtt AmiHelper.SourceName
AmiTags:
parallelcluster:ami-patching-stack: !Ref AWS::StackName
parallelcluster:source-ami: !Ref ParentImage
Expand All @@ -292,16 +360,18 @@ Resources:
ManagedPolicyArns:
- !Sub arn:${AWS::Partition}:iam::aws:policy/EC2InstanceProfileForImageBuilder
- !Sub arn:${AWS::Partition}:iam::aws:policy/AmazonSSMManagedInstanceCore
Policies:
- PolicyName: read-patch-script
PolicyDocument:
Version: 2012-10-17
Statement:
- Effect: Allow
Action: s3:GetObject
Resource: !Sub
- arn:${AWS::Partition}:s3:::${BucketAndKey}
- BucketAndKey: !Select [1, !Split ["s3://", !Ref PatchScriptS3Uri]]
Policies: !If
- BuildNoExecTmpImage
- !Ref AWS::NoValue
- - PolicyName: read-patch-script
PolicyDocument:
Version: 2012-10-17
Statement:
- Effect: Allow
Action: s3:GetObject
Resource: !Sub
- arn:${AWS::Partition}:s3:::${BucketAndKey}
- BucketAndKey: !Select [1, !Split ["s3://", !Ref PatchScriptS3Uri]]

BuildSecurityGroup:
Type: AWS::EC2::SecurityGroup
Expand Down
4 changes: 4 additions & 0 deletions tests/integration-tests/configs/develop.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ test-suites:
instances: ["g4dn.2xlarge"]
oss: {{ NO_RHEL_OS_X86 }}
schedulers: ["slurm"]
- regions: ["us-east-1"]
instances: ["c5.4xlarge"]
oss: [{{ OS_X86_2 }}]
schedulers: ["slurm"]
- regions: ["cnn1-az1"]
instances: ["g4dn.2xlarge"]
schedulers: ["slurm"]
Expand Down
41 changes: 41 additions & 0 deletions tests/integration-tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import time
from functools import partial
from itertools import product
from pathlib import Path
from shutil import copyfile
from traceback import format_tb
from typing import Any, Dict, List, Optional, Union
Expand Down Expand Up @@ -2474,3 +2475,43 @@ def _store_secret(region, secret_string=None, secret_binary=None):
for secret_arn in secrets:
logging.info("Deleting secret %s", secret_arn)
secrets_manager_client.delete_secret(SecretId=secret_arn)


@pytest.fixture()
def noexec_tmp_ami_factory(region, vpc_stack, request, cfn_stacks_factory):
"""Build temporary AMIs whose /tmp mount is noexec from early boot."""
template_path = Path(__file__).resolve().parents[2] / "cloudformation" / "patching" / "ami-patching.yaml"
template_body = template_path.read_text(encoding="utf-8")
built = []

def _build(parent_image, builder_instance):
stack_name = generate_stack_name("integ-tests-tmp-noexec-ami", request.config.getoption("stackname_suffix"))
stack = CfnStack(
name=stack_name,
region=region,
template=template_body,
parameters=[
{"ParameterKey": "ParentImage", "ParameterValue": parent_image},
{"ParameterKey": "InstanceType", "ParameterValue": builder_instance},
{"ParameterKey": "SubnetId", "ParameterValue": vpc_stack.get_public_subnet()},
{"ParameterKey": "VpcId", "ParameterValue": vpc_stack.cfn_outputs["VpcId"]},
{"ParameterKey": "EnableNoExecTmp", "ParameterValue": "true"},
],
capabilities=["CAPABILITY_IAM"],
)
logging.info("Building a temporary /tmp-noexec AMI from %s", parent_image)
cfn_stacks_factory.create_stack(stack)
ami_id = stack.cfn_outputs["AmiId"]
boto3.client("ec2", region_name=region).get_waiter("image_available").wait(ImageIds=[ami_id])
built.append((ami_id, stack_name))
logging.info("Temporary /tmp-noexec AMI %s is available", ami_id)
return ami_id

yield _build
if request.config.getoption("no_delete"):
logging.info("--no-delete specified: retaining temporary /tmp-noexec AMI(s) and stack(s): %s", built)
return

for ami_id, stack_name in reversed(built):
logging.info("Deleting stack %s and temporary /tmp-noexec AMI %s", stack_name, ami_id)
cfn_stacks_factory.delete_stack(stack_name, region)
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,6 @@ SharedStorage:
- MountDir: /shared
Name: name1
StorageType: Ebs
DevSettings:
Cookbook:
ExtraChefAttributes: '{"cluster": {"tmp_noexec": "true"}}'
6 changes: 4 additions & 2 deletions tests/integration-tests/tests/common/data/gpu_job.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ else
exit 2
fi

WORKDIR=$(mktemp -d)
trap 'rm -rf "$WORKDIR"' EXIT
WORKDIR=$(sudo -n mktemp -d "/opt/parallelcluster/tmp/pcluster-cuda-samples.XXXXXX")
sudo -n chown "$(id -u):$(id -g)" "$WORKDIR"
export TMPDIR="$WORKDIR"
trap 'sudo rm -rf "$WORKDIR"' EXIT

# Shared scaffolding required by every sample (Common/, top-level cmake/)
cp -r "$SAMPLES_SRC"/{Common,cmake,CMakeLists.txt} "$WORKDIR"/
Expand Down
13 changes: 8 additions & 5 deletions tests/integration-tests/tests/createami/test_createami.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,11 @@ def _get_base_ami(region, os, architecture):
Uses first-stage AMIs for RHEL/Rocky/Ubuntu (kernel version requirements),
remarkable (Deep Learning) AMIs for ubuntu2204, and official AMIs for everything else.
Returns (base_ami, feature_flags) where feature_flags is a dict with keys:
enable_nvidia, update_os_packages, enable_lustre_client, enable_dcv.
enable_nvidia, update_os_packages, enable_lustre_client.
"""
enable_nvidia = os not in ["ubuntu2404"]
update_os_packages = os in ["alinux2023", "rocky9"]
enable_lustre_client = True
enable_dcv = True
Comment thread
himani2411 marked this conversation as resolved.

if os in ["ubuntu2204"]:
# Test Deep Learning AMIs
Expand All @@ -87,7 +86,6 @@ def _get_base_ami(region, os, architecture):
"enable_nvidia": enable_nvidia,
"update_os_packages": update_os_packages,
"enable_lustre_client": enable_lustre_client,
"enable_dcv": enable_dcv,
}
return base_ami, feature_flags

Expand Down Expand Up @@ -189,6 +187,7 @@ def test_build_image(
s3_bucket_factory,
build_image_custom_resource,
images_factory,
noexec_tmp_ami_factory,
request,
clusters_factory,
scheduler_commands_factory,
Expand All @@ -213,15 +212,19 @@ def test_build_image(
# Get base AMI and feature flags
base_ami, flags = _get_base_ami(region, os, architecture)

enable_nvidia = flags["enable_nvidia"] and get_gpu_count(instance) > 0
if not enable_nvidia:
# Build from an AMI where /tmp is already noexec at boot.
base_ami = noexec_tmp_ami_factory(base_ami, instance)

image_config = pcluster_config_reader(
config_file="image.config.yaml",
parent_image=base_ami,
instance_role=instance_role,
bucket_name=bucket_name,
enable_nvidia=str(flags["enable_nvidia"] and get_gpu_count(instance) > 0).lower(),
enable_nvidia=str(enable_nvidia).lower(),
update_os_packages=str(flags["update_os_packages"]).lower(),
enable_lustre_client=str(flags["enable_lustre_client"]).lower(),
enable_dcv=str(flags["enable_dcv"]).lower(),
)

image = images_factory(image_id, image_config, region)
Expand Down
Loading
Loading