fix(core): idempotency pre-check сравнивает с live, а не с paramValue формы
Проблема: modifierDesiredEqualsCurrent сравнивает desired с paramValue из cfsParams (дефолт ФОРМЫ операции), а не с состоянием инстанса. Пропуск modify на такой основе может быть ложным (HAR/edge_.har: needEnableAVI paramValue=false при live=true). - core/modifier_compare.go: добавлен modifierDesiredEqualsLive (источник — instanceLiveParams/state.params; неопределённость => не пропускаем) и хелперы modifierValuesEqual / modifierCodeMap; modifierDesiredEqualsCurrent переведён на них. - core/operation_run_bycode.go: idempotent-путь использует live-pre-check; при ошибке чтения live modify НЕ пропускается. - resources_core/nsxt_snat_resource.go: setSnat -> RunInstanceOperationUniversalByIdempotent (лишний modify на повторном apply больше не отправляется). - modifier_compare_test.go: TestModifierDesiredEqualsLive (совпало/отличается/live недоступен). Проверено: go build ./... OK; go test ./internal/core/... -short -> PASS.
This commit is contained in:
Repinoid
@@ -1,6 +1,7 @@
|
||||
package core
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
|
||||
"terraform-provider-nubes/internal/core/jsonutil"
|
||||
@@ -19,15 +20,7 @@ func (c *UniversalClient) modifierDesiredEqualsCurrent(desired map[string]string
|
||||
return false
|
||||
}
|
||||
|
||||
codeToParam := make(map[string]universalCfsParam)
|
||||
for _, p := range cfsParams {
|
||||
if key := strings.ToLower(strings.TrimSpace(p.Code)); key != "" {
|
||||
codeToParam[key] = p
|
||||
}
|
||||
if key := strings.ToLower(strings.TrimSpace(p.SvcOperationCfsParam)); key != "" {
|
||||
codeToParam[key] = p
|
||||
}
|
||||
}
|
||||
codeToParam := modifierCodeMap(cfsParams)
|
||||
|
||||
for code, wanted := range desired {
|
||||
param, ok := codeToParam[strings.ToLower(strings.TrimSpace(code))]
|
||||
@@ -41,28 +34,75 @@ func (c *UniversalClient) modifierDesiredEqualsCurrent(desired map[string]string
|
||||
current = *param.ParamValue
|
||||
}
|
||||
|
||||
dataType := strings.ToLower(strings.TrimSpace(param.DataType))
|
||||
if strings.HasPrefix(dataType, "array") {
|
||||
// array-map-fixed: сравнивать сырые значения как JSON.
|
||||
if !jsonutil.JSONStringsEquivalent(wanted, current) {
|
||||
return false
|
||||
}
|
||||
continue
|
||||
}
|
||||
if strings.HasPrefix(dataType, "map") || strings.Contains(dataType, "json") {
|
||||
if !jsonutil.JSONStringsEquivalent(wanted, current) {
|
||||
return false
|
||||
}
|
||||
continue
|
||||
}
|
||||
|
||||
// Скаляры: нормализуем обе стороны.
|
||||
nw := normalizeUniversalValueV6(wanted, param)
|
||||
nc := normalizeUniversalValueV6(current, param)
|
||||
if nw != nc {
|
||||
if !modifierValuesEqual(wanted, current, param) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
// modifierValuesEqual сравнивает одно значение с учётом типа параметра:
|
||||
// - array*/map*/json — смысловое JSON-сравнение (порядок ключей не значим);
|
||||
// - скаляры — нормализация через normalizeUniversalValueV6 и сравнение строками.
|
||||
func modifierValuesEqual(wanted, current string, param universalCfsParam) bool {
|
||||
dataType := strings.ToLower(strings.TrimSpace(param.DataType))
|
||||
if strings.HasPrefix(dataType, "array") ||
|
||||
strings.HasPrefix(dataType, "map") ||
|
||||
strings.Contains(dataType, "json") {
|
||||
return jsonutil.JSONStringsEquivalent(wanted, current)
|
||||
}
|
||||
return normalizeUniversalValueV6(wanted, param) == normalizeUniversalValueV6(current, param)
|
||||
}
|
||||
|
||||
// modifierDesiredEqualsLive — idempotency-pre-check по ЖИВЫМ параметрам инстанса
|
||||
// (state.params), а не по paramValue формы операции.
|
||||
//
|
||||
// Зачем: paramValue из cfsParams — это дефолт ФОРМЫ операции, а не состояние
|
||||
// инстанса (см. instanceLiveParams, HAR/edge_.har). Пропуск modify на основе
|
||||
// формы может быть ложным. Здесь источник — live.
|
||||
//
|
||||
// (true, nil) возвращается только если ВСЕ desired-поля найдены в live и совпали.
|
||||
// Любая неопределённость (пусто, поле не найдено, ошибка) → (false, …): modify
|
||||
// лучше выполнить лишний раз, чем пропустить нужный.
|
||||
func (c *UniversalClient) modifierDesiredEqualsLive(ctx context.Context, instanceUid string, desired map[string]string, cfsParams []universalCfsParam) (bool, error) {
|
||||
if len(desired) == 0 {
|
||||
return false, nil
|
||||
}
|
||||
live, err := c.instanceLiveParams(ctx, instanceUid)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
if len(live) == 0 {
|
||||
return false, nil
|
||||
}
|
||||
codeToParam := modifierCodeMap(cfsParams)
|
||||
for code, wanted := range desired {
|
||||
param, ok := codeToParam[strings.ToLower(strings.TrimSpace(code))]
|
||||
if !ok {
|
||||
return false, nil
|
||||
}
|
||||
current, ok := lookupLiveParam(live, param)
|
||||
if !ok {
|
||||
return false, nil
|
||||
}
|
||||
if !modifierValuesEqual(wanted, current, param) {
|
||||
return false, nil
|
||||
}
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// modifierCodeMap индексирует cfsParams по нижнему регистру Code и SvcOperationCfsParam.
|
||||
func modifierCodeMap(cfsParams []universalCfsParam) map[string]universalCfsParam {
|
||||
m := make(map[string]universalCfsParam, len(cfsParams)*2)
|
||||
for _, p := range cfsParams {
|
||||
if key := strings.ToLower(strings.TrimSpace(p.Code)); key != "" {
|
||||
m[key] = p
|
||||
}
|
||||
if key := strings.ToLower(strings.TrimSpace(p.SvcOperationCfsParam)); key != "" {
|
||||
m[key] = p
|
||||
}
|
||||
}
|
||||
return m
|
||||
}
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
package core
|
||||
|
||||
import "testing"
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestModifierDesiredEqualsCurrent_Scalars(t *testing.T) {
|
||||
c := &UniversalClient{}
|
||||
@@ -61,3 +65,54 @@ func TestModifierDesiredEqualsCurrent_ArrayPreservesOrder(t *testing.T) {
|
||||
t.Errorf("array-map-fixed с другим порядком не должен быть равен")
|
||||
}
|
||||
}
|
||||
|
||||
func TestModifierDesiredEqualsLive(t *testing.T) {
|
||||
cfsParams := []universalCfsParam{
|
||||
{SvcOperationCfsParamId: 372, Code: "ipSpaceName", DataType: "string"},
|
||||
}
|
||||
|
||||
newClient := func(liveParams string) (*UniversalClient, func()) {
|
||||
return makeTestClient(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
_, _ = w.Write([]byte(`{"instance":{"state":{"params":` + liveParams + `}}}`))
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("совпало с live", func(t *testing.T) {
|
||||
c, cleanup := newClient(`{"ipSpaceName":"internet-ipv4-v1"}`)
|
||||
defer cleanup()
|
||||
equal, err := c.modifierDesiredEqualsLive(context.Background(), "inst-1", map[string]string{"ipSpaceName": "internet-ipv4-v1"}, cfsParams)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected err: %v", err)
|
||||
}
|
||||
if !equal {
|
||||
t.Errorf("expected equal when live matches desired")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("отличается от live", func(t *testing.T) {
|
||||
c, cleanup := newClient(`{"ipSpaceName":"no-needed"}`)
|
||||
defer cleanup()
|
||||
equal, err := c.modifierDesiredEqualsLive(context.Background(), "inst-1", map[string]string{"ipSpaceName": "internet-ipv4-v1"}, cfsParams)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected err: %v", err)
|
||||
}
|
||||
if equal {
|
||||
t.Errorf("expected not equal when live differs from desired")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("live недоступен — ошибка, не пропускаем", func(t *testing.T) {
|
||||
c, cleanup := makeTestClient(func(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
})
|
||||
defer cleanup()
|
||||
equal, err := c.modifierDesiredEqualsLive(context.Background(), "inst-1", map[string]string{"ipSpaceName": "internet-ipv4-v1"}, cfsParams)
|
||||
if err == nil {
|
||||
t.Errorf("expected error when live is unavailable")
|
||||
}
|
||||
if equal {
|
||||
t.Errorf("must not report equal when live is unavailable")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -60,10 +60,17 @@ func (c *UniversalClient) runInstanceOperationByCode(ctx context.Context, instan
|
||||
return err
|
||||
}
|
||||
|
||||
// Idempotency pre-check: если все desired уже равны live-значениям — пропускаем run.
|
||||
// desired = явно заданные пользователем коды (params, keyed by code), БЕЗ досылки.
|
||||
if idempotent && c.modifierDesiredEqualsCurrent(params, cfsParams) {
|
||||
return nil
|
||||
// Idempotency pre-check: если все desired уже равны LIVE-значениям инстанса —
|
||||
// пропускаем run. Источник сравнения — state.params (live), НЕ paramValue формы
|
||||
// операции: форма может не совпадать с состоянием (см. instanceLiveParams).
|
||||
// желаемые = явно заданные пользователем коды (params, keyed by code), БЕЗ досылки.
|
||||
if idempotent {
|
||||
equal, liveErr := c.modifierDesiredEqualsLive(ctx, instanceUid, params, cfsParams)
|
||||
if liveErr == nil && equal {
|
||||
return nil
|
||||
}
|
||||
// Ошибка/несовпадение live — не пропускаем: выполняем modify. Если live
|
||||
// действительно недоступен, следующий шаг вернёт ошибку явно (не молча).
|
||||
}
|
||||
|
||||
codeToParam := make(map[string]universalCfsParam)
|
||||
|
||||
@@ -250,9 +250,10 @@ func (r *NsxtSnatResource) setSnat(ctx context.Context, nsxtUID types.String, ip
|
||||
unlock := r.client.LockInstance(uid)
|
||||
defer unlock()
|
||||
|
||||
// ByCode, а не ByIdempotent: idempotency-сравнение идёт с paramValue ФОРМЫ операции,
|
||||
// а не с live-состоянием инстанса — можно ложно пропустить modify.
|
||||
return r.client.RunInstanceOperationUniversalByCode(ctx, uid, "modify", map[string]string{
|
||||
// Idempotent: core.RunInstanceOperationUniversalByIdempotent выполняет pre-check
|
||||
// по LIVE-значениям инстанса (state.params) и пропускает modify, если
|
||||
// ipSpaceName уже совпадает — повторный apply не дёргает платформу зря.
|
||||
return r.client.RunInstanceOperationUniversalByIdempotent(ctx, uid, "modify", map[string]string{
|
||||
"ipSpaceName": value,
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user