Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove limits #136

Merged
merged 3 commits into from
Sep 16, 2024
Merged

Remove limits #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14e0bad
instance: Remove limits
maveonair Sep 16, 2024
aa7645f
test: Remove limits
maveonair Sep 16, 2024
802cd7a
docs: Remove limits
maveonair Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions docs/resources/instance.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading