Skip to content

Add full context propagation and proper interrupt handling#675

Merged
adrianriobo merged 1 commit into
redhat-developer:mainfrom
jangel97:feature/cancel-on-sigint-630
Dec 5, 2025
Merged

Add full context propagation and proper interrupt handling#675
adrianriobo merged 1 commit into
redhat-developer:mainfrom
jangel97:feature/cancel-on-sigint-630

Conversation

@jangel97
Copy link
Copy Markdown
Contributor

@jangel97 jangel97 commented Dec 2, 2025

This MR adds full context propagation to MAPT so that interrupts (SIGINT/SIGTERM, e.g., Ctrl+C) cleanly cancel ongoing Pulumi tasks instead of leaving them running in the background. This completes the fix for #630.

Changes

  • CLI now creates a cancellable root context.
  • Added unified signal handling for SIGINT/SIGTERM.
  • All manager stack functions now require ctx.
  • All AWS/Azure provider operations updated to forward that context.
  • Replaced all remaining context.Background() usages.

Why

Previously, interrupting MAPT didn’t cancel the underlying Pulumi task, often leaving partial resources and inconsistent state. Cancellation is now fully propagated to Pulumi, ensuring immediate, deterministic shutdown.

Testing

  • Verified Ctrl+C cancels aws fedora create mid-update.
    Command used:

    out/mapt aws fedora create \
  --project-name aipcc-6664-2 \
  --backed-url file:///$PWD/state \
  --conn-details-output $PWD/outputs \
  --compute-sizes g5.xlarge,g5.2xlarge,g4dn.xlarge \
  --cpus 1 \
  --memory 8 \
  --spot-excluded-regions us-east-1a,us-east-1b,us-east-1c,us-east-1d,us-east-1e,us-east-1f,us-east-2a,us-east-2b,us-east-2c

  • Pulumi logs reflect clean cancellation events.

  • Destroy operations behave correctly after interruption.

Issue

Fixes #630.

@jangel97 jangel97 marked this pull request as draft December 2, 2025 23:40
@jangel97 jangel97 force-pushed the feature/cancel-on-sigint-630 branch 2 times, most recently from 6a7420f to f57c9e3 Compare December 3, 2025 00:26
@adrianriobo adrianriobo changed the title Add full context propagation and proper interrupt handling WIP: Add full context propagation and proper interrupt handling Dec 3, 2025
@jangel97 jangel97 force-pushed the feature/cancel-on-sigint-630 branch from f57c9e3 to f73701e Compare December 3, 2025 09:44
adrianriobo
adrianriobo reviewed Dec 3, 2025
View reviewed changes
Comment thread cmd/mapt/cmd/aws/hosts/fedora.go Outdated
return err
}
return fedora.Create(
cmd.Context(),
Copy link
Copy Markdown
Collaborator

@adrianriobo adrianriobo Dec 3, 2025

Choose a reason for hiding this comment

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

I know it is draft but if you are moving this all over, may would it make sense to make it part of the maptContext?

@jangel97 jangel97 force-pushed the feature/cancel-on-sigint-630 branch 4 times, most recently from c82ccc3 to 1598095 Compare December 3, 2025 18:09
@jangel97 jangel97 marked this pull request as ready for review December 3, 2025 18:18
@jangel97 jangel97 changed the title WIP: Add full context propagation and proper interrupt handling Add full context propagation and proper interrupt handling Dec 3, 2025
adrianriobo
adrianriobo reviewed Dec 4, 2025
View reviewed changes
Comment thread cmd/mapt/cmd/aws/hosts/fedora.go Outdated
@jangel97 jangel97 force-pushed the feature/cancel-on-sigint-630 branch 4 times, most recently from 668edfc to 10cec98 Compare December 4, 2025 11:49
adrianriobo
adrianriobo reviewed Dec 4, 2025
View reviewed changes
Comment thread pkg/provider/aws/action/eks/eks.go
return v.Struct(r)
}

func Create(mCtxArgs *mc.ContextArgs, args *EKSArgs) (err error) {
Copy link
Copy Markdown
Collaborator

@adrianriobo adrianriobo Dec 4, 2025

Choose a reason for hiding this comment

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

I am not against rename, but if you do not rename this...all these files will not be changed on this PR right?

Copy link
Copy Markdown
Collaborator

@adrianriobo adrianriobo Dec 4, 2025

Choose a reason for hiding this comment

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

Well not all, actually the ekf.go it would require 1 change but bunch of them will not change if you just kept the original name mcCtxArgs no?

@jangel97 jangel97 force-pushed the feature/cancel-on-sigint-630 branch from 10cec98 to 90d96fc Compare December 4, 2025 13:32
@jangel97
feat(context): complete end-to-end context propagation for SIGINT/SIG…
d953f56 
…TERM cancellation

This refactor removes all hard-coded context.Background() / context.TODO()
calls and ensures the CLI's cancellable context (cmd.Context()) flows
through every layer of MAPT:

• CLI -> ContextArgs -> mc.Init() -> Providers -> Actions -> Data layer
• AWS: AMI queries, region/zone discovery, S3 ops, Spot pricing, host waiters
• Azure: compute selectors, image discovery, ResourceGraph queries
• Pulumi: stack create/update/destroy, previews, refresh, output retrieval

Key changes:
- Add `Context` to ContextArgs and centralize fallback handling in mc.Init()
- Update Provider interface so Init(ctx) receives the real execution context
- Rename parameters for clarity (e.g., ca -> contextArgs)
- Replace all Background()/TODO() calls with propagated context
- Fix async loops and pagers to derive from parent context
- Ensure cancellations interrupt in-flight SDK calls and Pulumi operations

No functional logic changes besides enabling proper cancellation.

This ensures Ctrl+C properly cancels in-flight operations while
supporting library consumers who need custom context management.

Signed-off-by: Jose Angel Morena <jmorenas@redhat.com>
@jangel97 jangel97 force-pushed the feature/cancel-on-sigint-630 branch from 90d96fc to d953f56 Compare December 4, 2025 13:57
@adrianriobo
Copy link
Copy Markdown
Collaborator

adrianriobo commented Dec 5, 2025

Hey got one question did you try this out, I mean what is status for state file? if ctrl+c if you try to destroy them afterwards is it locked?

@jangel97
Copy link
Copy Markdown
Contributor Author

jangel97 commented Dec 5, 2025

Hey got one question did you try this out, I mean what is status for state file? if ctrl+c if you try to destroy them afterwards is it locked?

Hey @adrianriobo,

Here is the log when mapt is cancelled in the middle of a cmd create operation:
https://gist.github.com/jangel97/d37ee3c548c7296225786cf268adadb7

This demonstrates how a create behaves when interrupted with SIGINT (Ctrl-C).
The first Ctrl-C triggers Pulumi’s graceful cancellation path, and the second forces termination.
Pulumi reports update cancelled and marks the update as failed, but it still completes its shutdown
sequence and writes state correctly before exiting.

And here is the log when running mapt destroy:
https://gist.github.com/jangel97/76c0636b9cd587f3852d46123d31e7e1

Destroy runs normally. Pulumi emits the standard “pending operations” warning from the previously
interrupted update, but there are no lock issues.

adrianriobo
adrianriobo approved these changes Dec 5, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@adrianriobo adrianriobo left a comment

Choose a reason for hiding this comment

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

LGTM

@adrianriobo adrianriobo merged commit cd3e65f into redhat-developer:main Dec 5, 2025
7 checks passed
@adrianriobo adrianriobo mentioned this pull request Dec 5, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adrianriobo adrianriobo adrianriobo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jangel97 @adrianriobo