Skip to content

Commit

Permalink
Merge pull request #136 from maveonair/remove-limits
Browse files Browse the repository at this point in the history 
Remove limits
  • Loading branch information
@stgraber
stgraber authored Sep 16, 2024
2 parents cd04b33 + 802cd7a commit 3b88ad3
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 95 deletions.
8 changes: 1 addition & 7 deletions docs/resources/instance.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@ resource "incus_instance" "instance1" {
config = {
"boot.autostart" = true
}
limits = {
cpu = 2
"limits.cpu" = 2
}
}
```
Expand Down Expand Up @@ -120,9 +117,6 @@ resource "incus_instance" "instance2" {

* `file` - *Optional* - File to upload to the instance. See reference below.

* `limits` - *Optional* - Map of key/value pairs that define the
[instance resources limits](https://linuxcontainers.org/incus/docs/main/reference/instance_options/#resource-limits).

* `config` - *Optional* - Map of key/value pairs of
[instance config settings](https://linuxcontainers.org/incus/docs/main/reference/instance_options/).

Expand Down
73 changes: 6 additions & 67 deletions internal/instance/resource_instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ type InstanceModel struct {
Profiles types.List `tfsdk:"profiles"`
Devices types.Set `tfsdk:"device"`
Files types.Set `tfsdk:"file"`
Limits types.Map `tfsdk:"limits"`
Config types.Map `tfsdk:"config"`
Project types.String `tfsdk:"project"`
Remote types.String `tfsdk:"remote"`
Expand Down Expand Up @@ -150,18 +149,6 @@ func (r InstanceResource) Schema(_ context.Context, _ resource.SchemaRequest, re
},
},

"limits": schema.MapAttribute{
Optional: true,
Computed: true,
ElementType: types.StringType,
Default: mapdefault.StaticValue(types.MapValueMust(types.StringType, map[string]attr.Value{})),
Validators: []validator.Map{
// Prevent empty keys or values.
mapvalidator.KeysAre(stringvalidator.LengthAtLeast(1)),
mapvalidator.ValueStringsAre(stringvalidator.LengthAtLeast(1)),
},
},

"project": schema.StringAttribute{
Optional: true,
PlanModifiers: []planmodifier.String{
Expand Down Expand Up @@ -506,7 +493,7 @@ func (r InstanceResource) Read(ctx context.Context, req resource.ReadRequest, re

// Update updates the instance in the following order:
// - Ensure instance state (stopped/running)
// - Update configuration (config, limits, devices, profiles)
// - Update configuration (config, devices, profiles)
// - Upload files
// - Run exec commands
func (r InstanceResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
Expand Down Expand Up @@ -574,17 +561,14 @@ func (r InstanceResource) Update(ctx context.Context, req resource.UpdateRequest
return
}

// First extract profiles, devices, limits, config and config state.
// First extract profiles, devices, config and config state.
// Then merge user defined config with instance config (state).
profiles, diags := ToProfileList(ctx, plan.Profiles)
resp.Diagnostics.Append(diags...)

devices, diags := common.ToDeviceMap(ctx, plan.Devices)
resp.Diagnostics.Append(diags...)

limits, diag := common.ToConfigMap(ctx, plan.Limits)
resp.Diagnostics.Append(diag...)

userConfig, diags := common.ToConfigMap(ctx, plan.Config)
resp.Diagnostics.Append(diags...)

Expand All @@ -594,12 +578,6 @@ func (r InstanceResource) Update(ctx context.Context, req resource.UpdateRequest
return
}

// Merge limits into instance config.
for k, v := range limits {
key := fmt.Sprintf("limits.%s", k)
config[key] = v
}

newInstance := api.InstancePut{
Description: plan.Description.ValueString(),
Ephemeral: plan.Ephemeral.ValueBool(),
Expand Down Expand Up @@ -802,23 +780,10 @@ func (r InstanceResource) SyncState(ctx context.Context, tfState *tfsdk.State, s
// Extract user defined config and merge it with current resource config.
stateConfig := common.StripConfig(instance.Config, m.Config, m.ComputedKeys())

// Extract enteries with "limits." prefix.
instanceLimits := make(map[string]string)
for k, v := range stateConfig {
key, ok := strings.CutPrefix(k, "limits.")
if ok {
instanceLimits[key] = *v
delete(stateConfig, k)
}
}

// Convert config, limits, profiles, and devices into schema type.
// Convert config, profiles, and devices into schema type.
config, diags := common.ToConfigMapType(ctx, stateConfig, m.Config)
respDiags.Append(diags...)

limits, diags := common.ToConfigMapType(ctx, common.ToNullableConfig(instanceLimits), m.Config)
respDiags.Append(diags...)

profiles, diags := ToProfileListType(ctx, instance.Profiles)
respDiags.Append(diags...)

Expand All @@ -835,7 +800,6 @@ func (r InstanceResource) SyncState(ctx context.Context, tfState *tfsdk.State, s
m.Ephemeral = types.BoolValue(instance.Ephemeral)
m.Status = types.StringValue(instance.Status)
m.Profiles = profiles
m.Limits = limits
m.Devices = devices
m.Config = config

Expand Down Expand Up @@ -961,7 +925,7 @@ func (r InstanceResource) createInstanceFromSourceInstance(ctx context.Context,
return diags
}

// Extract profiles, devices, config and limits.
// Extract profiles, devices and config.
profiles, diags := ToProfileList(ctx, plan.Profiles)
diags.Append(diags...)

Expand All @@ -971,9 +935,6 @@ func (r InstanceResource) createInstanceFromSourceInstance(ctx context.Context,
config, diags := common.ToConfigMap(ctx, plan.Config)
diags.Append(diags...)

limits, diags := common.ToConfigMap(ctx, plan.Limits)
diags.Append(diags...)

if diags.HasError() {
return diags
}
Expand All @@ -985,11 +946,6 @@ func (r InstanceResource) createInstanceFromSourceInstance(ctx context.Context,
sourceInstance.Config[key] = value
}

for k, v := range limits {
key := fmt.Sprintf("limits.%s", k)
config[key] = v
}

// Allow setting device overrides
for k, m := range devices {
if sourceInstance.Devices[k] == nil {
Expand Down Expand Up @@ -1032,7 +988,7 @@ func (r InstanceResource) createInstanceFromSourceInstance(ctx context.Context,
return diags
}

// Extract profiles, devices, config and limits.
// Extract profiles, devices and config.
profiles, diags := ToProfileList(ctx, plan.Profiles)
diags.Append(diags...)

Expand All @@ -1042,9 +998,6 @@ func (r InstanceResource) createInstanceFromSourceInstance(ctx context.Context,
config, diags := common.ToConfigMap(ctx, plan.Config)
diags.Append(diags...)

limits, diags := common.ToConfigMap(ctx, plan.Limits)
diags.Append(diags...)

if diags.HasError() {
return diags
}
Expand All @@ -1056,11 +1009,6 @@ func (r InstanceResource) createInstanceFromSourceInstance(ctx context.Context,
sourceSnapshot.Config[key] = value
}

for k, v := range limits {
key := fmt.Sprintf("limits.%s", k)
config[key] = v
}

// Allow setting device overrides
for k, m := range devices {
if sourceSnapshot.Devices[k] == nil {
Expand Down Expand Up @@ -1139,7 +1087,7 @@ func (r InstanceResource) createInstanceWithoutImage(ctx context.Context, server
func prepareInstancesPost(ctx context.Context, plan InstanceModel) (api.InstancesPost, diag.Diagnostics) {
var diags diag.Diagnostics

// Extract profiles, devices, config and limits.
// Extract profiles, devices and config.
profiles, diags := ToProfileList(ctx, plan.Profiles)
diags.Append(diags...)

Expand All @@ -1149,19 +1097,10 @@ func prepareInstancesPost(ctx context.Context, plan InstanceModel) (api.Instance
config, diags := common.ToConfigMap(ctx, plan.Config)
diags.Append(diags...)

limits, diags := common.ToConfigMap(ctx, plan.Limits)
diags.Append(diags...)

if diags.HasError() {
return api.InstancesPost{}, diags
}

// Merge limits into instance config.
for k, v := range limits {
key := fmt.Sprintf("limits.%s", k)
config[key] = v
}

instance := api.InstancesPost{
Name: plan.Name.ValueString(),
Type: api.InstanceType(plan.Type.ValueString()),
Expand Down
20 changes: 10 additions & 10 deletions internal/instance/resource_instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,18 +586,18 @@ func TestAccInstance_configLimits(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("incus_instance.instance1", "name", instanceName),
resource.TestCheckResourceAttr("incus_instance.instance1", "status", "Running"),
resource.TestCheckResourceAttr("incus_instance.instance1", "limits.%", "1"),
resource.TestCheckResourceAttr("incus_instance.instance1", "limits.cpu", "1"),
resource.TestCheckResourceAttr("incus_instance.instance1", "config.%", "1"),
resource.TestCheckResourceAttr("incus_instance.instance1", "config.limits.cpu", "1"),
),
},
{
Config: testAccInstance_configLimits_2(instanceName),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("incus_instance.instance1", "name", instanceName),
resource.TestCheckResourceAttr("incus_instance.instance1", "status", "Running"),
resource.TestCheckResourceAttr("incus_instance.instance1", "limits.%", "2"),
resource.TestCheckResourceAttr("incus_instance.instance1", "limits.cpu", "2"),
resource.TestCheckResourceAttr("incus_instance.instance1", "limits.memory", "128MiB"),
resource.TestCheckResourceAttr("incus_instance.instance1", "config.%", "2"),
resource.TestCheckResourceAttr("incus_instance.instance1", "config.limits.cpu", "2"),
resource.TestCheckResourceAttr("incus_instance.instance1", "config.limits.memory", "128MiB"),
),
},
},
Expand Down Expand Up @@ -1251,8 +1251,8 @@ resource "incus_instance" "instance1" {
name = "%s"
image = "%s"
limits = {
"cpu" = 1
config = {
"limits.cpu" = 1
}
}
`, name, acctest.TestImage)
Expand All @@ -1264,9 +1264,9 @@ resource "incus_instance" "instance1" {
name = "%s"
image = "%s"
limits = {
"cpu" = 2
"memory" = "128MiB"
config = {
"limits.cpu" = 2
"limits.memory" = "128MiB"
}
}
`, name, acctest.TestImage)
Expand Down
14 changes: 3 additions & 11 deletions internal/instance/schema_validators.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,25 @@ package instance
import (
"context"
"fmt"
"strings"

"github.com/hashicorp/terraform-plugin-framework/schema/validator"

"github.com/lxc/terraform-provider-incus/internal/utils"
)

// configKeyValidator ensures config key does not start
// with "volatile.", "image.", or "limits.".
// with "volatile." or "image.".
type configKeyValidator struct{}

func (v configKeyValidator) Description(ctx context.Context) string {
return fmt.Sprintf("config key cannot have %q, %q, or %q prefix", "volatile.", "image.", "limits.")
return fmt.Sprintf("config key cannot have %q or %q prefix", "volatile.", "image.")
}
func (v configKeyValidator) MarkdownDescription(ctx context.Context) string {
return fmt.Sprintf("config key cannot have `%s`, `%s`, or `%s` prefix", "volatile.", "image.", "limits.")
return fmt.Sprintf("config key cannot have `%s` or `%s` prefix", "volatile.", "image.")
}

func (v configKeyValidator) ValidateString(ctx context.Context, req validator.StringRequest, resp *validator.StringResponse) {
value := req.ConfigValue.ValueString()

if strings.HasPrefix(value, "limits.") {
resp.Diagnostics.AddAttributeError(req.Path, "Invalid config key",
fmt.Sprintf("Config keys with %q prefix must be provided in %q map instead.", "limits.", "limits"),
)
}

if utils.HasAnyPrefix(value, []string{"volatile.", "image."}) {
resp.Diagnostics.AddAttributeError(req.Path, "Invalid config key",
fmt.Sprintf("Config key cannot have %q or %q prefix. Got: %q.", "volatile.", "image.", value),
Expand Down

0 comments on commit 3b88ad3

Please sign in to comment.