Skip to content

Conversation

@jbardin
Copy link
Member

@jbardin jbardin commented Oct 28, 2025

Once we have a resource instance we no longer need to have all possible forceReplace addresses, we can determine immediately if the instance address is in the set of replacement addresses. This avoids bugs caused by sharing the -replace flag addresses across all resource nodes.

After working out a regression test for this, it became apparent that the test was not useful at all. It's not possible to regress this fix if the shared slice no longer exists, and even then relies on Go implementation details to get the slice len and cap to line up for the sharing to happen, so it may never fail again in the future. Since no behavior changes have been made, the existing test coverage should be sufficient.

Fixes #37831

Once we have a resource instance we no longer need to have all possible
forceReplace addresses available within that instance. We can determine
immediately if the instance address is in the set if replacement
addresses. This avoids bugs caused by sharing the `-replace` flag
addresses across all resource nodes.
@jbardin jbardin force-pushed the jbardin/force-replace-instance branch from b3140f9 to cac484d Compare October 29, 2025 21:49
@jbardin jbardin changed the title don't carry all replace address through instances Don't carry all -replace addresses through to every instance Oct 29, 2025
@jbardin jbardin added the 1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Oct 29, 2025
@jbardin jbardin marked this pull request as ready for review October 29, 2025 22:01
@jbardin jbardin requested a review from a team as a code owner October 29, 2025 22:01
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

very nice!

@jbardin jbardin merged commit f8ae45c into main Oct 30, 2025
18 of 19 checks passed
@jbardin jbardin deleted the jbardin/force-replace-instance branch October 30, 2025 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.14-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect plan when using replace_triggered_by with -replace

3 participants