Fix: Fixes host edit to show correct vm_attributes which were used while creating host.#41
Conversation
m-bucher
left a comment
m-bucher
left a comment
There was a problem hiding this comment.
First test looks promising, apart from some smaller things I had to fix in the template 😄
IMHO disk-rendering might need some more cleanup.
Manisha15
force-pushed
the
fix_edit_host
branch
2 times, most recently
from
c0d97b1 to
271e9cd
Compare
…ile creating host. Also normalizes volume attributes as volumes_attributes to be consistent.
| def normalize_interfaces(vm_attrs) | ||
| attrs = vm_attrs.with_indifferent_access | ||
| return attrs if attrs[:interfaces_attributes].present? | ||
|
|
||
| networks = extract_networks(attrs) | ||
| return attrs if networks.blank? | ||
|
|
||
| attrs[:interfaces_attributes] = build_interfaces_attributes(networks) | ||
| attrs | ||
| end | ||
|
|
||
| def extract_networks(attrs) | ||
| attrs.dig(:vm, :network) || | ||
| attrs.dig('vm', 'network') || | ||
| attrs[:network] || | ||
| attrs['network'] | ||
| end | ||
|
|
||
| def build_interfaces_attributes(networks) | ||
| Array(networks).each_with_index.to_h do |net, idx| | ||
| nic = net.respond_to?(:with_indifferent_access) ? net.with_indifferent_access : net | ||
| [idx.to_s, { network_id: nic[:network_id].to_s }] | ||
| end | ||
| end |
There was a problem hiding this comment.
These seem pretty generic to me. If so, they should be moved into ProviderType so we can use them in all ProviderTypes.
| disk_render_source.each_with_index.filter_map do |disk, index| | ||
| # drop removed disks; tofu will drop them automatically, if they are no longer defined | ||
| next if disk['_delete'].to_i == 1 | ||
| next if disk.respond_to?(:[]) && disk['_delete'].to_i == 1 |
There was a problem hiding this comment.
This is a problem if on hetzner provider all attached volumes should be removed.
In my test, removing all volumes from a Hetzner host resulted in the disk_renderer() of the ProviderType not rendering anything. This is because if all volumes have set '_delete' to 1, then the rendered_disk() is never called.
This somehow clashed with the output-block in the template
volumes_attributes = try({
for v in values(hcloud_volume.volumes) : tostring(v.id) => {
id = v.id
name = v.name
size = v.size
automount = try(v.automount, null)
format = try(v.format, null)
}
}, {})
Tofu complained that no hcloud_volume.volumes are set.
A quick fix is to remove this line, but I assume it is needed for any other ProviderType we currently support :-/
m-bucher
left a comment
m-bucher
left a comment
There was a problem hiding this comment.
Works in combination with #39 if using non-UserData deployment (apart of the mentioned issue in regard to removing all attached volumes).
The problem with UserData deployment is that the UserData is detected as having changed.
IMHO none of the above Known-Issues should keep us from merging this, because it does improve Host edit nevertheless.
3 participants
Normalizes volume attributes as volumes_attributes to be consistent.