-
Notifications
You must be signed in to change notification settings - Fork 43
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
reafactor: many small fixes accross all storage functions.
refactor: start using juju/utils/v3 insead v4 chore: fix commentary for full status reasoning test: add unit test for StorageNotFound error chore: fix StorageContraints typo to StorageConstraints. chore: fix resource_application.go imports. chore: add comments to the storageSetRequiresReplace func. chore: Move from ValueSting to Sting in Sprintf chore: misseing error check chore: add returning errors to resp.Diagnostics refactor: move update storage to a separate method refactor: change channel on revision, not to break test in the future. refactor: move storageSetRequiresReplace to separate file fix: unit test for Storage not found (retry logic) fix: golangci-lint small fixes. chore: fix copyright issue.
- Loading branch information
Showing
10 changed files
with
283 additions
and
201 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ resource "juju_machine" "this_machine" { | |
|
|
||
| - `base` (String) The operating system to install on the new machine(s). E.g. [email protected]. | ||
| - `constraints` (String) Machine constraints that overwrite those available from 'juju get-model-constraints' and provider's defaults. | ||
| - `disks` (String) StorageContraints constraints for disks to attach to the machine(s). | ||
| - `disks` (String) Storage constraints for disks to attach to the machine(s). | ||
| - `name` (String) A name for the machine resource in Terraform. | ||
| - `placement` (String) Additional information about how to allocate the machine in the cloud. | ||
| - `private_key_file` (String) The file path to read the private key from. | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -298,6 +298,72 @@ func (s *ApplicationSuite) TestReadApplicationRetrySubordinate() { | |
| s.Assert().Equal("[email protected]", resp.Base) | ||
| } | ||
|
|
||
| // TestReadApplicationRetryNotFoundStorageNotFoundError tests the case where the first response is a storage not found error. | ||
| // The second response is a real application. | ||
| func (s *ApplicationSuite) TestReadApplicationRetryNotFoundStorageNotFoundError() { | ||
| defer s.setupMocks(s.T()).Finish() | ||
| s.mockSharedClient.EXPECT().ModelType(gomock.Any()).Return(model.IAAS, nil).AnyTimes() | ||
|
|
||
| appName := "testapplication" | ||
| aExp := s.mockApplicationClient.EXPECT() | ||
|
|
||
| // First response is a storage not found error. | ||
| aExp.ApplicationsInfo(gomock.Any()).Return([]params.ApplicationInfoResult{{ | ||
| Error: ¶ms.Error{Message: `storage "testapplication" not found`, Code: "not found"}, | ||
| }}, nil) | ||
|
|
||
| // Retry - expect ApplicationsInfo and Status to be called. | ||
| // The second time return a real application. | ||
| amdConst := constraints.MustParse("arch=amd64") | ||
| infoResult := params.ApplicationInfoResult{ | ||
| Result: ¶ms.ApplicationResult{ | ||
| Tag: names.NewApplicationTag(appName).String(), | ||
| Charm: "ch:amd64/jammy/testcharm-5", | ||
| Base: params.Base{Name: "ubuntu", Channel: "22.04"}, | ||
| Channel: "stable", | ||
| Constraints: amdConst, | ||
| Principal: true, | ||
| }, | ||
| Error: nil, | ||
| } | ||
|
|
||
| aExp.ApplicationsInfo(gomock.Any()).Return([]params.ApplicationInfoResult{infoResult}, nil) | ||
| getResult := ¶ms.ApplicationGetResults{ | ||
| Application: appName, | ||
| CharmConfig: nil, | ||
| ApplicationConfig: nil, | ||
| Charm: "ch:amd64/jammy/testcharm-5", | ||
| Base: params.Base{Name: "ubuntu", Channel: "22.04"}, | ||
| Channel: "stable", | ||
| Constraints: amdConst, | ||
| EndpointBindings: nil, | ||
| } | ||
| aExp.Get("master", appName).Return(getResult, nil) | ||
| statusResult := ¶ms.FullStatus{ | ||
| Applications: map[string]params.ApplicationStatus{appName: { | ||
| Charm: "ch:amd64/jammy/testcharm-5", | ||
| Units: map[string]params.UnitStatus{"testapplication/0": { | ||
| Machine: "0", | ||
| }}, | ||
| }}, | ||
| } | ||
| s.mockClient.EXPECT().Status(gomock.Any()).Return(statusResult, nil) | ||
|
|
||
| client := s.getApplicationsClient() | ||
| resp, err := client.ReadApplicationWithRetryOnNotFound(context.Background(), | ||
| &ReadApplicationInput{ | ||
| ModelName: s.testModelName, | ||
| AppName: appName, | ||
| }) | ||
| s.Require().NoError(err, "error from ReadApplicationWithRetryOnNotFound") | ||
| s.Require().NotNil(resp, "ReadApplicationWithRetryOnNotFound response") | ||
|
|
||
| s.Assert().Equal("testcharm", resp.Name) | ||
| s.Assert().Equal("stable", resp.Channel) | ||
| s.Assert().Equal(5, resp.Revision) | ||
| s.Assert().Equal("[email protected]", resp.Base) | ||
| } | ||
|
|
||
| // In order for 'go test' to run this suite, we need to create | ||
| // a normal test function and pass our suite to suite.Run | ||
| func TestApplicationSuite(t *testing.T) { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| // Copyright 2024 Canonical Ltd. | ||
| // Licensed under the AGPLv3, see LICENCE file for details. | ||
|
|
||
| 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" | ||
| "github.com/juju/utils/v3" | ||
| ) | ||
|
|
||
| // 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 | ||
| func storageSetRequiresReplace(ctx context.Context, req planmodifier.SetRequest, resp *setplanmodifier.RequiresReplaceIfFuncResponse) { | ||
| planSet := make(map[string]jujustorage.Constraints) | ||
| if !req.PlanValue.IsNull() { | ||
| var planStorageSlice []nestedStorage | ||
| resp.Diagnostics.Append(req.PlanValue.ElementsAs(ctx, &planStorageSlice, false)...) | ||
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } | ||
| if len(planStorageSlice) > 0 { | ||
| for _, storage := range planStorageSlice { | ||
| storageName := storage.Label.ValueString() | ||
| storageSize := storage.Size.ValueString() | ||
| storagePool := storage.Pool.ValueString() | ||
| storageCount := storage.Count.ValueInt64() | ||
|
|
||
| // Validate storage size | ||
| parsedStorageSize, err := utils.ParseSize(storageSize) | ||
| if err != nil { | ||
| resp.Diagnostics.AddError("Invalid Storage Size", fmt.Sprintf("Invalid storage size %q: %s", storageSize, err)) | ||
| return | ||
| } | ||
|
|
||
| planSet[storageName] = jujustorage.Constraints{ | ||
| Size: parsedStorageSize, | ||
| Pool: storagePool, | ||
| Count: uint64(storageCount), | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| stateSet := make(map[string]jujustorage.Constraints) | ||
| if !req.StateValue.IsNull() { | ||
| var stateStorageSlice []nestedStorage | ||
| resp.Diagnostics.Append(req.StateValue.ElementsAs(ctx, &stateStorageSlice, false)...) | ||
| if resp.Diagnostics.HasError() { | ||
| return | ||
| } | ||
| if len(stateStorageSlice) > 0 { | ||
| for _, storage := range stateStorageSlice { | ||
| storageName := storage.Label.ValueString() | ||
| storageSize := storage.Size.ValueString() | ||
| storagePool := storage.Pool.ValueString() | ||
| storageCount := storage.Count.ValueInt64() | ||
|
|
||
| // Validate storage size | ||
| parsedStorageSize, err := utils.ParseSize(storageSize) | ||
| if err != nil { | ||
| resp.Diagnostics.AddError("Invalid Storage Size", fmt.Sprintf("Invalid storage size %q: %s", storageSize, err)) | ||
| return | ||
| } | ||
|
|
||
| stateSet[storageName] = jujustorage.Constraints{ | ||
| Size: parsedStorageSize, | ||
| Pool: storagePool, | ||
| Count: uint64(storageCount), | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Return false if new items were added and old items were not changed | ||
| for key, value := range planSet { | ||
| stateValue, ok := stateSet[key] | ||
| if !ok { | ||
| resp.RequiresReplace = false | ||
| return | ||
| } | ||
| if (value.Size != stateValue.Size) || (value.Pool != stateValue.Pool) || (value.Count != stateValue.Count) { | ||
| resp.RequiresReplace = true | ||
| return | ||
| } | ||
| } | ||
|
|
||
| // Return true if old items were removed | ||
| for key := range stateSet { | ||
| if _, ok := planSet[key]; !ok { | ||
| resp.RequiresReplace = true | ||
| return | ||
| } | ||
| } | ||
|
|
||
| resp.RequiresReplace = false | ||
| } |
Oops, something went wrong.