-
Notifications
You must be signed in to change notification settings - Fork 8
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.
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
added flexcache resource, data source and data sources. #134
added flexcache resource, data source and data sources. #134
Conversation
The provider.tf in examples should be a link to examples/provider/provider.tf |
Missing doc and changelog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You missing the documentation pages.
examples/data-sources/netapp-ontap_storage_flexcache/provider.tf
Outdated
Show resolved
Hide resolved
examples/data-sources/netapp-ontap_storage_flexcache/provider.tf
Outdated
Show resolved
Hide resolved
examples/data-sources/netapp-ontap_storage_flexcache/provider.tf
Outdated
Show resolved
Hide resolved
examples/data-sources/netapp-ontap_storage_flexcaches/provider.tf
Outdated
Show resolved
Hide resolved
examples/data-sources/netapp-ontap_storage_flexcaches/provider.tf
Outdated
Show resolved
Hide resolved
…m-provider-netapp-ontap into 46-new-resource-storageflexcacheflexcaches
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the document with example section and Related ONTAP commands section.
data source: https://github.com/NetApp/terraform-provider-netapp-ontap/blob/integration/main/docs/data-sources/cluster_data_source.md?plain=1
resource: https://github.com/NetApp/terraform-provider-netapp-ontap/blob/integration/main/docs/resources/cluster_licensing_license_resource.md?plain=1
Also the modification function and its acc test.
Computed: true, | ||
Optional: true, | ||
}, | ||
"size_unit": schema.StringAttribute{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line 410 to me it's required. Or should this have a default value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“If not specified in POST, the following default property values are assigned:
size - 10% of origin volume size or 1GB per constituent, whichever is greater.”
It should just let the API decides what to set if user didn't give a input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If so, then on line 410 needs to be adjusted. If the user does not provide size and size unit, it will be failed with on line 410. i would suggest to have an acc test with minimum configuration (without size, size unit,...) to see if that works.
MarkdownDescription: "Name of the svm to use", | ||
Required: true, | ||
}, | ||
//there could be a space not enough or storage type error if the aggreates are not set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the comment, then aggregate should be required, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it doesn't have to be. It will be auto assign it no aggregate given. But that's not guarantee to work.
…cacheflexcaches added flexcache resource, data source and data sources.
#46
#47