[Enhancement]: Add SVM storage_limit parameter
#328
Conversation
…m-provider-netapp-ontap into 140-new-resource-securityroles
…tarules new resource quota rules
…count-add-import-and-update-reapply 243 enhancement security account add import and update reapply
…-invalid-memory-address-or-nil-pointer-dereference fix san igroup
Fix script for Acc tests
…ficiency-policies
…ce-setup 250 bug error on cifs service setup
new resource colume efficiency policies
fix typo and markdown format
Signed-off-by: Achim Christ <[email protected]>
Signed-off-by: Achim Christ <[email protected]>
Signed-off-by: Achim Christ <[email protected]>
Signed-off-by: Achim Christ <[email protected]>
Signed-off-by: Achim Christ <[email protected]>
Signed-off-by: Achim Christ <[email protected]>
acch
force-pushed
the
302-svm-storage_limit-parameter
branch
from
2fcb74a to
b576f5f
Compare
|
@acch Happy Holidays, just a head up we will be reviewing this shorting. Sorry for the hold up we were waiting on Security to finish their review of 2.0 before releasing it and that ended up taking significantly longer than we had hope. Now that, that out of the way we are starting to merge thing in again. Right now I'm working on getting all the dependencies updated and then we'll start review these. Probably will have review the first week of Jan (or end of this week if I have time). |
|
Happy Holidays @carchi8py! Thanks for the update. Please let me know what I can do to help. Thanks much for all your efforts — enjoy a few days off! 🎄 |
Signed-off-by: Achim Christ <[email protected]>
|
Hello @chuyich, thank you very much for reviewing this PR. I have updated the docs and added cluster version checks to Thanks much! |
Signed-off-by: Achim Christ <[email protected]>
acch
force-pushed
the
302-svm-storage_limit-parameter
branch
from
19f6f0b to
4880190
Compare
| @@ -13,6 +13,7 @@ Retrieves the configuration of SVM | |||
| ## Supported Platforms | |||
|
|
|||
| * On-prem ONTAP system 9.6 or higher | |||
| * `storage_limit` attribute supported with ONTAP system 9.13 or higher | |||
There was a problem hiding this comment.
This needs to be added in the list of attributes on line 37-46
| @@ -13,6 +13,7 @@ Retrieves the configuration of SVMs. | |||
| ## Supported Platforms | |||
|
|
|||
| * On-prem ONTAP system 9.6 or higher | |||
| * `storage_limit` attribute supported with ONTAP system 9.13 or higher | |||
There was a problem hiding this comment.
This needs to be added in the list of attributes on line 64-72
| @@ -21,11 +21,12 @@ Create/Modify/Delete a SVM | |||
| ## Supported Platforms | |||
|
|
|||
| * On-prem ONTAP system 9.6 or higher | |||
| * `storage_limit` attribute supported with ONTAP system 9.13 or higher | |||
There was a problem hiding this comment.
This needs to be added in the list of attributes on line 47-63
| MarkdownDescription: "Maximum storage permitted on svm, in bytes", | ||
| Optional: true, | ||
| Computed: true, | ||
| Default: int64default.StaticInt64(0), |
There was a problem hiding this comment.
It is an optional parameter based on the API doc https://docs.netapp.com/us-en/ontap-restapi-9131/ontap/post-svm-svms.html#recommended-optional-properties
The default value is not needed.
There was a problem hiding this comment.
Without a default value, the attribute remains "unknown" after the apply - resulting in the following error:
╷
│ Error: Provider returned invalid result object after apply
│
│ After the apply operation, the provider still indicated an unknown value for netapp-ontap_svm.example.storage_limit. All values must be known after apply, so this is
│ always a bug in the provider and should be reported in the provider's own repository. Terraform will still save the other known object values in the state.
╵
I could manually set the value to 0 in internal/provider/svm/svm_resource.go > Create(), but isn't a default value in the schema definition easier to read & understand?
internal/interfaces/svm.go
Outdated
| Aggregates []map[string]string `mapstructure:"aggregates"` | ||
| Name string `mapstructure:"name,omitempty"` | ||
| SnapshotPolicy SnapshotPolicy `mapstructure:"snapshot_policy,omitempty"` | ||
| Storage Storage `mapstructure:"storage"` |
There was a problem hiding this comment.
Since it's optional, it should be set omitempty.
There was a problem hiding this comment.
OK, I did mark the optional storage and (nested) limit params as 'omitempty'. However, there's some code in place to delete empty params from the request body, which I've adapted to... don't think 'omitempty' will make any difference.
if setAggrEmpty {
delete(body, "aggregates")
}
if setCommentEmpty {
delete(body, "comment")
}
if setStorageLimitEmpty {
// delete storage.limit from request body, so that ONTAP uses default value
if v, ok := body["storage"].(map[string]interface{}); ok {
delete(v, "limit")
}
}Signed-off-by: Achim Christ <[email protected]>
|
Thanks @carchi8py and @chuyich for your feedback! I've added the I've also marked the optional params as 'omitempty', and did verify that acceptance tests (still) pass. Thanks much! |
Signed-off-by: Achim Christ <[email protected]>
acch
force-pushed
the
302-svm-storage_limit-parameter
branch
from
ceeb032 to
98e9570
Compare
wenjun666
force-pushed
the
integration/main
branch
from
0eaabad to
ed4593e
Compare
Adds ability to configure
storage_limitparameter of SVM.Closes #302.
Acceptance tests pass:
$ TF_ACC=1 go test ./internal/provider/svm/svm_resource_test.go -v === RUN TestAccSvmResource --- PASS: TestAccSvmResource (33.36s) PASS ok command-line-arguments 33.367sExample Terraform Configuration: