Skip to content

Commit 0de5f5e

Browse files
alexottclaude
andauthored
Fix spurious diffs in model serving resources due to API ordering (#5188)
## Changes <!-- Summary of your changes that are easy to understand --> The model serving API returns served_models and served_entities in non-deterministic order (typically alphabetically by name), but Terraform treats them as ordered lists. This caused spurious diffs when the API returned items in a different order than specified in the HCL configuration. Solution: - Added reorderByName() generic helper function to re-order API response slices to match the user's HCL configuration order - Updated preserveConfigOrder() to use the helper for both served_models and served_entities - Updated preserveConfigOrderPt() for provisioned throughput endpoints - Added comprehensive unit tests with 11 test cases The fix preserves user intent by maintaining the order specified in the configuration while handling the API's non-deterministic behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [x] `make test` run locally - [ ] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [ ] using Go SDK - [ ] using TF Plugin Framework - [x] has entry in `NEXT_CHANGELOG.md` file Co-authored-by: Claude <[email protected]>
1 parent 5b6b519 commit 0de5f5e

File tree

4 files changed

+283
-0
lines changed

4 files changed

+283
-0
lines changed

NEXT_CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
### Bug Fixes
1010

11+
* Fixed spurious diffs in `databricks_model_serving` and `databricks_model_serving_provisioned_throughput` resources when the API returns `served_models` or `served_entities` in a different order than specified in the configuration ([#5188](https://github.com/databricks/terraform-provider-databricks/pull/5188)).
12+
1113
### Documentation
1214

1315
* Improve documentation about `preloaded_docker_images.basic_auth` in `databricks_cluster` and `databricks_instance_pool` ([#5154](https://github.com/databricks/terraform-provider-databricks/pull/5154)).

serving/preserve_order_test.go

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
package serving
2+
3+
import (
4+
"testing"
5+
6+
"github.com/databricks/databricks-sdk-go/service/serving"
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func TestReorderByName(t *testing.T) {
11+
t.Run("reorders items to match config order", func(t *testing.T) {
12+
configNames := []string{"prod", "staging", "dev"}
13+
apiItems := []serving.ServedModelOutput{
14+
{Name: "dev", ModelName: "model1"},
15+
{Name: "prod", ModelName: "model2"},
16+
{Name: "staging", ModelName: "model3"},
17+
}
18+
19+
result := reorderByName(configNames, apiItems,
20+
func(m serving.ServedModelOutput) string { return m.Name })
21+
22+
assert.Equal(t, "prod", result[0].Name)
23+
assert.Equal(t, "staging", result[1].Name)
24+
assert.Equal(t, "dev", result[2].Name)
25+
})
26+
27+
t.Run("handles alphabetical API ordering", func(t *testing.T) {
28+
// Simulates the actual bug: API returns alphabetically, config has different order
29+
configNames := []string{"prod_model", "candidate_model"}
30+
apiItems := []serving.ServedModelOutput{
31+
{Name: "candidate_model", ModelVersion: "2"},
32+
{Name: "prod_model", ModelVersion: "1"},
33+
}
34+
35+
result := reorderByName(configNames, apiItems,
36+
func(m serving.ServedModelOutput) string { return m.Name })
37+
38+
assert.Equal(t, "prod_model", result[0].Name)
39+
assert.Equal(t, "candidate_model", result[1].Name)
40+
})
41+
42+
t.Run("appends extra API items not in config", func(t *testing.T) {
43+
configNames := []string{"model1", "model2"}
44+
apiItems := []serving.ServedModelOutput{
45+
{Name: "model2", ModelName: "m2"},
46+
{Name: "model3", ModelName: "m3"}, // Not in config
47+
{Name: "model1", ModelName: "m1"},
48+
}
49+
50+
result := reorderByName(configNames, apiItems,
51+
func(m serving.ServedModelOutput) string { return m.Name })
52+
53+
assert.Len(t, result, 3)
54+
assert.Equal(t, "model1", result[0].Name)
55+
assert.Equal(t, "model2", result[1].Name)
56+
assert.Equal(t, "model3", result[2].Name) // Appended at end
57+
})
58+
59+
t.Run("handles empty config names", func(t *testing.T) {
60+
configNames := []string{}
61+
apiItems := []serving.ServedModelOutput{
62+
{Name: "model1"},
63+
}
64+
65+
result := reorderByName(configNames, apiItems,
66+
func(m serving.ServedModelOutput) string { return m.Name })
67+
68+
assert.Equal(t, apiItems, result) // Returns original unchanged
69+
})
70+
71+
t.Run("handles empty API items", func(t *testing.T) {
72+
configNames := []string{"model1"}
73+
apiItems := []serving.ServedModelOutput{}
74+
75+
result := reorderByName(configNames, apiItems,
76+
func(m serving.ServedModelOutput) string { return m.Name })
77+
78+
assert.Empty(t, result)
79+
})
80+
81+
t.Run("handles nil slices", func(t *testing.T) {
82+
var apiItems []serving.ServedModelOutput
83+
result := reorderByName([]string{"model1"}, apiItems,
84+
func(m serving.ServedModelOutput) string { return m.Name })
85+
86+
assert.Nil(t, result)
87+
})
88+
89+
t.Run("maintains order when already correct", func(t *testing.T) {
90+
configNames := []string{"model1", "model2", "model3"}
91+
apiItems := []serving.ServedModelOutput{
92+
{Name: "model1"},
93+
{Name: "model2"},
94+
{Name: "model3"},
95+
}
96+
97+
result := reorderByName(configNames, apiItems,
98+
func(m serving.ServedModelOutput) string { return m.Name })
99+
100+
assert.Equal(t, "model1", result[0].Name)
101+
assert.Equal(t, "model2", result[1].Name)
102+
assert.Equal(t, "model3", result[2].Name)
103+
})
104+
105+
t.Run("works with served entities", func(t *testing.T) {
106+
configNames := []string{"entity_b", "entity_a"}
107+
apiItems := []serving.ServedEntityOutput{
108+
{Name: "entity_a", EntityName: "cat.sch.entity_a"},
109+
{Name: "entity_b", EntityName: "cat.sch.entity_b"},
110+
}
111+
112+
result := reorderByName(configNames, apiItems,
113+
func(e serving.ServedEntityOutput) string { return e.Name })
114+
115+
assert.Equal(t, "entity_b", result[0].Name)
116+
assert.Equal(t, "entity_a", result[1].Name)
117+
})
118+
119+
t.Run("handles partial matches", func(t *testing.T) {
120+
// Some items in config don't exist in API response (shouldn't happen, but be safe)
121+
configNames := []string{"model1", "model2", "model3"}
122+
apiItems := []serving.ServedModelOutput{
123+
{Name: "model2"},
124+
{Name: "model1"},
125+
// model3 doesn't exist in API
126+
}
127+
128+
result := reorderByName(configNames, apiItems,
129+
func(m serving.ServedModelOutput) string { return m.Name })
130+
131+
assert.Len(t, result, 2)
132+
assert.Equal(t, "model1", result[0].Name)
133+
assert.Equal(t, "model2", result[1].Name)
134+
})
135+
136+
t.Run("handles single item", func(t *testing.T) {
137+
configNames := []string{"only_model"}
138+
apiItems := []serving.ServedModelOutput{
139+
{Name: "only_model", ModelName: "m1"},
140+
}
141+
142+
result := reorderByName(configNames, apiItems,
143+
func(m serving.ServedModelOutput) string { return m.Name })
144+
145+
assert.Len(t, result, 1)
146+
assert.Equal(t, "only_model", result[0].Name)
147+
})
148+
149+
t.Run("preserves all item properties", func(t *testing.T) {
150+
configNames := []string{"model2", "model1"}
151+
apiItems := []serving.ServedModelOutput{
152+
{
153+
Name: "model1",
154+
ModelName: "my_model",
155+
ModelVersion: "1",
156+
ScaleToZeroEnabled: true,
157+
},
158+
{
159+
Name: "model2",
160+
ModelName: "another_model",
161+
ModelVersion: "3",
162+
ScaleToZeroEnabled: false,
163+
},
164+
}
165+
166+
result := reorderByName(configNames, apiItems,
167+
func(m serving.ServedModelOutput) string { return m.Name })
168+
169+
// Check first item (model2)
170+
assert.Equal(t, "model2", result[0].Name)
171+
assert.Equal(t, "another_model", result[0].ModelName)
172+
assert.Equal(t, "3", result[0].ModelVersion)
173+
assert.False(t, result[0].ScaleToZeroEnabled)
174+
175+
// Check second item (model1)
176+
assert.Equal(t, "model1", result[1].Name)
177+
assert.Equal(t, "my_model", result[1].ModelName)
178+
assert.Equal(t, "1", result[1].ModelVersion)
179+
assert.True(t, result[1].ScaleToZeroEnabled)
180+
})
181+
}

serving/resource_model_serving.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,82 @@ func cleanWorkloadSize(s map[string]*schema.Schema, d *schema.ResourceData, apiR
172172
}
173173
}
174174

175+
// reorderByName is a generic helper that re-orders a slice of items from the API response
176+
// to match the order of names in the config. Items are matched by name, and any items in
177+
// the API response that aren't in the config are appended at the end.
178+
//
179+
// Parameters:
180+
// - configNames: ordered list of names from the user's HCL configuration
181+
// - apiItems: slice of items from the API response
182+
// - getName: function to extract the name from an API item
183+
//
184+
// Returns: re-ordered slice maintaining the same type as apiItems
185+
func reorderByName[T any](configNames []string, apiItems []T, getName func(T) string) []T {
186+
if len(configNames) == 0 || len(apiItems) == 0 {
187+
return apiItems
188+
}
189+
190+
reordered := make([]T, 0, len(apiItems))
191+
192+
// First pass: add items in config order
193+
for _, configName := range configNames {
194+
for _, apiItem := range apiItems {
195+
if getName(apiItem) == configName {
196+
reordered = append(reordered, apiItem)
197+
break
198+
}
199+
}
200+
}
201+
202+
// Second pass: append any items from API that weren't in config
203+
for _, apiItem := range apiItems {
204+
found := false
205+
for _, reorderedItem := range reordered {
206+
if getName(reorderedItem) == getName(apiItem) {
207+
found = true
208+
break
209+
}
210+
}
211+
if !found {
212+
reordered = append(reordered, apiItem)
213+
}
214+
}
215+
216+
return reordered
217+
}
218+
219+
// preserveConfigOrder re-orders the served_models and served_entities in the API response
220+
// to match the order specified in the HCL configuration. This prevents spurious diffs when
221+
// the API returns items in a different order (e.g., alphabetically) than submitted.
222+
func preserveConfigOrder(s map[string]*schema.Schema, d *schema.ResourceData, apiResponse *serving.EndpointCoreConfigOutput) {
223+
var config serving.CreateServingEndpoint
224+
common.DataToStructPointer(d, s, &config)
225+
226+
if config.Config == nil || apiResponse == nil {
227+
return
228+
}
229+
230+
// Re-order served_models to match config order
231+
if len(config.Config.ServedModels) > 0 && len(apiResponse.ServedModels) > 0 {
232+
configNames := make([]string, len(config.Config.ServedModels))
233+
for i, model := range config.Config.ServedModels {
234+
configNames[i] = model.Name
235+
}
236+
apiResponse.ServedModels = reorderByName(configNames, apiResponse.ServedModels,
237+
func(m serving.ServedModelOutput) string { return m.Name })
238+
}
239+
240+
// Re-order served_entities to match config order
241+
if len(config.Config.ServedEntities) > 0 && len(apiResponse.ServedEntities) > 0 {
242+
configNames := make([]string, len(config.Config.ServedEntities))
243+
for i, entity := range config.Config.ServedEntities {
244+
configNames[i] = entity.Name
245+
}
246+
apiResponse.ServedEntities = reorderByName(configNames, apiResponse.ServedEntities,
247+
func(e serving.ServedEntityOutput) string { return e.Name })
248+
}
249+
}
250+
175251
func ResourceModelServing() common.Resource {
176252
s := common.StructToSchema(
177253
serving.CreateServingEndpoint{},
@@ -274,6 +350,7 @@ func ResourceModelServing() common.Resource {
274350
}
275351
}
276352
cleanWorkloadSize(s, d, endpoint.Config)
353+
preserveConfigOrder(s, d, endpoint.Config)
277354

278355
err = common.StructToData(*endpoint, s, d)
279356
if err != nil {

serving/resource_model_serving_provisioned_throughput.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,28 @@ const (
1414
defaultPtProvisionTimeout = 10 * time.Minute
1515
)
1616

17+
// preserveConfigOrderPt re-orders the served_entities in the API response for provisioned
18+
// throughput endpoints to match the order specified in the HCL configuration. This prevents
19+
// spurious diffs when the API returns items in a different order (e.g., alphabetically).
20+
func preserveConfigOrderPt(s map[string]*schema.Schema, d *schema.ResourceData, apiResponse *serving.EndpointCoreConfigOutput) {
21+
var config serving.CreatePtEndpointRequest
22+
common.DataToStructPointer(d, s, &config)
23+
24+
if apiResponse == nil {
25+
return
26+
}
27+
28+
// Re-order served_entities to match config order
29+
if len(config.Config.ServedEntities) > 0 && len(apiResponse.ServedEntities) > 0 {
30+
configNames := make([]string, len(config.Config.ServedEntities))
31+
for i, entity := range config.Config.ServedEntities {
32+
configNames[i] = entity.Name
33+
}
34+
apiResponse.ServedEntities = reorderByName(configNames, apiResponse.ServedEntities,
35+
func(e serving.ServedEntityOutput) string { return e.Name })
36+
}
37+
}
38+
1739
func ResourceModelServingProvisionedThroughput() common.Resource {
1840
s := common.StructToSchema(
1941
serving.CreatePtEndpointRequest{},
@@ -72,6 +94,7 @@ func ResourceModelServingProvisionedThroughput() common.Resource {
7294
if err != nil {
7395
return err
7496
}
97+
preserveConfigOrderPt(s, d, endpoint.Config)
7598
err = common.StructToData(*endpoint, s, d)
7699
if err != nil {
77100
return err

0 commit comments

Comments
 (0)