Repository navigation
feat(aws-pcs): multi-user (OpenLDAP), multi-AZ, IAM policies, Region coverage, SSH access - #2
Closed
DaisukeMiyamoto wants to merge 91 commits into
Closed
DaisukeMiyamoto wants to merge 91 commits into
DaisukeMiyamoto wants to merge 91 commits into
Conversation
Two principals around the PCS reference cluster have very different
responsibilities, so split them into two sample policies:
- cluster-admin-policy.json (16 Sids, ~7.4 KB) — for the deploying
principal. Covers create / update / delete of every resource the
templates provision: CloudFormation parent + nested stacks; EC2
networking lifecycle (VPC, subnet, IGW, NAT, EIP, SG, route table, VPC
endpoint, launch template, placement group); FSx (Lustre + OpenZFS);
pcs:*; IAM role + instance profile lifecycle scoped to *PCS* /
*ImageBuilder* names with iam:PassRole gated by iam:PassedToService;
SSM Parameter Store for Grafana password + DLAMI auto-resolve;
KMS / Secrets Manager / Image Builder / CloudWatch Logs vended
delivery. Derived primarily from the AWS-published "minimum permissions
for an AWS PCS service administrator" policy
(security-min-permissions.html), with VPC + FSx + IAM lifecycle layered
because the all-in-one template provisions those itself.
- cluster-user-policy.json (6 Sids, ~1.5 KB) — for end users (ML
engineers) of an already-deployed cluster. Covers ec2/cfn describe for
login-node discovery; pcs:Get*/List* for cluster status; ssm:GetParameter
scoped to /pcs/*/grafana/*; ssm:StartSession scoped via
ssm:resourceTag/aws:pcs:compute-node-group-name=login* (so users CAN
open shells on login nodes but CANNOT on compute nodes); session
terminate-own only via session/${aws:username}-*. No write access of
any kind to cluster resources.
Both policies validated cleanly (0 findings) by aws accessanalyzer
validate-policy.
iam/README.md walks through what each policy covers, the design
intent (combined CRUD vs phase-split, customer-managed vs inline,
why no AmazonPCSFullAccess), what is intentionally NOT covered (the
compute instance role uses the AWS-managed AWSPCSComputeNodePolicy),
how to attach, AWS-managed policy pairing recipe, and the recommended
refinement loop via aws accessanalyzer start-policy-generation against
CloudTrail history of a sandbox deploy.
Sample-grade — slightly broader than strict least-privilege so they
work end-to-end in typical accounts. Production users should tighten
with resource-level conditions (account ID, region, stack-name prefix,
tag matchers) before adopting.
Two changes that came out of real-world verification:
1) The combined cluster-admin policy was ~7.4 KB, exceeding the IAM
per-policy 6,144-char limit (which applies to BOTH inline and
customer-managed policies — the previous README claim that
customer-managed had a 17,408-char limit was wrong; that 17,408 is
the per-user-aggregate inline-policy ceiling).
Split into:
* cluster-admin-policy.json (5,769 chars, 14 Sids — core CFN/EC2/
FSx/PCS/IAM/SSM/KMS/Secrets/Logs)
* cluster-admin-imagebuilder-policy.json (1,674 chars, 2 Sids —
Image Builder + scoped iam:PassRole to imagebuilder.amazonaws.com)
Both fit inside the 6,144-char limit; most users only need the core.
2) The cluster-user policy used
ssm:resourceTag/aws:pcs:compute-node-group-name to scope login-node
access, but PCS does not actually emit that tag (verified via
describe-instances on a deployed login node). Real tags are:
* aws:pcs:compute-node-group-id (random PCS ID, can't be policy-pinned)
* monitoring-role (semantic for monitoring exporters, not access)
* Name (the all-in-one templates set this to PCS-login on login CNGs
and PCS-<cng-name> on compute CNGs, e.g. PCS-cpu1, PCS-hpc8a)
Switched the SSM-StartSession scope to
ssm:resourceTag/Name=PCS-login*. The Name tag is operator-mutable, so
the README now flags this as a hardening point with a forking option
(add a dedicated IsLoginNode tag to the templates).
Wrap each policy file in a CloudFormation template and put a Deploy
button in iam/README.md:
architectures/aws-pcs/assets/cluster-admin-iam.yaml
Creates 2 customer-managed policies (core + optional Image Builder)
+ an IAM Group with both attached. AttachUsers param adds existing
users to the group; AttachImageBuilderPolicy=true pulls in the IB
statements (default false). Output: GroupName, CorePolicyArn,
ImageBuilderPolicyArn (when applicable).
architectures/aws-pcs/assets/cluster-user-iam.yaml
Creates 1 customer-managed policy + IAM Group. Same AttachUsers
pattern. Output: GroupName, PolicyArn.
YAML templates live in assets/ to match the rest of the reference
architecture's CFN artifacts (same S3 sync target, same Quick-create
URL pattern, same maintenance pipeline). The raw JSON files stay under
iam/ as the source of truth that the YAML embeds.
Verified end-to-end on us-east-2 / 2026-06-10:
- Both IAM CFN stacks reach CREATE_COMPLETE
- Test admin user (only customer-managed policies attached) deploys
pcs-ml-cluster-deploy-all.yaml hpc8a configuration end-to-end with
OnDemandEnableEfa=true to CREATE_COMPLETE in ~28 min, no AccessDenied
in 332+ CloudTrail API calls; UpdateStack on
OnDemandMaxCount→3 reaches UPDATE_COMPLETE; DeleteStack via the
admin user is allowed (simulator: cloudformation:DeleteStack=allowed)
- Test user can pcs:GetCluster, ssm:StartSession on login node (Name=
PCS-login), and is implicitDeny on compute node (Name=PCS-hpc8a)
and on cfn:CreateStack / pcs:CreateCluster / ec2:RunInstances
- aws accessanalyzer validate-policy on all three JSON files: 0 errors
iam/README.md updated to:
- Add Quick-create Deploy buttons pointing at the prod S3 path
awsome-distributed-ai.s3.amazonaws.com/templates/cluster-{admin,user}-iam.yaml
- Document the verification matrix and what the admin user actually
needs (the Name-tag scope rationale + caveat)
- Replace the wrong 17,408-char claim with the correct 6,144-char
explanation and the split rationale
- Keep the manual install path (raw JSON) for CLI-only environments
Templates were also uploaded to the test bucket (midaisuk-llm-dev) for
pre-merge sandbox testing.
…ault VPC name from stack name, label cleanup Non-breaking improvements identified in the parameter audit (43 params, no removals possible without breaking PR awslabs#1124 deployments): 1. **Auto-derive `OnDemandEfaInterfaceCount` from `OnDemandInstanceType`** (mirrors the pattern `add-cng-p5.yaml`/`add-cng-p6-*.yaml` use for GPU families). New default 0 means "auto"; old 1/2 values still work as explicit pins so existing stacks pass through unchanged. Auto map: hpc8a.96xlarge / hpc7a.{96,48,24,12}xlarge / hpc6id.32xlarge → 2 anything else (hpc6a.48xlarge, c7i.metal, etc.) → 1 Removes the 2-NIC HPC tax: hpc8a/hpc7a were the most common EFA targets but had to override the parameter; now the default just works. 2. **`VPCName` default → empty, auto-resolves to `${StackName}-VPC`**. Multiple deployments in one account now get unique VPC names without the user having to pass a different VPCName each time. 3. **ParameterGroup label cleanup**: "6. FSx for Lustre (/fsx) + FSx for OpenZFS (/home) (Advanced)" → "6. FSx for Lustre (/fsx) and FSx for OpenZFS (/home)" (Capacity / LustreDeploymentType / PerUnitStorageThroughput are commonly tuned and Region-dependent; not advanced) "7. Developer / Advanced (Nested Templates & Monitoring Source)" → "7. Developer (do not change for normal use)" 4. **ParameterLabel trims**: "Availability Zone (required)" → "Availability Zone" (CFN console already shows the asterisk for required params) "VPC name" → "VPC name (empty = ${StackName}-VPC)" "AMI ID (empty = latest PCS-Ready Deep Learning AMI from SSM; pin in production)" → "AMI ID (empty = SSM auto-resolve; pin in production)" "dcgm-exporter image (default DCGM 4.5.2 by digest; covers H100/B200/B300)" → "dcgm-exporter image (default covers H100/B200/B300)" "EFA interface count (1 for hpc6a, 2 for hpc8a/hpc7a/hpc6id)" → "EFA interface count (0 = auto from instance type)" PARAMETERS.md updated to match. Backward compatibility: every change preserves prior behavior for any stack that was deployed off the published default — VPCName defaulting from "ML-Cluster-VPC" to ${StackName}-VPC only affects new launches where the user did not pass VPCName explicitly. Old stacks keep their existing VPC name on UPDATE_STACK. (Larger reductions tracked in subagent plan for the next minor: collapse CngName/QueueName pairs, drop *MinCount, drop LustreVersion/Compression, rename Capacity → LustreCapacity for prefix consistency. All of those are renames or removes, so deferred to avoid breaking PR awslabs#1124 users.)
Adds opt-in multi-user support via OpenLDAP running on the login node. Default is off (DeployDirectory=false) — existing single-user (ubuntu) clusters are completely unchanged. When DeployDirectory=true: - Login node: installs slapd, stores DB on shared /home/ldap-db (OpenZFS), so the directory survives login CNG recreation. Admin password is stored in SSM Parameter Store at /pcs/<cluster-id>/ldap/admin-password. - Compute nodes: installs SSSD with ldap provider, discovers login node IP via ec2:DescribeInstances tag query, configures NSS/PAM so LDAP users are visible as POSIX users on all nodes. - Home directories auto-created via pam_mkhomedir on shared /home. - Slurm sees LDAP users transparently via NSS (no Slurm config change). New parameters on add-cng.yaml: - DeployDirectory (true/false, default false) - DirectoryDomainSuffix (e.g. dc=cluster,dc=internal) New scripts: - scripts/setup-openldap-server.sh — slapd install + config on login - scripts/setup-ldap-client.sh — SSSD client config on compute nodes - scripts/ldap-add-user.sh — helper to add POSIX users to the directory Not yet wired into deploy-all.yaml or GPU templates (next commit).
Covers enabling multi-user, managing users (add/remove/list/password/groups), running jobs as specific users, SSH access for LDAP users, UID/GID conventions, SSSD caching behavior, troubleshooting, data persistence, and upgrade path to Simple AD / Managed AD.
- setup-openldap-server.sh: add apt-get update before install (PCS-Ready DLAMI has limited apt sources cached; without update, slapd package is not found) - setup-ldap-client.sh: same fix - tests/multi-user-test.md: comprehensive test suite (5 parts: server health, user lifecycle, Slurm integration, multi-node consistency, resilience). Split from README.md to keep the main test guide concise. - tests/README.md: added Test 11 link to multi-user-test.md
…+ access methods doc - setup-ldap-client.sh: change min_id from 10000 to 1001. GID 3000 (clusterusers) was being filtered as "primary gid out of range" by SSSD, preventing user resolution. min_id=1001 excludes only system users (0-1000) while allowing all LDAP-provisioned GIDs. - setup-openldap-server.sh: persist the admin password to /home/ldap-db/.admin-password (shared OpenZFS). Previously the randomly generated password was lost after the setup script exited, making it impossible to administer the directory after login node replacement. The file is chmod 600 and on shared storage. - USER-MANAGEMENT.md: added "Access methods" section comparing SSM vs SSH-over-SSM vs Direct SSH (pro/con/best-for table), SSH key management approaches, and Slurm accounting with PCS (what's managed by PCS vs what admins still need to do manually with sacctmgr). Verified: getent passwd testuser1 now returns testuser1:*:10001:3000:Test User 1:/home/testuser1:/bin/bash on the login node after SSSD restart.
- pcs-ml-cluster-deploy-all.yaml: add DeployDirectory + DirectoryDomainSuffix params (group 7), forward to both LoginNodeGroupStack and OnDemandCNGStack. Without this wiring, compute nodes never received DeployDirectory=true and SSSD client setup was skipped. - docs/USER-MANAGEMENT.md: add ASCII art template structure diagram showing how DeployDirectory flows from deploy-all through the nested stacks into UserData on login (slapd) and compute (SSSD client) nodes.
…nLDAP-LoginNode)
Rename the boolean parameter to an enum that makes it clear both *what*
directory service is used and *where* it runs:
DirectoryService:
AllowedValues: [none, OpenLDAP-LoginNode]
# Future: SimpleAD, ManagedAD, OpenLDAP-External
This prepares for adding Simple AD or external LDAP options in the future
without a breaking rename. The value "OpenLDAP-LoginNode" communicates
that slapd runs on the login node (not an external server).
Also:
- README.md: added template nesting structure diagram (ASCII art)
showing how DirectoryService flows from deploy-all through login
(slapd server) and compute (SSSD client) CNGs.
- Fixed DeployMonitoring default accidentally changed to 'none' by
bulk sed (reverted to 'false' for add-cng, 'true' for deploy-all).
The bulk sed that renamed DeployDirectory→DirectoryService also changed Default:'false' to Default:'none' on every parameter in both templates. This broke FSxLustreEnableEfa, OnDemandEnableEfa, DeployPseriesCNG (deploy-all) and EnableEfa (add-cng) — their AllowedValues are [true,false] so 'none' is invalid. Reverted to 'false' for all 4. Only DirectoryService (default 'none') and MonitoringRole (default 'none') correctly use 'none'.
…ared /home The admin password was previously written to /home/ldap-db/.admin-password (shared OpenZFS, readable by root on all nodes). This is a security concern in multi-user clusters where compute-node users may have sudo. Now: - setup-openldap-server.sh first tries SSM PutParameter (SecureString) at /pcs/<cluster-id>/ldap/admin-password. SSM is access-controlled via IAM and auditable via CloudTrail. - Falls back to the file only if the instance role lacks ssm:PutParameter (with a WARNING in the install log). - cluster.yaml instance role: broadened the SSM Sid from grafana/* to also cover ldap/* (same pattern, same cluster-scoped resource ARN). - Removed the duplicate SSM put from add-cng.yaml UserData (the script now handles it internally with CLUSTER_ID exported from UserData). - USER-MANAGEMENT.md: added fallback note + clarified SSM is the primary storage mechanism.
…up-directory.sh Single script with role argument (server|client) replaces two separate scripts. Benefits: - One URL to manage, no naming inconsistency - Full picture visible in one file (server + client + shared SSSD config) - Natural extension point for future SimpleAD (add a case branch) - Server role now also configures SSSD on the login node itself (so getent works locally without a separate manual step) UserData in add-cng.yaml simplified to: curl setup-directory.sh if login → bash setup-directory.sh server else → bash setup-directory.sh client Environment variable interface unchanged (LDAP_DOMAIN_SUFFIX, CLUSTER_ID, DIRECTORY_DNS_IPS for future SimpleAD). DIRECTORY_DNS_IPS="" triggers login-IP discovery via ec2:DescribeInstances tag query. Also: README template structure diagram updated with external scripts and SSM parameter references.
…ated DirectoryRole param MonitoringRole is for monitoring exporter placement — reusing it for directory server/client branching was a semantic mismatch. New parameter DirectoryRole (none | server | client): - add-cng.yaml: DirectoryRole param added. UserData passes it directly to setup-directory.sh as the role argument. No more MonitoringRole reference in the directory block. - deploy-all.yaml: forwards DirectoryRole='server' to login CNG and DirectoryRole='client' to compute CNG (both conditional on DirectoryEnabled). New Condition DirectoryEnabled added. - Removed unused Conditions (IsLoginNode, SetupLdapServer, SetupLdapClient) that depended on MonitoringRole for directory logic. MonitoringRole remains unchanged for its original purpose (monitoring exporter placement). The two concerns are now fully independent.
Scripts are now fetched from the same S3 bucket as the nested templates (S3BucketName/S3KeyPrefix + scripts/). This eliminates: - Hardcoded GitHub raw URLs that need branch-switching for testing - Dependency on GitHub availability at instance boot time - Version skew between templates and scripts Changes: - add-cng.yaml: UserData uses `aws s3 cp` from the S3 bucket to fetch setup-directory.sh. New params S3BucketName + S3KeyPrefix forwarded from deploy-all. - deploy-all.yaml: forwards S3BucketName/S3KeyPrefix to both login and compute CNG stacks. - cluster.yaml: instance role gets s3:GetObject on */templates/scripts/* so nodes can fetch scripts at boot. Development workflow: 1. Edit scripts/ locally 2. aws s3 sync scripts/ s3://test-bucket/templates/scripts/ 3. Deploy with S3BucketName=test-bucket 4. Scripts are fetched from test bucket — no branch URL changes needed Production: scripts sync alongside templates to the prod bucket. Future PCS post-install hook: pass the same S3 URL.
…ally
The prod S3 sync pipeline runs:
aws s3 sync assets/ s3://awsome-distributed-ai/templates/
Previously scripts/ was a sibling directory requiring a separate sync
command (which was never added). Moving scripts under assets/ means the
existing one-line sync deploys both templates and scripts with zero
operational change for maintainers.
Result on S3:
s3://bucket/templates/pcs-ml-cluster-deploy-all.yaml
s3://bucket/templates/add-cng.yaml
s3://bucket/templates/scripts/setup-directory.sh ← new
s3://bucket/templates/scripts/install-enroot-pyxis.sh
s3://bucket/templates/scripts/ldap-add-user.sh
UserData fetches via:
aws s3 cp s3://${S3BucketName}/${S3KeyPrefix}scripts/setup-directory.sh
README path references updated.
…README USER-MANAGEMENT.md completely rewritten: - Quick reference table at top (common tasks → one-liner commands) - How-it-works diagram (slapd → SSSD → NSS/PAM flow) - Step-by-step for every operation: add/delete/list users, reset password, create groups, batch add, Slurm accounting registration - Verification steps (how to confirm user is visible on compute nodes) - Troubleshooting section (common errors + fixes) - UID/GID convention table - Data persistence + backup/restore - Template structure (how DirectoryRole flows) - Upgrade path to Simple AD README.md: - Fixed template diagram: old script names → setup-directory.sh server/client - Fixed "fetched from GitHub raw" → "fetched from S3" - Added USER-MANAGEMENT.md + DEPLOY-TESTING.md to Additional Resources
- DeployDirectory → DirectoryService=OpenLDAP-LoginNode in tests/ - scripts/install-enroot-pyxis.sh → assets/scripts/... in template descriptions (add-cng*.yaml), ROADMAP, tests/README.md - PostInstallScriptUrl default URL: .../scripts/... → .../assets/scripts/... (file was git-mv'd, GitHub raw URL must match new repo path) - No remaining references to old paths or old param names
…r-monitoring link
…skips shellscript sections)
Test 12 (accounting-test.md):
- Validates Slurm managed accounting with LDAP multi-user
- Covers: sacctmgr user/account creation, resource limits (GrpTRESRunMins),
job tracking (sacct), reporting (sreport), fairshare, and
AccountingPolicyEnforcement behavior (none vs associations,limits,safe)
- Based on https://aws.amazon.com/blogs/hpc/introducing-managed-accounting-for-aws-parallel-computing-service/
Test 13 (gpu-healthcheck-test.md):
- Integrates 4.validation_and_observability/2.gpu-cluster-healthcheck suite
- Lightweight (checks 0-3: nvidia-smi, DCGM L2, EFA enum, topology) ~15 min
- NCCL multi-node (check 5) with per-instance bandwidth thresholds
- Slurm prolog integration guide for production
- When-to-run decision table (deploy, pre-training, steady-state, quarantine)
README: added GPU Health Check link to Additional Resources.
tests/README.md: added Test 12 + Test 13 entries with links.
README.md was too long — reduced from 842 to 73 lines (index + matrix only). Test procedures moved to per-category files: infra-test.md — Tests 1-3, 8 (monitoring, container runtime, AMI build) compute-test.md — Tests 4-6 (CPU queue, GPU families, NCCL EFA) training-test.md — Test 7 (FSDP Llama-2 7B) hpc-efa-test.md — Test 9 (EFA on CPU HPC, OSU benchmarks) storage-test.md — Test 10 (FSx health + performance regression) multi-user-test.md — Tests 11-12 (OpenLDAP + accounting, merged) gpu-healthcheck-test.md — Test 13 (GPU health check suite) Removed accounting-test.md (merged into multi-user-test.md since they are always tested together).
…reaks cloud-init parsing)
…ion numbering
Before: 13 top-level sections mixing core usage with advanced features and reference.
After: 11 sections with clear separation:
§1-7: Core (Key Features → Running a GPU job)
§8: Advanced Features
8.1 Monitoring
8.2 Pre-baking AMI
8.3 User Management (new — was standalone §11)
8.4 IAM Permissions (new)
§9: Templates (reference)
§10: Testing and Validation (reference)
§11: Additional Resources
This makes it clear that §1-7 is the primary user path (deploy + run jobs),
and everything in §8 is opt-in for production/multi-user/security hardening.
…y from cluster core Before: DeployMonitoring + GrafanaPublicAccessCidr were under "PCS Cluster Configuration" (which is really scheduler/AMI/accounting settings). After: §2. PCS Cluster Configuration — Slurm version, login instance, AMI, accounting §7. Additional Cluster Configuration — Monitoring + Multi-User Directory This separates "what the cluster IS" (§2) from "what extra features are layered on top" (§7). Also renamed §6 from "(Advanced)" to just "FSx Storage" (Capacity/DeploymentType are commonly tuned, not advanced) and §8 to just "Developer / Advanced" (shorter).
…cAccessCidr→GrafanaAccessCidr Three changes in deploy-all parameter interface: 1. **SSHAccessCidr** (new, §2 PCS Cluster Configuration): CIDR to open port 22 on the login node. Empty = SSH over SSM only. For multi-user clusters where users connect via standard SSH. 2. **DeployMonitoring → MonitoringStack** (rename, §7): Boolean true/false → enum: 'Prometheus-LoginNode' (default) | 'none'. Aligns with DirectoryService naming pattern (<what>-<where>). Nested stacks receive DeployMonitoring="true"/"false" via !If [MonitoringEnabled] so add-cng.yaml is unchanged (backward compat within the nest). 3. **GrafanaPublicAccessCidr → GrafanaAccessCidr** (rename, §7): Shorter name, same semantics. Opens HTTPS/443 on login node. Security group redesign: - LoginAccessSecurityGroup now conditionally includes BOTH SSH (from SSHAccessCidr) and HTTPS (from GrafanaAccessCidr) rules via !If. - Created only when at least one CIDR is set (LoginAccessEnabled = OR). - Attached only to login node via ExtraSecurityGroupId (compute unaffected). New Conditions: SSHAccessEnabled, GrafanaAccessEnabled, LoginAccessEnabled (OR of both), MonitoringEnabled.
…esults - training-test.md: Test 7b Megatron-LM GPT-3 TP/PP/DP on p6-b200 x4 (~134 TFLOP/s/GPU, lm loss 10.91->10.46, EFA efa-direct 8 nics) - gpu-healthcheck-test.md: intensive suite EFA loopback PASS on p6-b200; caveat that check 5 (NCCL) needs the ECR '#' URI + in-image binary path, so validate multi-node NCCL via canonical Test 6 - README.md: matrix row 7,7b + Megatron canonical-asset row
Capture the follow-up to make CapacityReservationId work for targeted ODCRs (not just Capacity Blocks): a CapacityReservationType enum (none/capacity-block/targeted-odcr) that branches MarketType + placement group, with none/capacity-block staying backward compatible. Notes how to verify without GPU capacity (MinCount=0 launch-template assertion + cheap-type consumption check).
… slim advanced detail - §4 Configuration: add DirectoryService / SSHAccessCidr / GrafanaAccessCidr / AdditionalSubnetAZ2-3 rows so the new user-facing features are discoverable - §4: fix HomeThroughput AllowedValues (SINGLE_AZ_HA_1 includes 64) - §1: correct capacity wording (open ODCR auto-consumed; targeted ODCR on roadmap) - §9: p6-b300 row shows 17 interfaces (16 EFA + 1 ENA), consistent with §4 - move EFA-on-CPU + FSx-Lustre-EFA into Advanced (§8.5/§8.6); consolidate the DCGM version detail into a note at the end of §8.1 Monitoring - §8.2: note the single-login-node / SPOF constraint for DirectoryService - §7: replace specific NCCL busbw numbers with a pointer to tests/compute-test.md - §4: document OnDemandInstanceType + PseriesInstanceType accepted values
…tale diagram memo - OPERATIONS.md: correct OpenZFS HomeThroughput allowed values (192/384/768 are not valid AWS values); SINGLE_AZ_2 groups with HA_2 (160..10240), SINGLE_AZ_HA_1/SINGLE_AZ_1 = 64..4096 — matches the template Rules - USER-MANAGEMENT.md: note that slurm-25.11 paths must become slurm-25.05 when deployed with SlurmVersion=25.05 - delete docs/architecture-components.md: an orphan diagram-drafting memo (not linked anywhere, the architecture image is already published) with stale content (32x EFA on all GPUs, single private subnet, optional Enroot/Pyxis)
…hitecture, slim test memos README: - §1 Key Features: add multi-user (OpenLDAP), access control (IAM + SSH/Grafana CIDR), and multi-AZ / broad Region coverage - §2 Architecture: subnets across up to 3 AZs, SSH/Grafana CIDR access, and the optional OpenLDAP / IAM add-ons - §4 Configuration: reorder the parameter table to match the deploy-all console parameter-group order (Network → PCS Cluster → On-Demand → GPU → Additional); point OnDemandEfaInterfaceCount at §8.5 tests/README: - drop the one-off 'GPU health check verified on B200' memo and the Mumbai 'exercised most deeply' paragraph (results live in the per-test files) - region-coverage GPU column is now a simple ran-on-reserved-capacity check (us-east-1/us-east-2/us-west-2 verified via Capacity Block for ML); drop the per-instance-type detail from the table
…m earlier rounds - README §1: remove the 'Multi-AZ and broad Region coverage' Key Features bullet (multi-AZ stays documented in §4 / Architecture; not a headline feature) - tests/README region coverage: clarify the GPU column ✅ reflects earlier GPU validation rounds, not necessarily the row's deploy/monitoring verification date
…etail sections Key Features bullets describe capability + link to the relevant section instead of embedding parameter names (MonitoringStack/GrafanaAccessCidr/DirectoryService/ SSHAccessCidr). Detailed config stays in §4 and §8.
…s in the table - replace the two overlapping intro paragraphs with one: only PrimarySubnetAZ is required; the table is the most-used subset grouped by the console's parameter groups (separator rows), with storage in its own table and the full reference in PARAMETERS.md - keep the in-table group separator rows for discoverability
…move FSx after it Container Runtime (PostInstallScriptUrl/Args) is rarely touched directly, so it no longer gets its own console group: folded into '5. Additional Cluster Configuration (Monitoring, Multi-User, Container Runtime)'. FSx Storage moves after Additional (now group 6). New order: Network, PCS Cluster, On-Demand, GPU, Additional, FSx, Developer. README §4 separators and PARAMETERS.md §2b updated to match.
…from §7 - move the PCS-Ready DLAMI 'no AMI build needed' note from Key Features into §2 Architecture, pointing to §4 'AMI and container runtime' for the detail (dedup) - §7: link the GPU Cluster Health Check suite as a pre-long-run check - §8.1 Monitoring: replace the MonitoringRepo/Version/DcgmExporterImage bullets with a one-line pointer to PARAMETERS.md (users rarely change these); move the monitoring-role vs Name-tag note to the end of §8.1
Replace the single long table (with in-table separator rows and verbose Purpose cells) with one small table per console parameter group (Network / PCS cluster / On-Demand / GPU / Additional). Trim each Purpose to one line and move the long PseriesInstanceType accepted-values list to the GPU compute subsection. Easier to scan; group structure is obvious from the subheadings.
…with deploy-small tip - §4 config sub-tables now carry the console group numbers (1 Network … 5 Additional) - move the FSx deployment-types/sizing content out of §4 into a new §8.1 Storage at the top of Advanced Features; renumber the rest (Monitoring 8.2 … FSx-EFA 8.7) and fix all §8.x anchor references in README + PARAMETERS.md - add a 'deploy small, expand after' tip: FSx Lustre/OpenZFS can be grown after create, and a smaller filesystem deploys faster — start near minimum Capacity/HomeCapacity and expand once the stack is CREATE_COMPLETE
…s, refocus DEPLOY-TESTING, move IAM verification to tests - remove --profile claude (project test setting) from DEPLOY-TESTING.md and tests/infra-test.md; generalize fixed bucket/region to placeholders - DEPLOY-TESTING.md rewritten for the real audience: a third party deploying not-yet-published templates by hosting them in their own S3 bucket and pointing S3BucketName/S3KeyPrefix at it - PARAMETERS.md sections + order now match the deploy-all console parameter groups exactly (1 Network, 2 PCS Cluster, 3 On-Demand, 4 GPU, 5 Additional incl. monitoring repo/version/dcgm + container runtime, 6 FSx, 7 Developer); fix stale anchor links - move the IAM verification results out of docs/IAM.md into a reproducible tests/iam-test.md (representative two-role use case); add the row to tests/README
The standalone §8.7 'FSx for Lustre over EFA (GPUDirect Storage)' is storage content, so it's now a #### subsection at the end of §8.1 Storage instead of a separate top-level Advanced feature. Removes the cross-reference note.
…template deploys Adds an Advanced Features subsection that links docs/DEPLOY-TESTING.md (deploying fork/ branch/PR templates from your own S3 bucket via S3BucketName/S3KeyPrefix). This also gives DEPLOY-TESTING.md its README entry point — every docs/ file is now linked from the README.
The '5. Additional' heading said 'container runtime' but the table only listed the common params (MonitoringStack/GrafanaAccessCidr/DirectoryService). Retitle to '(monitoring, multi-user)' and add a one-line note that the rarely-changed monitoring-source + container-runtime params live in the same console group (see PARAMETERS.md).
The 'console's group 5 also holds…' line was redundant with the PARAMETERS.md pointer right below it. Group 5 is just the heading + the 3 common params.
…p names The §4 sub-table headings now use the exact console parameter-group labels (1. Network Configuration … 5. Additional Cluster Configuration (Monitoring, Multi-User, Container Runtime)), and group 5 lists PostInstallScriptUrl so the heading and its rows agree.
The default is no longer the awslabs/main GitHub-raw URL — it's empty, which resolves to s3://<S3BucketName>/<S3KeyPrefix>scripts/install-enroot-pyxis.sh. Rewrite the caveat: when testing unpublished templates, point S3BucketName at your own bucket and sync the scripts there, or first-boot Enroot/Pyxis fetch fails (cluster still CREATE_COMPLETE). Link DEPLOY-TESTING.md.
Generalize the s3:// verification note and Test 8 build/deploy commands to use <bucket>/<prefix> placeholders instead of the project's private midaisuk-llm-dev bucket, matching DEPLOY-TESTING.md.
… docs/CUSTOM-AMI.md - IAM.md: add 1-click Deploy buttons to the two-role summary table (matching the deploy table further down) - move the §8.5 step-by-step pre-bake procedure into docs/CUSTOM-AMI.md; README §8.5 keeps a concise summary + Launch button + link (heading text unchanged so the awslabs#85-... anchor referenced elsewhere stays valid)
…single location Match the custom-AMI style: the cluster-admin / cluster-user Deploy buttons now use the launch-stack.svg image, kept in one place (the 'Deploying the policies' table); drop the duplicate kbd buttons from the role summary table.
…uttons Replace the kbd 🚀 buttons in the Templates table with the launch-stack.svg image, consistent with the Quick Start, §8.5 custom-AMI, and IAM policy Deploy buttons.
Turn the two-role bullet list into a table with per-role Launch-stack Deploy buttons, consistent with §9 Templates, §8.5, and docs/IAM.md.
…fy NIC wording, flag CB billing - tests: number the file headings (Tests 11-12 / Test 13 / Test 14) so they line up with the test matrix in tests/README.md - compute-test: '16 cards' → 'the 16 EFA NICs' to avoid confusion with GPU count - README §4: add a CB-billing⚠️ to the CapacityReservationId row (links to GPU compute)
…ope for this doc)
# Conflicts: # architectures/aws-pcs/tests/README.md
DaisukeMiyamoto
added a commit
that referenced
this pull request
Aug 24, 2026
mount-openzfs-home.sh and mount-lustre-fsx.sh run as OnError:TERMINATE lifecycle actions with a single-shot mount. The common first-boot NFS DNS settle race (mount.nfs: Failed to resolve server) fails instantly, which under TERMINATE replaces the node into the same window. Wrap the mount in a bounded retry loop (6 attempts, 10s backoff) verified with mountpoint(8), so TERMINATE fires only on a persistent failure.
DaisukeMiyamoto
added a commit
that referenced
this pull request
Aug 28, 2026
… lifecycle actions (awslabs#1236) * feat(aws-pcs): extract first-boot logic to scripts, harden all boot fetches Phase 1 of the node-lifecycle-actions migration: move the needrestart guard and the FSx OpenZFS//home + Lustre//fsx mounts out of inline cloud-init runcmd into standalone bash scripts under assets/scripts/, fetched from the templates bucket like the existing post-install and directory scripts. All five boot-script fetches now go through a pcs-fetch helper: - 3 attempts 15s apart, every attempt logged to /var/log/pcs-boot-fetch.log, grep-able 'ERROR: failed to fetch' on final failure (a silent directory fetch failure on a replacement login node cost a support round-trip) - pins the AWS CLI to the classic transfer client via a scoped AWS_CONFIG_FILE: on P5/P6 instance types 'auto' resolves to the CRT client, which does not follow S3 region redirects, so cross-region fetches fail (the root cause of post-install exit 127 on GPU CNGs) - re-probes the bucket region each attempt; an empty probe falls back to the classic client's redirect-following instead of failing HTTPS fetches (post-install http(s), monitoring GitHub raw) get curl --retry 3 plus the same ERROR logging. Verified e2e on us-east-2 deploy-all (login + compute): all fetches logged, mounts up, Enroot/Pyxis exit 0, OpenLDAP server+client working, ldap-add-user resolves on both nodes, srun + Pyxis container jobs pass. Failure path verified: nonexistent key exits 1 after 3 logged attempts. * feat(aws-pcs): replace CNG UserData with PCS node lifecycle actions Move ALL first-boot logic from cloud-init UserData to NodeLifecycleActions on the ComputeNodeGroup resource (requires PCS agent >= 1.5.0-1, in PCS-Ready DLAMI builds since 2026-07-20 — see docs/PCS-READY-DLAMI.md). The four CNG templates no longer carry a UserData block at all. nodeBootstrapped (in order, before slurmd starts): 1. needrestart-guard FIRST_BOOT_ONLY / CONTINUE 2. mount-openzfs-home EVERY_BOOT / TERMINATE 3. mount-lustre-fsx EVERY_BOOT / TERMINATE (only when Lustre is set) 4. setup-directory FIRST_BOOT_ONLY / CONTINUE (only when enabled) 5. post-install FIRST_BOOT_ONLY / CONTINUE (only when set) nodeReady: 6. install-monitoring FIRST_BOOT_ONLY / CONTINUE (only when enabled) Why all-at-once instead of piecemeal: nodeBootstrapped runs only after cloud-init user-data completes (pcs_bootstrap_finalize), so any UserData step that depends on a lifecycle-mounted filesystem deadlocks — verified on a real deploy where directory setup waited 10 minutes for a /home mount that could not happen until cloud-init exited. Dependencies must live in one execution domain; within a stage, ordering is guaranteed. Script interface changes (lifecycle actions pass positional args, not env): - setup-directory.sh: role/domain-suffix/cluster-id/bucket/prefix as args 1-5 (env interface kept); generates the admin password itself (SSM reuse logic unchanged); hard-fails unless /home is a mountpoint. - install-enroot-pyxis.sh: accepts the Slurm version as arg 1 (PCS_SLURM_VERSION env kept for the custom-AMI build path). - install-monitoring.sh (new): monitoring installer wrapper — GitHub fetch with curl retry, apt dpkg-lock drop-in, 3-attempt install loop. The agent replaces the hand-rolled fetch plumbing: per-script retries and logs (/var/log/amazon/pcs/lifecycle/actions/<stage>/<name>.log), and its downloader is unaffected by the AWS CLI CRT region-redirect bug that motivated pcs-fetch. Lifecycle config changes via UpdateComputeNodeGroup now reach existing CNGs with DRAIN semantics instead of requiring CNG recreation (the LaunchTemplate Version pin problem). lint-docs.sh: the four-template lock-step check now covers the whole NodeLifecycleActions block, and every referenced lifecycle script must exist in assets/scripts/. Verified e2e on us-east-2 deploy-all (login + compute, directory + monitoring + Enroot/Pyxis enabled, cross-region templates bucket): all six scripts exit 0 in order, /home //fsx mounted, slapd + SSSD up, ldap-add-user resolves on both nodes, Grafana/Prometheus containers running, srun + Pyxis container jobs pass. * docs+fix(aws-pcs): align docs with lifecycle actions; keep space-skip working Template fixes surfaced by the user-impact audit: - PostInstallScriptUrl skip sentinel: deploy-all passes a single space through to add-cng, where the lifecycle ScriptLocation pattern would reject it and fail CNG creation. HasPostInstall/HasPostInstallArgs now treat empty AND single-space as 'no post-install' (verified: a CNG with the skip value creates cleanly with no post-install entry). - AmiId descriptions (4x add-cng + deploy-all + PARAMETERS.md) now state the PCS agent >= 1.5.0-1 floor for pinned/custom AMIs. - PostInstallScriptUrl/Args descriptions document the new contract: https:// only (no plain http), SlurmVersion as first argument, PostInstallScriptArgs as a single unsplit argument. Docs updated for the new mechanics: log paths moved to /var/log/amazon/pcs/lifecycle/actions/<stage>/<name>.log (README diagram, OPERATIONS 2.3/4.1/4.2/4.4/6, PARAMETERS, DEPLOY-TESTING, USER-MANAGEMENT 5.2, tests/infra + storage + readme-walkthrough), mount failures now TERMINATE and replace the node (storage-test troubleshooting), Lustre tuning hooks reference lifecycle actions instead of UserData. Pre-PR e2e on a fresh us-east-2 deploy-all (directory + monitoring + Enroot/Pyxis): all six lifecycle scripts exit 0 on login + compute, no UserData leftovers on nodes, docs commands run as written (executor grep, sinfo, docker ps, enroot/pyxis paths, Prometheus targets up, Grafana 200), srun + Pyxis container job pass, ldap-add-user resolves on both nodes, skip-sentinel CNG attach validated. * feat(aws-pcs)!: replace PostInstallScriptUrl/Args with InstallEnrootPyxis The generic post-install hook existed because PCS had no native way to run a custom script at first boot. Node lifecycle actions ARE that native way now, and the only thing this repo ever shipped through the hook is the Enroot/Pyxis installer — so name the feature for what it does: - InstallEnrootPyxis ('true'/'false') replaces PostInstallScriptUrl + PostInstallScriptArgs on deploy-all and all four add-cng templates. The lifecycle entry is named install-enroot-pyxis (its log follows: .../nodeBootstrapped/install-enroot-pyxis.log). - The script location is fixed to s3://<S3BucketName>/<S3KeyPrefix>scripts/install-enroot-pyxis.sh — dev overrides ride the existing S3BucketName/S3KeyPrefix redirection, which removes the last reason pre-merge deploys had to set a URL. - Defaults preserve prior behavior at both layers: deploy-all 'true' (was: empty auto-installs), modular add-cng 'false' (was: empty skips). Users with custom post-install scripts add them to the compute node group's NodeLifecycleActions directly (documented in the param descriptions, README and PARAMETERS.md). - lint-docs: PostInstallScriptUrl/Args are now BANNED names in docs; the empty-vs-space skip-wording check is gone with the sentinel. BREAKING CHANGE: PostInstallScriptUrl / PostInstallScriptArgs no longer exist. Set InstallEnrootPyxis=false instead of the single-space sentinel; attach custom first-boot scripts as node lifecycle actions. * chore(aws-pcs): publish new lifecycle scripts; lint manifest coverage The publish manifest is an explicit allowlist — without these entries the lifecycle-migration templates would reach the production bucket while their scripts did not, and every post-merge deploy would fail at the agent's script download (the mount scripts TERMINATE, so nodes would enter a replace loop). Add the four new scripts: needrestart-guard.sh, mount-openzfs-home.sh, mount-lustre-fsx.sh, install-monitoring.sh. lint-docs.sh now cross-checks every template-referenced lifecycle script against the manifest so this class of gap fails the publish workflow's lint step instead of production deploys (verified: removing an entry makes the lint FAIL). * docs(aws-pcs): migration notes for the lifecycle-actions changes; final sweep Add OPERATIONS.md §8 documenting what changes for users coming from the UserData-based templates (InstallEnrootPyxis replaces PostInstallScriptUrl/ Args, agent >= 1.5.0-1 floor, new log locations, mount failures now replace the node, installer argument contract) — the repo had no in-tree record of these behavior changes. Sweep of README/docs/tests for remaining UserData-era wording: README architecture diagram now lists all six lifecycle scripts, ROADMAP's FSx-EFA item targets a lifecycle-action script, OPERATIONS 4.2/6.1 and infra/storage-test phrasing updated. * fix(aws-pcs): harden IMDS region lookup in the TERMINATE mount scripts Under set -euo pipefail, REGION=$(curl ...) took curl's exit status, so a connection failure killed the script on the assignment line and skipped the ${REGION:?} diagnostic one line below — on an OnError:TERMINATE action the instance is then replaced before anyone can read the (truncated, error-less) log. And curl -s (no -f) exits 0 on a 4xx/5xx and captures the error body, so under HttpTokens=required a throttled token PUT or a 401 GET landed an HTML/401 body in REGION and passed the guard, producing a bogus FSx DNS name and a mount failure -> terminate. Add -f (4xx/5xx -> empty), --retry/--connect-timeout/--max-time (ride out a throttled/slow IMDS), and || true (reach the :? diagnostic instead of dying on the assignment). Both mount scripts, all three call sites. Verified: a connection failure now exits 1 with the message instead of a silent exit 7. Addresses review batch 1/6 (IMDS guard). * Retry mount in TERMINATE lifecycle scripts (PR awslabs#1236 #2) mount-openzfs-home.sh and mount-lustre-fsx.sh run as OnError:TERMINATE lifecycle actions with a single-shot mount. The common first-boot NFS DNS settle race (mount.nfs: Failed to resolve server) fails instantly, which under TERMINATE replaces the node into the same window. Wrap the mount in a bounded retry loop (6 attempts, 10s backoff) verified with mountpoint(8), so TERMINATE fires only on a persistent failure. * Constrain FSx filesystem ID params with AllowedPattern (PR awslabs#1236 #3) FSxOpenZFSFilesystemId feeds the unconditional mount-openzfs-home action (OnError:TERMINATE) with no AllowedPattern/Default. An empty or malformed value passes stack create, then the mount script fails at boot and the node is TERMINATEd and replaced into the same bad value -- an endless terminate/replace loop. Add a mandatory '^fs-[0-9a-f]{8,17}$' pattern so CFN rejects it at create time. Add the sibling '^$|^fs-...$' pattern to FSxLustreFilesystemId, whose empty value is the documented 'no /fsx' opt-out (HasLustre). Applied identically across all 4 add-cng templates. * Constrain MonitoringRepo/MonitoringVersion with AllowedPattern (PR awslabs#1236 #4) Both feed the raw.githubusercontent.com URL that install-monitoring.sh fetches and bash-runs. These are admin-only params, so the AllowedPattern is input hygiene (catch typos at CFN validation, match the AmiId/FSx-id convention) rather than an anti-exploit measure. MonitoringRepo: ^[A-Za-z0-9._-]+/[A-Za-z0-9._-]+$ (exactly owner/repo). MonitoringVersion: ^[A-Za-z0-9._/-]+$ -- deliberately allows '/' so the documented fork+branch testing workflow (feat/... branches) keeps working; rejects whitespace and shell metacharacters. Applied across all 4 add-cng templates. * Set only the slurmd needrestart override, not the whole hash (PR awslabs#1236 awslabs#5) $nrconf{override_rc} = { qr(^slurmd) => 0 } reassigns the entire override_rc hash, discarding needrestart's shipped defaults and leaving slurmd as the only entry. Mutate the single key instead: $nrconf{override_rc}{qr(^slurmd)} = 0; so the shipped defaults are preserved and only slurmd is added. Pre-existing since awslabs#1165 (ac66712); this PR carries the one-line fix in the extracted script rather than deferring it to a separate PR. * Update OPERATIONS §6.2 needrestart snippet to match the awslabs#5 fix The doc code sample still showed the whole-hash reassignment; align it with the one-key form now written by needrestart-guard.sh. * Document the stack-update upgrade path in OPERATIONS §8 (PR awslabs#1236 M1) Two gaps the review flagged: - §8 said existing stacks keep working as-is but never said the update itself DRAINs and replaces the whole fleet (launch-template version bump + NodeLifecycleActions change). Added a fleet-cycle note. - A custom-bucket stack has none of the four scripts this revision adds; updating it fails mount-openzfs-home (TERMINATE) on every replacement = replace loop to an empty cluster. Added a sync-before-update prerequisite linking DEPLOY-TESTING §2. Also: fixed the mount-row debug advice (a fleet-wide failure leaves no surviving node; point to OnError:STOP_SEQUENCE / configure-cloudwatch-logs.sh), noted the load-bearing S3 read in the PARAMETERS S3BucketName/S3KeyPrefix rows, and framed the DEPLOY-TESTING sync as an update prerequisite. * Remove the unused S3BucketName=local template sentinel (PR awslabs#1236 M2) UseLocalTemplates (S3BucketName='local') switched nested-stack TemplateURLs to relative paths for an aws cloudformation package local-dev flow. It is undocumented, has no reference in docs/tests, and dates to the first reference-cluster commit -- never exercised by this project's workflows, which sync to a real bucket. It is also now a trap: the sentinel is forwarded verbatim to the CNG child stacks as their script bucket, so boot scripts resolve to a literal s3://local/... and fail; with the mounts now load-bearing (OnError:TERMINATE) that terminates the node instead of degrading gracefully. cfn package rewrites nested TemplateURLs but not the runtime script fetch, so 'local' cannot supply boot scripts by design. Delete the condition and collapse all 7 TemplateURLs to the S3 URL form. * Verify LDAP admin bind before writing SSM in setup-directory (PR awslabs#1236 M3) debconf-set-selections seeds slapd's olcRootPW only when apt-get actually installs slapd. If slapd is already present, the install is a no-op and slapd keeps its previous admin password, but the script still overwrote SSM with the newly configured password and swallowed the failing OU-creation binds (2>/dev/null || true) -- a false success leaving a stored credential that does not bind. Add an ldapwhoami check after slapd is up: on bind failure, log loudly and return before creating OUs or overwriting SSM, so a working stored credential is never clobbered by a non-binding one. Directory action is OnError:CONTINUE, so this logs and the node continues. * fix(pcs): match whole fstab line and drop stray mount arg in mount-openzfs-home grep -qF is a substring match: a commented-out /home fstab line (added by an operator debugging a hung mount) satisfies the guard, so the active entry is never re-added. mount -a then no-ops, the script reports success, and the stash is restored onto — then deleted from — a local /home that the next boot silently shadows. Use grep -qxF (whole-line match) on both guards so a commented line no longer counts as present. Also drop the stray 'defaults' positional from 'mount -a -t nfs defaults' (introduced with the mount retry loop); mount -a takes no such argument. * fix(pcs): don't let a failed /fsx chmod terminate a healthy node mount-lustre-fsx.sh writes no fstab entry, so it re-mounts and re-chmods on every boot (EVERY_BOOT). Under set -e a failing 'chmod 1777 /fsx' exits non-zero even when the mount at the previous line succeeded, so OnError: TERMINATE destroys a node whose /fsx is fine. A chmod failure here is a shared-side condition (root_squash / read-only FSx / MDS hiccup), never node-local — terminating just replaces the node into the same failure (launch/drain/replace loop). Decouple the chmod from the exit status and warn loudly instead, keeping the healthy node. Leaves the every-boot chmod itself in place (a safe first-boot-only guard on shared Lustre is non-trivial given .lustre/lost+found); tracked in review reply. * docs(pcs): drop removed empty/single-space InstallEnrootPyxis semantics InstallEnrootPyxis is now AllowedValues ['true','false']; empty and single-space are no longer legal. Rewrote the three passages that still described them (OPERATIONS.md 2.1, CUSTOM-AMI.md, PARAMETERS.md 5.3 preamble), naming the per-template default (deploy-all 'true', add-cng* 'false'). Renamed the PARAMETERS.md 5.3 heading and the README console label to the actual console group 'Container Runtime (Enroot/Pyxis)', restoring the mirror-the-console invariant. The OPERATIONS.md 8 migration row keeps its single-space mention (correct: it explains the old behavior when migrating off PostInstallScript*). * feat(pcs): default InstallEnrootPyxis=true in the standalone add-cng templates The four add-cng*.yaml defaulted InstallEnrootPyxis to 'false' while deploy-all defaults 'true', so the documented one-click path for adding a queue to an existing cluster (README Launch Stack buttons) produced nodes with no container runtime — srun --container-image would fail on the new queue only, while the README says the runtime is on by default. Rather than paper over the split with per-template scoping in the docs, make the default consistent: the container runtime is a headline behavior of this ML reference architecture, and the installer is idempotent (a fast no-op when pre-baked into AmiId). Deploy-all already passes the value down explicitly, so this only changes the standalone add-cng* path. Set 'false' to opt out. Simplifies the docs that had to name the per-template default (OPERATIONS 2.1, CUSTOM-AMI); README's unscoped 'on by default' is now accurate everywhere. All four templates validate; docs lint passes. * docs(pcs): update in-file comments crediting UserData for the moved interface Five comments still described the pre-PR UserData env interface, one made false by this PR's own change (setup-directory.sh:30 said LDAP_ADMIN_PASSWORD is 'auto-generated by UserData' though the script now generates it itself). Swept all five to describe the current interface: the lifecycle action passes values positionally (env vars remain for manual / custom-AMI runs), and SlurmVersion arrives as $1 with PCS_SLURM_VERSION as the fallback. * test(pcs): widen manifest cross-check to scripts fetched by other scripts The manifest guard grepped assets/add-cng*.yaml only, so ldap-add-user.sh — fetched at boot by setup-directory.sh, not via a template ScriptLocation — was invisible to it. Dropping its manifest entry left both lint-docs.sh and the staging script green while the object never reached the production bucket, so the login node's aws s3 cp fails and the cluster comes up without the ldap-add-user.sh helper USER-MANAGEMENT.md documents (silent degrade). Scan assets/*.yaml and assets/scripts/*.sh so boot-time fetches from other scripts are covered too. Verified: normal run still PASS (7/7 present and in the manifest, no false positives); dropping the ldap-add-user.sh entry now FAILs as intended. * fix(pcs): don't silently rotate the LDAP admin password on an SSM read error setup-directory.sh read the stored admin password with 'aws ssm get-parameter ... 2>/dev/null || echo ""', collapsing every failure (ParameterNotFound, AccessDenied, throttling) into "nothing stored yet". A transient read failure during a login-node replacement therefore regenerated the password, reconfigured slapd, and overwrote SSM with --overwrite while the user DB on /home/ldap-db persisted — the exact silent rotation the reuse block exists to prevent. Capture the exit status (set -e-safe if/else) and branch: reuse a returned value; keep the freshly generated password only on rc==0-empty or a genuine ParameterNotFound; on any other error, log loudly and return 1 rather than regenerate against the persistent DB. The directory action is OnError:CONTINUE, so the node continues (log-and-continue), consistent with the ldapwhoami bind check added earlier for the debconf-only-on-fresh-install case. * docs(pcs): drop non-working 'latest' from MonitoringVersion guidance raw.githubusercontent.com resolves only real tag/branch refs, so a MonitoringVersion of 'latest' 404s the post-install.sh fetch and (under OnError:CONTINUE) leaves monitoring silently uninstalled. Remove 'latest' from the parameter Description / ConstraintDescription across all five templates and from PARAMETERS.md; the default stays a real tag (v2.10.2). * fix(pcs): harden mount + monitoring boot scripts, fix doc anchors mount-openzfs-home.sh: keep exactly one active /home fstab entry via ensure_home_fstab_entry (mountpoint-field match, rewrite in place), tolerate rsync exit 23/24 via safe_rsync so a benign ACL/vanished-file code can't trip set -e into TERMINATE, and mount /home specifically instead of mount -a so an unrelated custom-AMI NFS entry can't fail the action. install-monitoring.sh: fetch post-install.sh into a mktemp -d workdir (no fixed /tmp path a dispatched job could race), reconcile the role header (login=server, compute=exporters), note curl --retry doesn't retry HTTP 4xx, scope the apt lock-timeout drop-in comment, and chmod 600 the upstream install log to close the Grafana-password exposure on the multi-user login node. docs: fix the 3.1 cross-file anchor double-hyphen (README, PARAMETERS) and note that under CACHE_ONCE a re-sync needs an instance replacement to reach a running node (DEPLOY-TESTING). * fix(pcs): preserve existing /home dir modes on first-boot restore The first-boot /home restore rsynced the node-local snapshot back over the freshly mounted shared export with `-aA --ignore-existing`. For a directory that already exists on the shared side (e.g. an admin-tightened /home/ubuntu), rsync still applied the snapshot's mode/times to it, silently loosening the shared directory's permissions on the next new node's first boot. Add `--no-perms --omit-dir-times` so the restore only fills in missing files and never rewrites the mode/times of a directory that already exists on the export. `--ignore-existing` still protects existing shared files. (PR awslabs#1236 review #3) Also: - docs(OPERATIONS §8): note the standalone add-cng*.yaml default flip to InstallEnrootPyxis=true (deploy-all already defaulted true) so migrating stacks that relied on the old `false` default aren't surprised. - docs(README): fix the Test 9 cross-file link (heading lives in tests/hpc-efa-test.md, not tests/README.md).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A major update to the AWS PCS reference architecture (
architectures/aws-pcs/). It addsfive user-facing capabilities on top of the existing cluster:
Validated end-to-end on real hardware (hpc6a/hpc8a, p6-b200 ×4) across 10 Regions.
A. New features (direct user benefit)
Each item: what you can do / how it's built / constraints.
A-1. Multi-user support (OpenLDAP)
UID on every node and can run Slurm jobs directly.
The user database lives on FSx OpenZFS
/home./home, it survives a login-node replacement, butthe directory service is a single point of failure — enabling it implies a
single-login-node cluster.
A-2. Multi-AZ VPC / subnets
to match capacity (Capacity Block / reservation) in a specific AZ.
gateway stays in the primary AZ (lowest cost).
A-3. cluster-admin / cluster-user IAM policy stacks
ready-to-deploy CloudFormation stacks.
run
deploy-allto completion (noiam:CreatePolicyneeded — see B-3).password.
A-4. Expanded Region coverage
Region-specific pitfalls.
working storage deployment type and the verification date).
OpenZFS in Osaka) — the fallback value is noted in the table.
A-5. SSH access to the login node
port-forward required).
CIDR; compute nodes stay private. Grafana (443) uses the same mechanism.
B. Supporting changes (needed to make the features above work)
Each item notes why it was needed.
B-1. Parameter cleanup (rename / consolidate / remove)
Why: keep deploy-all's parameter naming and granularity consistent as the new features
(SSH/Grafana access, monitoring) were added. Callers must update (see breaking changes).
DeployMonitoring(bool) →MonitoringStack(enum); behaviour is equivalent.GrafanaPublicAccessCidr→GrafanaAccessCidr(pairs withSSHAccessCidr).OnDemandEnableEfa, consolidating intoOnDemandEfaInterfaceCount(0/1/2).VPCName(the VPC is named from the stack name).B-2. VPC private-subnet CIDR split (for A-2 multi-AZ)
Why: carving out subnets for the additional AZs required splitting the VPC CIDR into
more blocks.
B-3. Inline split of the PCS instance-role permissions (for A-3 IAM)
Why: to let the admin policy run
deploy-allwithoutiam:CreatePolicy, the managedpolicy that required it had to go.
(always-on vs monitoring-only).
B-4. PostInstallScriptUrl fetch method / location (for A-1, A-4)
Why: to distribute the directory setup script and to fetch boot scripts in accounts
that can't use public S3, the source moved to S3 (fetched via the instance role).
s3://scheme support (works with a private bucket, no public S3);http(s)://still works as before.
assets/scripts/, included in the distribution-bucket sync.B-5. Docs / tests reorganization (for all features)
Why: the README grew as features were added; basic flow vs advanced features needed
separating, and parameter drift needed automated detection.
§8; the§4Configuration parameter tables are grouped by — and named after — the deploy-all console
ParameterGroups).
docs/:IAM.md/USER-MANAGEMENT.md/CUSTOM-AMI.md(AMI pre-bake) /
DEPLOY-TESTING.md(deploying unpublished templates from your ownbucket) /
PARAMETERS.md/OPERATIONS.md. Verification records moved intotests/asreproducible procedures.
multi-user / gpu-healthcheck / iam); the README is an index + matrix.
tests/lint-docs.sh: stale/renamed params, undocumentedparams, broken anchors) as pre-merge Test 0.
C. Bug fixes (found while adding the features)
Nametag was hard-coded; now derived from the stack name.execute the shellscript part).
Breaking changes & migration
DeployMonitoring=true/falseMonitoringStack=Prometheus-LoginNode/noneGrafanaPublicAccessCidrGrafanaAccessCidrOnDemandEnableEfa+OnDemandEfaInterfaceCountOnDemandEfaInterfaceCount=0/1/2VPCName/17/18(+ reserved for additional AZs)Validation
ap-south-1 (GPU: p6-b200 ×4, Capacity Block), plus 10 Regions total. Both Slurm 25.05
and 25.11.
SSHAccessCidr, monitoring stack, OpenLDAP server + compute SSSD auto-join, Slurm jobsas an LDAP user with multi-node UID consistency and user-delete propagation, EFA
on/off, CFN lint on every template.
check.
iam:CreatePolicy); SSM-only access as the cluster-user (cannot read the LDAP password).architectures/aws-pcs/tests/.Note for maintainers
The templates fetch the nested CFN templates and boot scripts from a distribution S3
bucket (under
S3BucketName/S3KeyPrefix; boot scripts underassets/scripts/).After merge,
architectures/aws-pcs/assets/(templates +scripts/) must be synced tothe distribution bucket — otherwise nested-stack fetches and node first-boot
(Enroot/Pyxis, directory setup) will fail.