From 06849eab0b002ce2625df5e8c7e693617a50dfdc Mon Sep 17 00:00:00 2001 From: Vitaly Antonenko Date: Wed, 26 Jun 2024 21:31:40 +0300 Subject: [PATCH] WIP --- internal/provider/modifer_storageset.go | 14 +- internal/provider/resource_application.go | 148 +++++++++++++----- .../provider/resource_application_test.go | 15 +- 3 files changed, 125 insertions(+), 52 deletions(-) diff --git a/internal/provider/modifer_storageset.go b/internal/provider/modifer_storageset.go index 5e5d3d2d..084077d6 100644 --- a/internal/provider/modifer_storageset.go +++ b/internal/provider/modifer_storageset.go @@ -6,7 +6,6 @@ package provider import ( "context" "fmt" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier" jujustorage "github.com/juju/juju/storage" @@ -16,10 +15,10 @@ import ( // storageSetRequiresReplace is a plan modifier function that determines if the storage set requires a replace. // It compares the storage set in the plan with the storage set in the state. // Return false if new items were added and old items were not changed. -// Return true if old items were removed +// Return true if old items were removed. func storageSetRequiresReplace(ctx context.Context, req planmodifier.SetRequest, resp *setplanmodifier.RequiresReplaceIfFuncResponse) { planSet := make(map[string]jujustorage.Constraints) - if !req.PlanValue.IsNull() { + if !req.PlanValue.IsUnknown() { var planStorageSlice []nestedStorage resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &planStorageSlice, false)...) if resp.Diagnostics.HasError() { @@ -35,7 +34,7 @@ func storageSetRequiresReplace(ctx context.Context, req planmodifier.SetRequest, // Validate storage size parsedStorageSize, err := utils.ParseSize(storageSize) if err != nil { - resp.Diagnostics.AddError("1Invalid Storage Size", fmt.Sprintf("1Invalid storage size %q: %s", storageSize, err)) + resp.Diagnostics.AddError("Invalid Storage Size", fmt.Sprintf("Invalid storage size %q: %s", storageSize, err)) return } @@ -47,11 +46,8 @@ func storageSetRequiresReplace(ctx context.Context, req planmodifier.SetRequest, } } } - stateSet := make(map[string]jujustorage.Constraints) - - // print the state stateSet - if !req.StateValue.IsNull() { + if !req.StateValue.IsUnknown() { var stateStorageSlice []nestedStorage resp.Diagnostics.Append(req.StateValue.ElementsAs(ctx, &stateStorageSlice, false)...) if resp.Diagnostics.HasError() { @@ -67,7 +63,7 @@ func storageSetRequiresReplace(ctx context.Context, req planmodifier.SetRequest, // Validate storage size parsedStorageSize, err := utils.ParseSize(storageSize) if err != nil { - resp.Diagnostics.AddError("2Invalid Storage Size", fmt.Sprintf("2Invalid storage size %q [%q]: %s", storageSize, stateStorageSlice, err)) + resp.Diagnostics.AddError("Invalid Storage Size", fmt.Sprintf("Invalid storage size %q [%q]: %s", storageSize, stateStorageSlice, err)) return } diff --git a/internal/provider/resource_application.go b/internal/provider/resource_application.go index 2117de7a..f3ea6f1b 100644 --- a/internal/provider/resource_application.go +++ b/internal/provider/resource_application.go @@ -5,7 +5,9 @@ package provider import ( "context" + "encoding/json" "fmt" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier" "strings" "github.com/dustin/go-humanize" @@ -21,8 +23,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default" "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/setplanmodifier" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/tfsdk" @@ -173,6 +173,7 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest "storage": schema.SetNestedAttribute{ Description: "Configure storage constraints for the juju application.", Optional: true, + Computed: true, NestedObject: schema.NestedAttributeObject{ Attributes: map[string]schema.Attribute{ "label": schema.StringAttribute{ @@ -186,7 +187,6 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest Description: "The size of each volume. E.g. 100G.", Optional: true, Computed: true, - Default: stringdefault.StaticString("1G"), PlanModifiers: []planmodifier.String{ stringplanmodifier.UseStateForUnknown(), }, @@ -194,6 +194,7 @@ func (r *applicationResource) Schema(_ context.Context, _ resource.SchemaRequest "pool": schema.StringAttribute{ Description: "Name of the storage pool to use. E.g. ebs on aws.", Optional: true, + Computed: true, PlanModifiers: []planmodifier.String{ stringplanmodifier.UseStateForUnknown(), }, @@ -536,7 +537,7 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq // Parse storage var storageConstraints map[string]jujustorage.Constraints - if !plan.Storage.IsNull() { + if !plan.Storage.IsUnknown() { var storageSlice []nestedStorage resp.Diagnostics.Append(plan.Storage.ElementsAs(ctx, &storageSlice, false)...) if resp.Diagnostics.HasError() { @@ -552,10 +553,14 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq storageCount := storage.Count.ValueInt64() // Validate storage size - parsedStorageSize, err := utils.ParseSize(storageSize) - if err != nil { - resp.Diagnostics.AddError("3Invalid Storage Size", fmt.Sprintf("3Invalid storage size %q: %s", storageSize, err)) - return + var parsedStorageSize uint64 + if storageSize != "" { + var err error + parsedStorageSize, err = utils.ParseSize(storageSize) + if err != nil { + resp.Diagnostics.AddError("Invalid Storage Size", fmt.Sprintf("Invalid storage size %q: %s", storageSize, err)) + return + } } storageConstraints[storageName] = jujustorage.Constraints{ @@ -592,6 +597,21 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to create application, got error: %s", err)) return } + + // trace storage constraints + r.trace("create application storage constraints", map[string]interface{}{"storageConstraints": storageConstraints}) + + // write storage constraints to private state + var setStorageConstraintBytes []byte + if len(storageConstraints) > 0 { + setStorageConstraintBytes, err = json.Marshal(storageConstraints) + if err != nil { + resp.Diagnostics.AddError("Internal Error", fmt.Sprintf("Unable to marshal storage constraints, got error: %s", err)) + return + } + } + resp.Private.SetKey(ctx, "storage", setStorageConstraintBytes) + r.trace(fmt.Sprintf("create application resource %q", createResp.AppName)) readResp, err := r.client.Applications.ReadApplicationWithRetryOnNotFound(ctx, &juju.ReadApplicationInput{ @@ -620,6 +640,45 @@ func (r *applicationResource) Create(ctx context.Context, req resource.CreateReq resp.Diagnostics.Append(dErr...) return } + + if plan.Storage.IsUnknown() { + var getStorageConstraintBytes []byte + getStorageConstraintBytes, dErr = resp.Private.GetKey(ctx, "storage") + if dErr.HasError() { + resp.Diagnostics.Append(dErr...) + return + } + var privateStorageConstraints map[string]jujustorage.Constraints + if len(getStorageConstraintBytes) > 0 { + if err = json.Unmarshal(getStorageConstraintBytes, &privateStorageConstraints); err != nil { + resp.Diagnostics.AddError("Internal Error", fmt.Sprintf("Unable to unmarshal storage constraints, got error: %s", err)) + return + } + } + storageType := req.Config.Schema.GetAttributes()[StorageKey].(schema.SetNestedAttribute).NestedObject.Type() + var nestedStorageSlice []nestedStorage + for name, storage := range readResp.Storage { + if _, ok := privateStorageConstraints[name]; !ok { + humanizedSize := transformSizeToHumanizedFormat(storage.Size) + nestedStorageSlice = append(nestedStorageSlice, nestedStorage{ + Label: types.StringValue(name), + Size: types.StringValue(humanizedSize), + Pool: types.StringValue(storage.Pool), + Count: types.Int64Value(int64(storage.Count)), + }) + } + } + if len(nestedStorageSlice) > 0 { + plan.Storage, dErr = types.SetValueFrom(ctx, storageType, nestedStorageSlice) + if dErr.HasError() { + resp.Diagnostics.Append(dErr...) + return + } + } else { + plan.Storage = types.SetNull(storageType) + } + } + plan.ID = types.StringValue(newAppID(plan.ModelName.ValueString(), createResp.AppName)) r.trace("Created", applicationResourceModelForLogging(ctx, &plan)) @@ -687,7 +746,7 @@ func (r *applicationResource) Read(ctx context.Context, req resource.ReadRequest if response == nil { return } - r.trace(fmt.Sprint("read application", map[string]interface{}{"resource": appName, "response": response})) + r.trace("read application", map[string]interface{}{"resource": appName, "response": response}) state.ApplicationName = types.StringValue(appName) state.ModelName = types.StringValue(modelName) @@ -747,33 +806,46 @@ func (r *applicationResource) Read(ctx context.Context, req resource.ReadRequest } } - // convert the storage map to a list of nestedStorage - nestedStorageSlice := make([]nestedStorage, 0, len(response.Storage)) - for name, storage := range response.Storage { - humanizedSize := transformSizeToHumanizedFormat(storage.Size) - - if storage.Pool != "lxd" { - nestedStorageSlice = append(nestedStorageSlice, nestedStorage{ - Label: types.StringValue(name), - Size: types.StringValue(humanizedSize), - Pool: types.StringValue(storage.Pool), - Count: types.Int64Value(int64(storage.Count)), - }) - } else { - // 'lxd' is not a pool, it's a special case used for lxd storage. - nestedStorageSlice = append(nestedStorageSlice, nestedStorage{ - Label: types.StringValue(name), - Size: types.StringValue(humanizedSize), - Count: types.Int64Value(int64(storage.Count)), - }) - } - storageType := req.State.Schema.GetAttributes()[StorageKey].(schema.SetNestedAttribute).NestedObject.Type() - state.Storage, dErr = types.SetValueFrom(ctx, storageType, nestedStorageSlice) - if dErr.HasError() { - resp.Diagnostics.Append(dErr...) + var getPrivateStorageConstraintBytes []byte + getPrivateStorageConstraintBytes, dErr = req.Private.GetKey(ctx, "storage") + if dErr.HasError() { + resp.Diagnostics.Append(dErr...) + return + } + var privateStorageConstraints map[string]jujustorage.Constraints + if len(getPrivateStorageConstraintBytes) > 0 { + if err := json.Unmarshal(getPrivateStorageConstraintBytes, &privateStorageConstraints); err != nil { + resp.Diagnostics.AddError("Internal Error", fmt.Sprintf("Unable to unmarshal storage constraints, got error: %s", err)) return } } + // trace private storage constraints + r.trace("read private storage constraints", map[string]interface{}{"privateStorageConstraints": privateStorageConstraints}) + + if state.Storage.IsUnknown() { + // convert the storage map to a list of nestedStorage + nestedStorageSlice := make([]nestedStorage, 0, len(response.Storage)) + for name, storage := range response.Storage { + if _, ok := privateStorageConstraints[name]; !ok { + humanizedSize := transformSizeToHumanizedFormat(storage.Size) + nestedStorageSlice = append(nestedStorageSlice, nestedStorage{ + Label: types.StringValue(name), + Size: types.StringValue(humanizedSize), + Pool: types.StringValue(storage.Pool), + Count: types.Int64Value(int64(storage.Count)), + }) + } + } + if len(nestedStorageSlice) > 0 { + storageType := req.State.Schema.GetAttributes()[StorageKey].(schema.SetNestedAttribute).NestedObject.Type() + state.Storage, dErr = types.SetValueFrom(ctx, storageType, nestedStorageSlice) + if dErr.HasError() { + resp.Diagnostics.Append(dErr...) + return + } + } + } + resourceType := req.State.Schema.GetAttributes()[ResourceKey].(schema.MapAttribute).ElementType state.Resources, dErr = r.configureResourceData(ctx, resourceType, state.Resources, response.Resources) if dErr.HasError() { @@ -1016,8 +1088,14 @@ func (r *applicationResource) Update(ctx context.Context, req resource.UpdateReq // Check if we have new storage in plan that not existed in the state, and add their constraints to the // update application input. if !plan.Storage.Equal(state.Storage) { - if r.updateStorage(ctx, resp, plan, state, updateApplicationInput) { - return + if !plan.Storage.IsUnknown() { + if r.updateStorage(ctx, resp, plan, state, updateApplicationInput) { + return + } + } + if len(updateApplicationInput.StorageConstraints) == 0 { + storageType := req.Config.Schema.GetAttributes()[StorageKey].(schema.SetNestedAttribute).NestedObject.Type() + plan.Storage = types.SetNull(storageType) } } diff --git a/internal/provider/resource_application_test.go b/internal/provider/resource_application_test.go index 8d8aad48..2b0406f9 100644 --- a/internal/provider/resource_application_test.go +++ b/internal/provider/resource_application_test.go @@ -155,9 +155,15 @@ func TestAcc_ResourceApplication_UpdatesRevisionConfig(t *testing.T) { if testingCloud != LXDCloudTesting { t.Skip(t.Name() + " only runs with LXD") } + modelName := acctest.RandomWithPrefix("tf-test-application") appName := "github-runner" configParamName := "runner-storage" + + // plan for step 1 + t.Log(testAccResourceApplicationWithRevisionAndConfig(modelName, appName, 88, "", "", -1)) + // print plan for step 2 + t.Log(testAccResourceApplicationWithRevisionAndConfig(modelName, appName, 96, configParamName, "", -1)) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: frameworkProviderFactories, @@ -516,9 +522,6 @@ func TestAcc_ResourceApplication_Storage(t *testing.T) { storageConstraints := map[string]string{"label": "files", "size": "102M"} - // print plan - t.Log(testAccResourceApplicationStorage(modelName, appName, storageConstraints)) - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: frameworkProviderFactories, @@ -528,7 +531,7 @@ func TestAcc_ResourceApplication_Storage(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr("juju_application."+appName, "model", modelName), resource.TestCheckResourceAttr("juju_application."+appName, "storage.#", "1"), - resource.TestCheckTypeSetElemNestedAttrs("juju_application."+appName, "storage.*", map[string]string{"size": "102M", "label": "files"}), + resource.TestCheckTypeSetElemNestedAttrs("juju_application."+appName, "storage.*", storageConstraints), ), }, { @@ -617,10 +620,6 @@ resource "juju_application" "{{.AppName}}" { config = { {{.ConfigParamName}} = "{{.ConfigParamName}}-value" } - storage = [{ - label = "runner" - size = "107G" - }] {{ end }} {{ if ne .ResourceParamName "" }}