Skip to content

Commit fa91e08

Browse files
committed
Merge branch 'issue-1253'
2 parents bb1fe6a + df451fe commit fa91e08

3 files changed

Lines changed: 74 additions & 3 deletions

File tree

internal/codegen/parser/libvirtxml.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,11 +218,15 @@ func (r *LibvirtXMLReflector) applyFieldPatterns(structName string, fields []*ge
218218
field.IsOptional = false
219219
field.IsRequired = false
220220
field.PlanModifier = "UseStateForUnknown"
221+
// Pool capacity is purely reported by libvirt; always read from XML.
222+
field.PreserveUserIntent = false
221223

222224
case "allocation", "available":
223225
field.IsComputed = true
224226
field.IsOptional = false
225227
field.IsRequired = false
228+
// Purely informational; always read from XML.
229+
field.PreserveUserIntent = false
226230
}
227231
}
228232

@@ -233,10 +237,21 @@ func (r *LibvirtXMLReflector) applyFieldPatterns(structName string, fields []*ge
233237
}
234238

235239
switch field.TFName {
236-
case "capacity", "allocation", "physical":
240+
case "capacity":
241+
field.IsComputed = true
242+
field.IsOptional = false
243+
field.IsRequired = false
244+
// Keep PreserveUserIntent = true (set by analyzeField for pointer fields)
245+
// so that when the user specifies capacity with a capacity_unit, the
246+
// plan value is preserved on readback instead of the bytes-normalised
247+
// value that libvirt returns (fixes issue #1253).
248+
249+
case "allocation", "physical":
237250
field.IsComputed = true
238251
field.IsOptional = false
239252
field.IsRequired = false
253+
// Purely informational; always read from XML.
254+
field.PreserveUserIntent = false
240255
}
241256
}
242257
}

internal/codegen/templates/convert.go.tmpl

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,9 +248,10 @@ func {{ .Name }}ToXML(ctx context.Context, model *{{ .Name }}Model) (*libvirtxml
248248
{{- if .IsFlattenedValue }}
249249
// Flattened chardata+attributes field: {{ .GoName }}
250250
{{- $valueField := .NestedStruct.ValueWithUnitPattern.ValueFieldName }}
251-
{{- $alwaysRead := .IsComputed }}
251+
{{- $alwaysRead := and .IsComputed (not .PreserveUserIntent) }}
252+
{{- $readIfMissing := and .IsComputed .PreserveUserIntent }}
252253
plan{{ .GoName }}HasValue := plan != nil && !plan.{{ .GoName }}.IsNull() && !plan.{{ .GoName }}.IsUnknown()
253-
if plan == nil || {{ if $alwaysRead }}true{{ else }}false{{ end }} {
254+
if plan == nil || {{ if $alwaysRead }}true{{ else if $readIfMissing }}!plan{{ .GoName }}HasValue{{ else }}false{{ end }} {
254255
{{- if .IsPointer }}
255256
if xml.{{ .GoName }} != nil {
256257
{{- if eq .TFType "types.String" }}

internal/provider/volume_resource_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,61 @@ resource "libvirt_domain" "test" {
287287
`, name, poolPath)
288288
}
289289

290+
// TestAccVolumeResource_capacityUnit reproduces issue #1253: using capacity_unit
291+
// (e.g. "GiB") causes "Provider produced inconsistent result after apply" because
292+
// libvirt normalises the value to bytes on readback and the provider was always
293+
// reading the raw bytes value instead of preserving the plan's unit-converted value.
294+
func TestAccVolumeResource_capacityUnit(t *testing.T) {
295+
poolPath := t.TempDir()
296+
297+
resource.Test(t, resource.TestCase{
298+
PreCheck: func() { testAccPreCheck(t) },
299+
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
300+
Steps: []resource.TestStep{
301+
{
302+
// Create a 1 GiB volume using capacity_unit.
303+
// Before the fix this errors: "Provider produced inconsistent result
304+
// after apply: .capacity: was cty.NumberIntVal(1), but now
305+
// cty.NumberIntVal(1073741824)".
306+
Config: testAccVolumeResourceConfigCapacityUnit("test-volume-cap-unit", poolPath),
307+
Check: resource.ComposeAggregateTestCheckFunc(
308+
resource.TestCheckResourceAttr("libvirt_volume.test", "capacity", "1"),
309+
resource.TestCheckResourceAttr("libvirt_volume.test", "capacity_unit", "GiB"),
310+
),
311+
},
312+
{
313+
// Ensure there is no perpetual diff after the first apply.
314+
Config: testAccVolumeResourceConfigCapacityUnit("test-volume-cap-unit", poolPath),
315+
PlanOnly: true,
316+
},
317+
},
318+
})
319+
}
320+
321+
func testAccVolumeResourceConfigCapacityUnit(name, poolPath string) string {
322+
return fmt.Sprintf(`
323+
resource "libvirt_pool" "test" {
324+
name = "test-pool-cap-unit"
325+
type = "dir"
326+
target = {
327+
path = %[2]q
328+
}
329+
}
330+
331+
resource "libvirt_volume" "test" {
332+
name = "%[1]s.qcow2"
333+
pool = libvirt_pool.test.name
334+
capacity = 1
335+
capacity_unit = "GiB"
336+
target = {
337+
format = {
338+
type = "qcow2"
339+
}
340+
}
341+
}
342+
`, name, poolPath)
343+
}
344+
290345
func TestAccVolumeResource_uploadFromFile(t *testing.T) {
291346
poolPath := t.TempDir()
292347

0 commit comments

Comments
 (0)