Add GPU support: gpu_card / vgpu_profile data sources and GPU on service offerings - #315
Add GPU support: gpu_card / vgpu_profile data sources and GPU on service offerings#315poddm wants to merge 10 commits into
Conversation
| }, | ||
| "count": schema.Int32Attribute{ | ||
| Description: "the number of GPUs to assign to the guest VM", | ||
| Optional: true, |
There was a problem hiding this comment.
should we set a default value or computed here?
| "display": schema.BoolAttribute{ | ||
| Description: "whether the GPU is presented as a display device to the guest VM", | ||
| Optional: true, | ||
| PlanModifiers: []planmodifier.Bool{ |
There was a problem hiding this comment.
should we set a default value or computed here?
|
@poddm can you resolve the conflicts? |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds provider support for CloudStack GPU/vGPU discovery and service offering GPU configuration, aligning with newer CloudStack GPU APIs.
Changes:
- Introduces
cloudstack_gpu_cardandcloudstack_vgpu_profiledata sources with filter support. - Adds an optional
gpunested block to service offering resources (constrained/fixed/unconstrained). - Updates docs and bumps
cloudstack-godependency to include GPU API bindings.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/d/vgpu_profile.html.markdown | Adds documentation for the vGPU profile data source and its exported attributes. |
| website/docs/d/gpu_card.html.markdown | Adds documentation for the GPU card data source and its exported attributes. |
| go.mod | Bumps github.com/apache/cloudstack-go/v2 to a version that includes GPU bindings. |
| go.sum | Updates module checksums for the dependency bump. |
| cloudstack/provider.go | Registers the new GPU-related data sources in the provider. |
| cloudstack/data_source_cloudstack_vgpu_profile.go | Implements the cloudstack_vgpu_profile data source and filtering logic. |
| cloudstack/data_source_cloudstack_vgpu_profile_test.go | Adds acceptance test coverage for the vGPU profile data source. |
| cloudstack/data_source_cloudstack_gpu_card.go | Implements the cloudstack_gpu_card data source and filtering logic. |
| cloudstack/data_source_cloudstack_gpu_card_test.go | Adds acceptance test coverage for the GPU card data source. |
| cloudstack/service_offering_schema.go | Adds the gpu nested block schema shared by service offering resources. |
| cloudstack/service_offering_models.go | Adds the ServiceOfferingGpu model and wires it into the common resource model. |
| cloudstack/service_offering_util.go | Adds common read/create param helpers for the service offering gpu block. |
| cloudstack/service_offering_constrained_resource.go | Wires gpu block into constrained service offering create/read flows. |
| cloudstack/service_offering_constrained_resource_test.go | Adds acceptance test coverage for constrained service offering GPU configuration. |
| cloudstack/service_offering_fixed_resource.go | Wires gpu block into fixed service offering create/read flows. |
| cloudstack/service_offering_fixed_resource_test.go | Adds acceptance test coverage for fixed service offering GPU configuration. |
| cloudstack/service_offering_unconstrained_resource.go | Wires gpu block into unconstrained service offering create/read flows. |
| cloudstack/service_offering_unconstrained_resource_test.go | Adds acceptance test coverage for unconstrained service offering GPU configuration. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| func applyVgpuProfileFilters(profile *cloudstack.VgpuProfile, filters *schema.Set) (bool, error) { | ||
| val := reflect.ValueOf(profile).Elem() | ||
|
|
||
| for _, f := range filters.List() { | ||
| filter := f.(map[string]interface{}) | ||
| r, err := regexp.Compile(filter["value"].(string)) | ||
| if err != nil { | ||
| return false, fmt.Errorf("invalid regex: %s", err) | ||
| } | ||
| updatedName := strings.ReplaceAll(filter["name"].(string), "_", "") | ||
| profileField := val.FieldByNameFunc(func(fieldName string) bool { | ||
| if strings.EqualFold(fieldName, updatedName) { | ||
| updatedName = fieldName | ||
| return true | ||
| } | ||
| return false | ||
| }).String() | ||
|
|
||
| if !r.MatchString(profileField) { | ||
| return false, nil | ||
| } | ||
| } |
| func applyGpuCardFilters(card *cloudstack.GpuCard, filters *schema.Set) (bool, error) { | ||
| val := reflect.ValueOf(card).Elem() | ||
|
|
||
| for _, f := range filters.List() { | ||
| filter := f.(map[string]interface{}) | ||
| r, err := regexp.Compile(filter["value"].(string)) | ||
| if err != nil { | ||
| return false, fmt.Errorf("invalid regex: %s", err) | ||
| } | ||
| updatedName := strings.ReplaceAll(filter["name"].(string), "_", "") | ||
| cardField := val.FieldByNameFunc(func(fieldName string) bool { | ||
| if strings.EqualFold(fieldName, updatedName) { | ||
| updatedName = fieldName | ||
| return true | ||
| } | ||
| return false | ||
| }).String() | ||
|
|
||
| if !r.MatchString(cardField) { | ||
| return false, nil | ||
| } | ||
| } |
| state.VgpuProfileId = types.StringValue(cs.Vgpuprofileid) | ||
| } | ||
| if cs.Gpucount > 0 { | ||
| state.Count = types.Int32Value(int32(cs.Gpucount)) |
| } | ||
| } | ||
|
|
||
| return fmt.Errorf("no vGPU profiles found") |
| } | ||
| } | ||
|
|
||
| return fmt.Errorf("no GPU cards found") |
| const testAccServiceOfferingUnconstrained_gpu = ` | ||
| resource "cloudstack_service_offering_unconstrained" "gpu" { | ||
| display_text = "gpu" | ||
| name = "gpu" | ||
|
|
||
| host_tags = "test0101,test0202" | ||
| network_rate = 1024 | ||
| deployment_planner = "UserDispersingPlanner" | ||
|
|
||
| dynamic_scaling_enabled = true | ||
| is_volatile = true | ||
| limit_cpu_use = true | ||
| offer_ha = true | ||
|
|
||
| gpu = { | ||
| vgpu_profile_id = "a6000-8a-profile" | ||
| count = 1 | ||
| display = true | ||
| } | ||
| } | ||
| ` |
- Add Computed: true to count field to indicate server-managed attribute - Add Computed: true and Default: false to display field for consistency with other boolean attributes in the schema
Review Fixes: 1. Fix filter panic in vGPU profile datasource - Safely validate field exists before accessing - Use fmt.Sprintf for safe type conversion - Return clear error for unknown filter fields 2. Fix filter panic in GPU card datasource - Same safety improvements as vGPU profile filters - Prevents panics on non-string fields or invalid names 3. Fix state drift detection in service offering GPU block - Explicitly set VgpuProfileId to null when empty - Explicitly set Count to null when 0 - Allows drift detection when GPU config changes out-of-band 4. Update service offering documentation - Add GPU block examples to fixed/constrained/unconstrained offerings - Document GPU block attributes and defaults Note: Client-side filtering remains; API doesn't expose server-side filter params for GPU cards/vGPU profiles as suggested in review.
- Add testAccPreCheckGPU to provider_test.go for version validation - Update GPU datasource tests to use testAccPreCheckGPU - Create separate GPU test functions for service offerings - TestAccServiceOfferingFixed_GPU - TestAccServiceOfferingConstrained_GPU - TestAccServiceOfferingUnconstrained_GPU - Tests will skip if CloudStack version < 4.22.0.0 This ensures GPU tests only run on CloudStack versions that support the GPU API (4.22.0+).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Suppressed comments (5)
cloudstack/data_source_cloudstack_vgpu_profile.go:174
- The regex is compiled for every filter evaluation and (as written) would be recompiled for every item in the API response. Precompile the regex patterns once per read (e.g., build a slice/map of compiled filters before iterating the returned profiles) to avoid repeated compilation overhead, especially if
listVgpuProfilesreturns many entries.
for _, f := range filters.List() {
filter := f.(map[string]interface{})
r, err := regexp.Compile(filter["value"].(string))
if err != nil {
return false, fmt.Errorf("invalid regex: %s", err)
}
cloudstack/data_source_cloudstack_gpu_card.go:127
- The regex is compiled inside the filter application loop, which is called once per returned GPU card. Consider compiling all filter regexes once (before iterating
csGpuCards.GpuCards) and reusing them for each card to reduce CPU overhead.
for _, f := range filters.List() {
filter := f.(map[string]interface{})
r, err := regexp.Compile(filter["value"].(string))
if err != nil {
return false, fmt.Errorf("invalid regex: %s", err)
}
website/docs/r/service_offering_constrained.html.markdown:45
- The docs use block syntax (
gpu { ... }), but the acceptance tests and PR description use object assignment (gpu = { ... }). Since the schema is implemented as aschema.SingleNestedAttribute(object attribute), the docs should match the correct configuration style (or the schema should be changed to a nested block type if block syntax is intended). Please update the service offering docs consistently (constrained/fixed/unconstrained) to avoid user confusion.
gpu {
vgpu_profile_id = "gpu-profile-uuid"
count = 1
display = false
}
cloudstack/service_offering_schema.go:257
gpu.countshould be validated to prevent invalid values (e.g., 0 or negative), which CloudStack is unlikely to accept for a GPU assignment count. Add an Int32 validator (e.g., at least 1) so bad configs fail fast during planning.
"count": schema.Int32Attribute{
Description: "the number of GPUs to assign to the guest VM",
Optional: true,
Computed: true,
PlanModifiers: []planmodifier.Int32{
int32planmodifier.RequiresReplace(),
},
},
cloudstack/service_offering_schema.go:266
- In terraform-plugin-framework schemas, combining
Computed: truewithDefault: ...is typically invalid/unsupported because defaults apply to optional attributes while computed values are set by the provider. Consider removingComputed: true(keepOptional + Default) or removing theDefaultand handling unknowns via plan modifiers/state to avoid schema validation/runtime errors.
"display": schema.BoolAttribute{
Description: "whether the GPU is presented as a display device to the guest VM",
Optional: true,
Computed: true,
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.RequiresReplace(),
},
Default: booldefault.StaticBool(false),
},
| var profileField reflect.Value | ||
| val.FieldByNameFunc(func(fieldName string) bool { | ||
| if strings.EqualFold(fieldName, updatedName) { | ||
| updatedName = fieldName | ||
| profileField = val.FieldByName(fieldName) | ||
| return true | ||
| } | ||
| return false | ||
| }) | ||
|
|
||
| // Validate field was found | ||
| if !profileField.IsValid() { | ||
| return false, fmt.Errorf("unknown filter field '%s'", filterName) | ||
| } |
| var cardField reflect.Value | ||
| val.FieldByNameFunc(func(fieldName string) bool { | ||
| if strings.EqualFold(fieldName, updatedName) { | ||
| updatedName = fieldName | ||
| cardField = val.FieldByName(fieldName) | ||
| return true | ||
| } | ||
| return false | ||
| }) | ||
|
|
||
| // Validate field was found | ||
| if !cardField.IsValid() { | ||
| return false, fmt.Errorf("unknown filter field '%s'", filterName) | ||
| } | ||
|
|
|
e449cd9 to
2fd9627
Compare
Summary
Adds GPU support to the provider, aligned with the GPU/vGPU APIs introduced in
recent CloudStack releases. This lets operators discover GPU cards and vGPU
profiles and attach GPUs to service offerings.
What's included
New data sources
cloudstack_gpu_card— looks up a GPU card via thelistGpuCardsAPI.Exposes
id,name,device_id,device_name,vendor_id,vendor_name.cloudstack_vgpu_profile— looks up a vGPU profile via thelistVgpuProfilesAPI. Exposes
id,name,description,device_id,device_name,gpu_card_id,gpu_card_name,max_heads,max_resolution_x,max_resolution_y,max_vgpu_per_physical_gpu,vendor_id,vendor_name,video_ram.Both data sources support the standard
filterblock (regex matching onreturned fields).
Service offering GPU configuration
gpunested block to the service offering resources(constrained / fixed / unconstrained), with:
vgpu_profile_id(required) — vGPU profile to associate with the offeringcount(optional) — number of GPUs assigned to the guest VMdisplay(optional) — whether the GPU is presented as a display deviceDocs
gpu_cardandvgpu_profiledata sources.Dependency
github.com/apache/cloudstack-go/v2fromv2.18.1tov2.19.1for theGPU API bindings.
Example usage