Allow increasing the size of CSI volumes #382
Conversation
so that if they remove min/max after initial create, it doesn't perpetually show "something" -> null changes on every plan
| }) | ||
| } | ||
| } | ||
| capacityMin, capacityMax, capacityDiags := parseCapacity(d) |
| @@ -623,3 +690,107 @@ func flattenCSIVolumeTopologyRequests(topologyReqs *api.CSITopologyRequest) []in | |||
|
|
|||
| return topologyRequestList | |||
| } | |||
|
|
|||
| func csiErrIsRetryable(err error) bool { | |||
There was a problem hiding this comment.
Not the prettiest but we talked about it already :)
| {"10kib", "10 KiB"}, | ||
| {"5.5 Gib", "5.5 GiB"}, | ||
| {"5GB", "4.7 GiB"}, | ||
| {"ugh", "ugh"}, // validation happens elsewhere |
There was a problem hiding this comment.
lovely test values jejeje
There was a problem hiding this comment.
no test for empty values?
There was a problem hiding this comment.
oh sure fair point, here go: cc1511a
|
I didn't really start adding steps to acceptance tests, but your question @Juanadelacuesta was enough for me to go ahead and poke at it. The current challenge I have with it is that the publicly-available version of Nomad doesn't support volume expansion, and the
and it seems like the test framework -- specifically this All that to say, I think updating the acceptance test to confirm changes will make more sense after volume expansion in Nomad is released (and so available to pull in here). |
Nomad is enabling CSI volume expansion per hashicorp/nomad#10324 -- slated for the next
1.6.xrelease (looks like1.6.3specifically)To that end here,
capacity_minandcapacity_maxchanges will no longer force replacement ofnomad_csi_volumes,nomad_csi_volume_registrations can take those values too, and updating them appropriately can trigger a volume expansion for CSI plugins that support it.On older versions of Nomad, this should produce idempotent Create or Register API calls (for
nomad_csi_volumeandnomad_csi_volume_registrationrespectively), and if this TF provider detects that expansion has not occurred (by comparing against actual Capacity as returned by Nomad API), it will issue a warning to the user.I did not update
nomad_external_volumeas it's marked as deprecated, and honestly I considered excludingnomad_csi_volume_registrationtoo, because it didn't take capacity min/max to begin with. But! It shares chunks of code withnoamd_csi_volume, and I did update the Register Nomad API to behave the ~same as Create so registration came along too.There's quite a bit of unit test coverage of the new supporting functions, but very minimal change to the acceptance test. For example, from what I can tell there is nothing testing Update operations at all, and I have not really improved that situation. I can take a swing at it though if anyone would like to push me to.