Skip to content

Commit fc33893

Browse files
authored
Merge pull request #14 from platform9/feat/compute-followups
feat(compute): flavor extra_specs, instance resize + image_name, interface/volume attach
2 parents f7a6581 + 83397be commit fc33893

12 files changed

Lines changed: 1099 additions & 24 deletions

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ All notable changes to this project are documented here. The format is based on
4545
`pcd_compute_servergroup` (acceptance-tested); `pcd_compute_instance` (code-complete —
4646
boot verification is blocked on a lab image-library issue, see DECISIONS.md); data
4747
sources `pcd_compute_flavor`, `pcd_compute_keypair`, `pcd_compute_availability_zones`.
48+
- Compute follow-ups: `pcd_compute_flavor` gains settable `extra_specs` (added/changed/
49+
removed in place; the flavor's other attributes are now correctly immutable);
50+
`pcd_compute_instance` supports in-place **resize** on a flavor change and booting by
51+
`image_name` (resolved via Glance, alternative to `image_id`); new resources
52+
`pcd_compute_interface_attach` (attach a port/network to a server) and
53+
`pcd_compute_volume_attach` (attach a Cinder volume to a server).
4854
- Block storage (Cinder v3): `pcd_blockstorage_volume` resource (create/extend/import;
4955
code-complete — acceptance is blocked on the CE lab having no storage backend, see
5056
DECISIONS.md) and `pcd_blockstorage_volume` / `pcd_blockstorage_snapshot` data sources.

DECISIONS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ yet passable on this lab (reason noted). Generated registry docs are not committ
3030
| Compute | `pcd_compute_keypair`, `_flavor`, `_servergroup` | **VALIDATED** |
3131
| Compute (DS) | `pcd_compute_flavor`, `_keypair`, `_availability_zones` | **VALIDATED** |
3232
| Compute | `pcd_compute_instance` (boot) | **PENDING** — lab image-library gap: images don't reach the onboarded host's local library → nova returns HTTP 204 for image data. Create/schedule/wait/error-report verified; passes with a library-backed image. |
33+
| Compute | `pcd_compute_flavor` `extra_specs` | **PENDING** — code-complete; in-place add/change/remove; other flavor attrs now correctly force replacement. Acc test written (create + in-place update + import). Not yet run live (credentials unavailable this session). No lab blocker expected — flavors work on the lab. |
34+
| Compute | `pcd_compute_instance` resize + `image_name` | **PENDING** — code-complete; flavor change → Nova resize/confirm (revert on failure); `image_name` resolved via Glance. Boot-blocked (same as instance boot above). |
35+
| Compute | `pcd_compute_interface_attach` | **PENDING** — code-complete; needs a booted instance (boot-blocked). Acc test written. |
36+
| Compute | `pcd_compute_volume_attach` | **PENDING** — code-complete; needs a booted instance **and** a Cinder backend (both lab-blocked). Best-effort volume waiter degrades gracefully without Cinder. |
3337
| Block storage | `pcd_blockstorage_volume` | **PENDING** — no Cinder storage backend on the lab (`storageBackends={}`); volumes go `creating → error`. Create + waiter + error-detection verified. |
3438
| Block storage (DS) | `pcd_blockstorage_volume`, `_snapshot` | **PENDING** — untestable without volumes on this lab. |
3539

internal/provider/provider.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ func (p *pcdProvider) Resources(_ context.Context) []func() resource.Resource {
6363
compute.NewInstanceResource,
6464
compute.NewFlavorResource,
6565
compute.NewServergroupResource,
66+
compute.NewInterfaceAttachResource,
67+
compute.NewVolumeAttachResource,
6668
blockstorage.NewVolumeResource,
6769
}
6870
}

internal/services/compute/compute.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,26 @@
66
package compute
77

88
import (
9+
"context"
910
"fmt"
11+
"strings"
1012

1113
"github.com/hashicorp/terraform-plugin-framework/diag"
14+
"github.com/hashicorp/terraform-plugin-framework/types"
1215

1316
"github.com/platform9/terraform-provider-pcd/internal/clients"
1417
)
1518

19+
// splitInstanceScopedID parses a composite resource/import ID of the form
20+
// "<instance_id>/<sub_id>" (used by the attach resources).
21+
func splitInstanceScopedID(id string) (instanceID, sub string, err error) {
22+
parts := strings.SplitN(id, "/", 2)
23+
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
24+
return "", "", fmt.Errorf("expected import ID in the form <instance_id>/<id>, got %q", id)
25+
}
26+
return parts[0], parts[1], nil
27+
}
28+
1629
// configureClient extracts the shared *clients.Config from ProviderData.
1730
func configureClient(providerData any, diags *diag.Diagnostics) *clients.Config {
1831
if providerData == nil {
@@ -28,3 +41,14 @@ func configureClient(providerData any, diags *diag.Diagnostics) *clients.Config
2841
}
2942
return config
3043
}
44+
45+
// extractStringMap converts a Terraform map value to a Go map[string]string,
46+
// returning nil for a null/unknown map. Conversion diagnostics are appended.
47+
func extractStringMap(ctx context.Context, m types.Map, diags *diag.Diagnostics) map[string]string {
48+
if m.IsNull() || m.IsUnknown() {
49+
return nil
50+
}
51+
out := map[string]string{}
52+
diags.Append(m.ElementsAs(ctx, &out, false)...)
53+
return out
54+
}

internal/services/compute/flavor_resource.go

Lines changed: 88 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@ import (
1414

1515
"github.com/gophercloud/gophercloud/v2"
1616
"github.com/gophercloud/gophercloud/v2/openstack/compute/v2/flavors"
17+
"github.com/hashicorp/terraform-plugin-framework/diag"
1718
"github.com/hashicorp/terraform-plugin-framework/path"
1819
"github.com/hashicorp/terraform-plugin-framework/resource"
1920
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
2021
"github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault"
22+
"github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier"
2123
"github.com/hashicorp/terraform-plugin-framework/resource/schema/float64planmodifier"
2224
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default"
25+
"github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier"
2326
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
2427
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
2528
"github.com/hashicorp/terraform-plugin-framework/types"
@@ -53,6 +56,7 @@ type flavorModel struct {
5356
RxTxFactor types.Float64 `tfsdk:"rx_tx_factor"`
5457
IsPublic types.Bool `tfsdk:"is_public"`
5558
Ephemeral types.Int64 `tfsdk:"ephemeral"`
59+
ExtraSpecs types.Map `tfsdk:"extra_specs"`
5660
Region types.String `tfsdk:"region"`
5761
}
5862

@@ -63,20 +67,23 @@ func (r *flavorResource) Metadata(_ context.Context, req resource.MetadataReques
6367
func (r *flavorResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
6468
fn := []planmodifier.String{stringplanmodifier.RequiresReplace()}
6569
fnC := []planmodifier.String{stringplanmodifier.RequiresReplace(), stringplanmodifier.UseStateForUnknown()}
70+
fnInt := []planmodifier.Int64{int64planmodifier.RequiresReplace()}
6671
resp.Schema = schema.Schema{
67-
MarkdownDescription: "Manages a compute flavor in PCD's Nova service (admin). Flavors are immutable; " +
68-
"any change forces a new resource.",
72+
MarkdownDescription: "Manages a compute flavor in PCD's Nova service (admin). Every attribute except " +
73+
"`extra_specs` is immutable; changing one forces a new resource. `extra_specs` can be added, " +
74+
"changed, or removed in place.",
6975
Attributes: map[string]schema.Attribute{
7076
"id": schema.StringAttribute{Computed: true, MarkdownDescription: "The flavor ID.", PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()}},
71-
"name": schema.StringAttribute{Required: true, MarkdownDescription: "The name of the flavor.", PlanModifiers: fn},
72-
"ram": schema.Int64Attribute{Required: true, MarkdownDescription: "Memory in MB.", PlanModifiers: []planmodifier.Int64{}},
73-
"vcpus": schema.Int64Attribute{Required: true, MarkdownDescription: "Number of vCPUs.", PlanModifiers: []planmodifier.Int64{}},
74-
"disk": schema.Int64Attribute{Required: true, MarkdownDescription: "Root disk in GB.", PlanModifiers: []planmodifier.Int64{}},
75-
"flavor_id": schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: "The desired flavor ID (auto-generated if omitted).", PlanModifiers: fnC},
76-
"swap": schema.Int64Attribute{Optional: true, Computed: true, Default: int64default.StaticInt64(0), MarkdownDescription: "Swap space in MB."},
77-
"rx_tx_factor": schema.Float64Attribute{Optional: true, Computed: true, MarkdownDescription: "RX/TX factor.", PlanModifiers: []planmodifier.Float64{float64planmodifier.UseStateForUnknown()}},
78-
"is_public": schema.BoolAttribute{Optional: true, Computed: true, Default: booldefault.StaticBool(true), MarkdownDescription: "Whether the flavor is public."},
79-
"ephemeral": schema.Int64Attribute{Optional: true, Computed: true, Default: int64default.StaticInt64(0), MarkdownDescription: "Ephemeral disk in GB."},
77+
"name": schema.StringAttribute{Required: true, MarkdownDescription: "The name of the flavor. Changing this forces a new resource.", PlanModifiers: fn},
78+
"ram": schema.Int64Attribute{Required: true, MarkdownDescription: "Memory in MB. Changing this forces a new resource.", PlanModifiers: fnInt},
79+
"vcpus": schema.Int64Attribute{Required: true, MarkdownDescription: "Number of vCPUs. Changing this forces a new resource.", PlanModifiers: fnInt},
80+
"disk": schema.Int64Attribute{Required: true, MarkdownDescription: "Root disk in GB. Changing this forces a new resource.", PlanModifiers: fnInt},
81+
"flavor_id": schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: "The desired flavor ID (auto-generated if omitted). Changing this forces a new resource.", PlanModifiers: fnC},
82+
"swap": schema.Int64Attribute{Optional: true, Computed: true, Default: int64default.StaticInt64(0), MarkdownDescription: "Swap space in MB. Changing this forces a new resource.", PlanModifiers: fnInt},
83+
"rx_tx_factor": schema.Float64Attribute{Optional: true, Computed: true, MarkdownDescription: "RX/TX factor. Changing this forces a new resource.", PlanModifiers: []planmodifier.Float64{float64planmodifier.RequiresReplace(), float64planmodifier.UseStateForUnknown()}},
84+
"is_public": schema.BoolAttribute{Optional: true, Computed: true, Default: booldefault.StaticBool(true), MarkdownDescription: "Whether the flavor is public. Changing this forces a new resource.", PlanModifiers: []planmodifier.Bool{boolplanmodifier.RequiresReplace()}},
85+
"ephemeral": schema.Int64Attribute{Optional: true, Computed: true, Default: int64default.StaticInt64(0), MarkdownDescription: "Ephemeral disk in GB. Changing this forces a new resource.", PlanModifiers: fnInt},
86+
"extra_specs": schema.MapAttribute{Optional: true, Computed: true, ElementType: types.StringType, MarkdownDescription: "Key/value extra specs (e.g. `hw:cpu_policy`). Can be added, changed, or removed on an existing flavor without replacing it."},
8087
"region": schema.StringAttribute{Optional: true, Computed: true, MarkdownDescription: "The region. Defaults to the provider's region.", PlanModifiers: []planmodifier.String{stringplanmodifier.UseStateForUnknown()}},
8188
},
8289
}
@@ -121,7 +128,18 @@ func (r *flavorResource) Create(ctx context.Context, req resource.CreateRequest,
121128
return
122129
}
123130

131+
if specs := extractStringMap(ctx, plan.ExtraSpecs, &resp.Diagnostics); len(specs) > 0 {
132+
if resp.Diagnostics.HasError() {
133+
return
134+
}
135+
if _, err := flavors.CreateExtraSpecs(ctx, client, flavor.ID, flavors.ExtraSpecsOpts(specs)).Extract(); err != nil {
136+
resp.Diagnostics.AddError("compute: setting flavor extra specs", err.Error())
137+
return
138+
}
139+
}
140+
124141
r.flatten(flavor, &plan)
142+
resp.Diagnostics.Append(r.refreshExtraSpecs(ctx, client, flavor.ID, &plan)...)
125143
resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
126144
}
127145

@@ -151,16 +169,55 @@ func (r *flavorResource) Read(ctx context.Context, req resource.ReadRequest, res
151169
}
152170

153171
r.flatten(flavor, &state)
172+
resp.Diagnostics.Append(r.refreshExtraSpecs(ctx, client, state.ID.ValueString(), &state)...)
154173
resp.Diagnostics.Append(resp.State.Set(ctx, &state)...)
155174
}
156175

157-
// Update is required by the interface but never invoked (flavors are immutable).
176+
// Update reconciles extra_specs — the only in-place-mutable attribute (every
177+
// other flavor field forces replacement).
158178
func (r *flavorResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
159-
var plan flavorModel
179+
var plan, state flavorModel
160180
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
181+
resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
182+
if resp.Diagnostics.HasError() {
183+
return
184+
}
185+
186+
client, err := r.config.ComputeV2Client()
187+
if err != nil {
188+
resp.Diagnostics.AddError("compute: building v2 client", err.Error())
189+
return
190+
}
191+
192+
oldSpecs := extractStringMap(ctx, state.ExtraSpecs, &resp.Diagnostics)
193+
newSpecs := extractStringMap(ctx, plan.ExtraSpecs, &resp.Diagnostics)
161194
if resp.Diagnostics.HasError() {
162195
return
163196
}
197+
198+
id := plan.ID.ValueString()
199+
for k := range oldSpecs {
200+
if _, ok := newSpecs[k]; !ok {
201+
if err := flavors.DeleteExtraSpec(ctx, client, id, k).ExtractErr(); err != nil {
202+
resp.Diagnostics.AddError("compute: deleting flavor extra spec", err.Error())
203+
return
204+
}
205+
}
206+
}
207+
changed := map[string]string{}
208+
for k, v := range newSpecs {
209+
if oldSpecs[k] != v {
210+
changed[k] = v
211+
}
212+
}
213+
if len(changed) > 0 {
214+
if _, err := flavors.CreateExtraSpecs(ctx, client, id, flavors.ExtraSpecsOpts(changed)).Extract(); err != nil {
215+
resp.Diagnostics.AddError("compute: updating flavor extra specs", err.Error())
216+
return
217+
}
218+
}
219+
220+
resp.Diagnostics.Append(r.refreshExtraSpecs(ctx, client, id, &plan)...)
164221
resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...)
165222
}
166223

@@ -189,6 +246,24 @@ func (r *flavorResource) ImportState(ctx context.Context, req resource.ImportSta
189246
resource.ImportStatePassthroughID(ctx, path.Root("id"), req, resp)
190247
}
191248

249+
// refreshExtraSpecs lists the flavor's extra specs and writes them onto the
250+
// model as an (always non-null) map, so state reflects the server.
251+
func (r *flavorResource) refreshExtraSpecs(ctx context.Context, client *gophercloud.ServiceClient, id string, m *flavorModel) diag.Diagnostics {
252+
var diags diag.Diagnostics
253+
specs, err := flavors.ListExtraSpecs(ctx, client, id).Extract()
254+
if err != nil {
255+
diags.AddError("compute: listing flavor extra specs", err.Error())
256+
return diags
257+
}
258+
if specs == nil {
259+
specs = map[string]string{}
260+
}
261+
sm, d := types.MapValueFrom(ctx, types.StringType, specs)
262+
diags.Append(d...)
263+
m.ExtraSpecs = sm
264+
return diags
265+
}
266+
192267
func (r *flavorResource) flatten(flavor *flavors.Flavor, m *flavorModel) {
193268
m.ID = types.StringValue(flavor.ID)
194269
m.FlavorID = types.StringValue(flavor.ID)

internal/services/compute/flavor_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,73 @@ data "pcd_compute_flavor" "by_name" {
5555
}
5656
`
5757

58+
// TestAccComputeFlavor_extraSpecs verifies extra_specs can be added, changed, and
59+
// removed in place (without replacing the flavor).
60+
func TestAccComputeFlavor_extraSpecs(t *testing.T) {
61+
const rn = "pcd_compute_flavor.test"
62+
var flavorID string
63+
64+
resource.Test(t, resource.TestCase{
65+
PreCheck: func() { acctest.PreCheck(t) },
66+
ProtoV6ProviderFactories: acctest.ProtoV6ProviderFactories,
67+
CheckDestroy: testAccCheckFlavorDestroy(t),
68+
Steps: []resource.TestStep{
69+
{
70+
Config: testAccFlavorExtraSpecsConfig(`extra_specs = { "hw:cpu_policy" = "shared", "hw:numa_nodes" = "1" }`),
71+
Check: resource.ComposeAggregateTestCheckFunc(
72+
testAccCheckFlavorExists(t, rn),
73+
testAccCaptureID(rn, &flavorID),
74+
resource.TestCheckResourceAttr(rn, "extra_specs.%", "2"),
75+
resource.TestCheckResourceAttr(rn, "extra_specs.hw:cpu_policy", "shared"),
76+
resource.TestCheckResourceAttr(rn, "extra_specs.hw:numa_nodes", "1"),
77+
),
78+
},
79+
{
80+
// Replace cpu_policy, remove numa_nodes, add mem_page_size.
81+
Config: testAccFlavorExtraSpecsConfig(`extra_specs = { "hw:cpu_policy" = "dedicated", "hw:mem_page_size" = "large" }`),
82+
Check: resource.ComposeAggregateTestCheckFunc(
83+
resource.TestCheckResourceAttrWith(rn, "id", func(v string) error {
84+
if v != flavorID {
85+
return fmt.Errorf("flavor was replaced (%s -> %s); extra_specs should update in place", flavorID, v)
86+
}
87+
return nil
88+
}),
89+
resource.TestCheckResourceAttr(rn, "extra_specs.%", "2"),
90+
resource.TestCheckResourceAttr(rn, "extra_specs.hw:cpu_policy", "dedicated"),
91+
resource.TestCheckResourceAttr(rn, "extra_specs.hw:mem_page_size", "large"),
92+
resource.TestCheckNoResourceAttr(rn, "extra_specs.hw:numa_nodes"),
93+
),
94+
},
95+
{ResourceName: rn, ImportState: true, ImportStateVerify: true},
96+
},
97+
})
98+
}
99+
100+
func testAccFlavorExtraSpecsConfig(specs string) string {
101+
return fmt.Sprintf(`
102+
resource "pcd_compute_flavor" "test" {
103+
name = "tf-acc-flavor-es"
104+
ram = 256
105+
vcpus = 1
106+
disk = 1
107+
%s
108+
}
109+
`, specs)
110+
}
111+
112+
// testAccCaptureID records a resource's ID for later comparison (e.g. asserting
113+
// the resource was not replaced across steps).
114+
func testAccCaptureID(n string, dst *string) resource.TestCheckFunc {
115+
return func(s *terraform.State) error {
116+
rs := s.RootModule().Resources[n]
117+
if rs == nil {
118+
return fmt.Errorf("not found in state: %s", n)
119+
}
120+
*dst = rs.Primary.ID
121+
return nil
122+
}
123+
}
124+
58125
func testAccCheckFlavorExists(t *testing.T, n string) resource.TestCheckFunc {
59126
return func(s *terraform.State) error {
60127
rs := s.RootModule().Resources[n]

0 commit comments

Comments
 (0)