perf: pre-compile templates, hoist regex, filter DescribeLoadBalancers#652
Merged
ArangoGutierrez merged 1 commit intoNVIDIA:mainfrom Feb 13, 2026
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements performance optimizations based on audit findings by pre-compiling templates, hoisting regex compilation, and filtering AWS API calls. The changes eliminate redundant template and regex compilations that were previously happening on every execution, and reduce the scope of AWS DescribeLoadBalancers calls by filtering by name instead of listing all load balancers in the account.
Changes:
- Pre-compiled text/templates moved to package-level variables in provisioner and template files
- Regex pattern compilation hoisted to package level in gitref resolver
- AWS DescribeLoadBalancers call now filters by load balancer name
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/provisioner/templates/kubernetes.go | Added package-level template variables for kubeadm (release/git/latest), kind (release/git/latest), and microk8s templates |
| pkg/provisioner/templates/kubeadm_cluster.go | Added package-level template variables for kubeadm init, join, and prereq templates |
| pkg/provisioner/templates/containerd.go | Added package-level template variables for containerd v1 and v2 templates |
| pkg/provisioner/templates/container-toolkit.go | Added package-level template variables for container toolkit package, git, and latest templates |
| pkg/provisioner/provisioner.go | Added package-level template variables for shebang and common functions templates |
| pkg/provider/aws/delete.go | Modified DescribeLoadBalancers to filter by name instead of listing all load balancers |
| pkg/gitref/resolver.go | Hoisted regex compilation to package-level variable |
Comments suppressed due to low confidence (1)
pkg/provider/aws/delete.go:74
- The DescribeLoadBalancers call with a specific name will return a LoadBalancerNotFoundException error if the load balancer doesn't exist. This should be handled gracefully since the load balancer may have been manually deleted or failed to create. Consider checking for this error type and treating it as a success case (load balancer already gone), similar to how other resource deletions handle NotFound errors elsewhere in this file (lines 177, 270, 345, etc.).
describeOutput, err := p.elbv2.DescribeLoadBalancers(ctx, describeInput)
if err != nil {
return fmt.Errorf("error describing load balancers: %w", err)
}
e9c4579 to
d0b6398
Compare
Templates were recompiled on every Execute call (75+ times for a 15-node cluster). Move to package-level vars. Move ParseRepoURL regex to package level. Filter DescribeLoadBalancers by name instead of listing all LBs in the account. Audit findings NVIDIA#22 (MEDIUM), NVIDIA#23 (MEDIUM), NVIDIA#24 (MEDIUM). Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
d0b6398 to
084fc0d
Compare
Pull Request Test Coverage Report for Build 21997438359Details
💛 - Coveralls |
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
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
regexp.MustCompilefrom function body to package-level var in resolver.goDescribeLoadBalancersby name instead of listing all LBs in the accountAudit Findings
Changes
pkg/provisioner/provisioner.go: Package-level template vars for addScriptHeaderpkg/provisioner/templates/*.go: Package-level template vars for Execute methodspkg/gitref/resolver.go: Package-level regex varpkg/provider/aws/delete.go: Filter DescribeLoadBalancers by nameTest plan
gofmt— no formatting issuesgo build— compilesgo test ./pkg/...— all tests pass