Repository navigation
Print cloudformation template diff on every update - #7685
Conversation
…r, and their diff, on update and update dryrun On every update-cluster, and on every update-cluster dryrun, log at INFO, for the cluster stack and each of its nested stacks: the deployed template, the new template and the unified diff between them. The messages start with [update] or [update dryrun]. * The dryrun builds the templates as the update does, but uploads nothing to the cluster bucket. * The templates are logged as compact JSON, in parts of at most 150000 characters, so that each message stays under the 256 KB limit of a CloudWatch Logs event when logged by the API Lambda. * The query strings of presigned S3 URLs are redacted. * A failure to read a deployed template is logged as a warning: the update goes on.
… update dryrun Log, for each stack, only the diff between the deployed and new templates. The new template is logged instead when there is no deployed template to compare with: a new nested stack, or a deployed template that cannot be read.
…best-effort step Keep the update dryrun as it was, validating the request, and only add a call that logs the diff between the deployed templates and the ones generated from the target config. The step uploads nothing, leaves the cluster unchanged and logs any failure as a warning, so that it does not change the outcome of the dryrun.
…on templates diff Log the templates diff of an update as a best-effort step, as for the update dryrun: any failure is logged as a warning and the update goes on.
…nes cannot be read When a deployed template cannot be read, only log a warning instead of the new templates: a policy that denies GetTemplate would otherwise make every update log every new template in full. The new template of a new nested stack is still logged. Make the size of the logged parts private, and add the changelog entry.
hgreebe
changed the title
| try: | ||
| self._add_tags(target_config) | ||
| template_body, assets_metadata = CDKTemplateBuilder().build_cluster_template( | ||
| cluster_config=target_config, | ||
| bucket=self.bucket, | ||
| stack_name=self.stack_name, | ||
| log_group_name=self.stack.log_group_name, | ||
| upload_assets=False, | ||
| ) | ||
| except Exception as e: | ||
| LOGGER.warning("Unable to log the CloudFormation templates of the %s: %s", operation, e) | ||
| return | ||
| self._log_templates_best_effort(template_body, assets_metadata, operation) |
There was a problem hiding this comment.
How about making the code simpler and by removing _log_templates_best_effort and use _log_templates directly?:
try:
self._add_tags(target_config)
template_body, assets_metadata = CDKTemplateBuilder().build_cluster_template(
cluster_config=target_config,
bucket=self.bucket,
stack_name=self.stack_name,
log_group_name=self.stack.log_group_name,
upload_assets=False,
)
self._log_templates(template_body, assets_metadata, operation)
except Exception as e:
LOGGER.warning("Unable to log the CloudFormation templates of the %s: %s", operation, e)
return
| def _stack_resources(self) -> dict: | ||
| """Return the resources of the cluster stack by logical ID, or none, with a warning, if they cannot be read.""" | ||
| try: | ||
| return AWSApi.instance().cfn.describe_stack_resources(self.stack_name) |
There was a problem hiding this comment.
Only the first 100 resources will be returned. If your stack has more resources than this, you should use ListStackResources instead.
Do we always have fewer than 100 resources?
There was a problem hiding this comment.
We only need the physical ID of each nested stack, so I now look each one up with DescribeStackResources(StackName, LogicalResourceId): the response holds just that resource, so the 100 limit can't drop it. I didn't use ListStackResources because it would need cloudformation:ListStackResources, which isn't in the ParallelCluster policies; this keeps the same action. The cost is one call per deployed nested stack.
|
|
||
|
|
||
| def _parse_template(template_body): | ||
| """Return the template as a dict: GetTemplate returns a JSON template already parsed, and a YAML one as text.""" |
There was a problem hiding this comment.
Do we really touch the YAML text path?
There was a problem hiding this comment.
added note: The template of the cluster stack is uploaded as YAML (see S3Bucket.upload_cfn_template), so it is text; the
templates of the nested stacks are uploaded as JSON, so they are already parsed.
|
|
||
| # Maximum characters of a template logged in one message. Templates are logged in parts so that each message, once | ||
| # escaped in a JSON log event (e.g. by the API Lambda logger), stays under the 256 KB limit of a CloudWatch Logs event. | ||
| _TEMPLATE_LOG_PART_SIZE = 150_000 |
There was a problem hiding this comment.
Let's shrink this by 10% to allow some space for escape characters
| deployed_resources = deployed_template.get("Resources") or {} | ||
| stack_resources = None | ||
| for logical_id, resource in (template_body.get("Resources") or {}).items(): | ||
| if resource.get("Type") != _NESTED_STACK_RESOURCE_TYPE: |
There was a problem hiding this comment.
[INFO] the code only handles one level of nesting. That's fine for the ParallelCluster today,
There was a problem hiding this comment.
Added a note in the doctring
…ne at a time, shrink the log part size
Things to note:
Description of changes
pcluster update-cluster, and everyupdate-cluster --dryrun, log the diff between the deployed CloudFormation templates of the cluster and the new ones, for the cluster stack and each of its nested stacks (e.g. Queues, LoginNodes). This shows what an update changes in the stacks, which the config change set alone does not, e.g. resources that change because of values evaluated at update time.~/.parallelcluster/pcluster-cli.logfor the CLI and in the CloudWatch log group of the API Lambda for the API. They start with[update]or[update dryrun]:[update] Diff of the CloudFormation template of the cluster stack:followed by a unified diff (--- deployed …/+++ new …), orNo change.[update] Diff of the CloudFormation template of the nested stack <logical id>:for each nested stack.ConfigVersion,cluster_config_version,instance_types_data_versionare empty).GetTemplate, and the nested stacks are found withDescribeStackResources; both are already allowed by the ParallelCluster policies. A failure to read them is logged as a warning and does not prevent the update.(part i/n)), so that each one stays under the 256 KB limit of a CloudWatch Logs event once escaped by the API Lambda logger.ConfigVersion, the config versions in the launch templates, and the renamed head node wait condition.Example:
API diff not very readable unless read with jq -r:
Tests
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.