Skip to content

[nexus] Separate action to chain instance-updates#6630

Merged
hawkw merged 4 commits into
mainfrom
eliza/siu-chain-successor
Sep 23, 2024
Merged

[nexus] Separate action to chain instance-updates#6630
hawkw merged 4 commits into
mainfrom
eliza/siu-chain-successor

Conversation

@hawkw

@hawkw hawkw commented Sep 21, 2024

Copy link
Copy Markdown
Member

Currently, the instance-update saga's siu_commit_instance_updates saga attempts to check if an additional update saga is needed and start one if so. This is done in the same action that writes back the updated instance record. In this comment on PR #6503, @davepacheco pointed out that conflating these two operations makes the idempotency tests for the update saga less effective, since the action performs multiple operations (even if some of them are permitted to fail). This commit refactors the instance-update saga so that the chaining operation is performed in a separate action from the rest of the saga.

There should be no functional change to the saga's behavior as a result of this, beyond making the idempotency tests more correct.

Currently, the `instance-update` saga's `siu_commit_instance_updates`
saga attempts to check if an additional update saga is needed and start
one if so. This is done in the same action that writes back the updated
instance record. In [this comment][1] on PR #6503, @davepacheco pointed
out that conflating these two operations makes the idempotency tests for
the update saga less effective, since the action performs multiple
operations (even if some of them are permitted to fail). This commit
refactors the instance-update saga so that the chaining operation is
performed in a separate action from the rest of the saga.

There should be no functional change to the saga's behavior as a result
of this, beyond making the idempotency tests more correct.

[1]: #6503 (comment)
@hawkw

hawkw commented Sep 23, 2024

Copy link
Copy Markdown
Member Author

helios / build TUF repo CI failure looks like it was a transient network connectivity issue; will re-run it.

Comment thread nexus/src/app/sagas/instance_update/mod.rs Outdated
Co-authored-by: Greg Colombo <greg@oxidecomputer.com>
@hawkw hawkw enabled auto-merge (squash) September 23, 2024 16:40
@hawkw hawkw disabled auto-merge September 23, 2024 17:04
; Conflicts:
;	nexus/src/app/sagas/instance_update/mod.rs
@hawkw hawkw enabled auto-merge (squash) September 23, 2024 18:29
@hawkw hawkw merged commit f4b29bb into main Sep 23, 2024
@hawkw hawkw deleted the eliza/siu-chain-successor branch September 23, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants