chore: Adds plan-modify logic for replication specs #3223
Conversation
…BER_REPLICATION_LAG`
internal/service/advancedclustertpf/model_to_ClusterDescription20240805.go
Outdated
Show resolved
Hide resolved
| @@ -167,13 +173,34 @@ func ensureSpecRespectChanges(ctx context.Context, spec *TFSpecsModel, req *cust | |||
| } | |||
|
|
|||
| func replicationSpecsKeepUnknownWhenChanged(ctx context.Context, state attr.Value, req *customplanmodifier.UnknownReplacementRequest[PlanModifyResourceInfo]) []string { | |||
There was a problem hiding this comment.
q: how is the process of calling replicationSpecsKeepUnknownWhenChanged and the other funcs e.g. for electable_specs and read_only_specs, etc.?
to have a better understading of all the rules/logic we're applying to unkown values
There was a problem hiding this comment.
Good point. Added extra docs in efed2ea
…UDP-308783_plan_modify_replication_specs
…d of global failure
…cestor path behavior
…ry diagnostics parameter and enhance logging for error handling
…ements function, the diff will always return a single level of unknown
…UDP-308783_plan_modify_replication_specs
| keepUnknowns = append(keepUnknowns, "id") // When using new sharding config, the legacy id must never be copied | ||
| } | ||
| // for isShardingConfigUpgrade, it will be empty in the plan, so we need to keep it unknown | ||
| // for listLenChanges, it might be an insertion in the middle of replication spec leading to wrong value from state copied |
There was a problem hiding this comment.
Would note that API does support sending the ids as they are stored in the state, even when adding a replication spec in the middle. Not sure if this can help plan modifier or reduce some complexity (thinking more long term).
There was a problem hiding this comment.
Not sure I understand correctly.
This handles the case of not using a state value for the legacy id field used by the old API when the new sharding config is used.
There was a problem hiding this comment.
Yes sorry, I was referring to the logic of not sending external_id if req.Changes.ListLenChanged(rootPath)
There was a problem hiding this comment.
Ahh ok. I think it is safer to not send these when a replication_spec is added/removed. But no strong opinion
|
This PR has gone 7 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 7 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy! |
|
This PR has gone 7 days without any activity and meets the project’s definition of "stale". This will be auto-closed if there is no new activity over the next 7 days. If the issue is still relevant and active, you can simply comment with a "bump" to keep it open, or add the label "not_stale". Thanks for keeping our repository healthy! |
Description
Link to any related issue(s): CLOUDP-308783
Type of change:
Required Checklist:
Further comments