Add rootDiskAvailabilityZone to MachineClass

[rhizoet]

How to categorize this PR?
/area storage
/kind enhancement
/platform openstack

What this PR does / why we need it:
Adds an optional rootDiskAvailabilityZone field to the MachineClass provider spec.

If rootDiskType is set, the machine-controller-manager creates the root volume in Cinder itself and passes the availability zone of the machine (spec.availabilityZone) to Cinder. On clouds where the compute and Cinder availability zones are named differently (e.g. compute zone AZ1, Cinder zone nova, common for SCS clouds), Cinder rejects this with Availability zone 'AZ1' is invalid.
The new field controls the zone used for the root volume:

• unset: the availability zone of the machine is used (unchanged behavior)
• empty string: no availability zone is passed, Cinder uses its default zone (default_availability_zone, falling back to storage_availability_zone)
• any other value: the root volume is created in that zone

Without a rootDiskType, Nova creates the volume itself, so nothing changes there.

Which issue(s) this PR fixes:
Part of gardener/gardener-extension-provider-openstack#1229

Special notes for your reviewer:
The extension counterpart (CloudProfileConfig.rootDiskAvailabilityZone) follows once this is released.
Not yet tested against a cloud with differing compute and Cinder zones. Unit tests cover the zone selection.
The generated code was adapted by hand.

Release note:

The `MachineClass` of the OpenStack provider supports the new optional field `rootDiskAvailabilityZone` to create root disk volumes in an availability zone different from the one of the machine. This allows for differing compute and Cinder availability zones.

[gardener-prow]

Welcome @rhizoet!

It looks like this is your first PR to gardener/machine-controller-manager-provider-openstack 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if gardener/machine-controller-manager-provider-openstack has its own contribution guidelines.

Thank you, and welcome to Gardener. 😃

[federated-github-access]

The PR needs to be labeled with ok-to-test by a maintainer to trigger the automated validation of the change

[aaronfern]

Thanks for the PR @rhizoet!

The generated code was adapted by hand.

Why was this done? make generate would normally generate all files properly. In fact, now that I ran make generate, it did make further changes. Could you do this instead of adapting this code by hand?

Not yet tested against a cloud with differing compute and Cinder zones.

I assume it's because the openstack installation you have does not contain differing zones (Please correct me if I'm wrong). If that is the case then what is the purpose of this PR? is it for expected changes that will come up, or is it for theoretical completion?

[federated-github-access]

The PR needs to be labeled with ok-to-test by a maintainer to trigger the automated validation of the change

[rhizoet]

Thanks for the PR @rhizoet!

The generated code was adapted by hand.

Why was this done? make generate would normally generate all files properly. In fact, now that I ran make generate, it did make further changes. Could you do this instead of adapting this code by hand?

Not yet tested against a cloud with differing compute and Cinder zones.

I assume it's because the openstack installation you have does not contain differing zones (Please correct me if I'm wrong). If that is the case then what is the purpose of this PR? is it for expected changes that will come up, or is it for theoretical completion?

Thanks for the review @aaronfern!

Generated code: That was my mistake. I was new to the repo's tooling and didn't realise make generate would handle this, so I adapted the files by hand. I've now run make generate (including make format) and committed the result. A second run produces no further changes, and make check and the unit tests pass.

Validation: Good point, thanks. I added a check in validation.go that rejects rootDiskAvailabilityZone when rootDiskType is not set, plus unit tests for both cases.

Purpose of the PR: This is not theoretical. A customer has a compute zone (AZ1) that differs from their Cinder zone (nova). Creating a Shoot currently fails there, because the root volume is created with the machine's compute zone and Cinder rejects it with "Availability zone is invalid". The new optional field lets the MachineClass set the volume zone separately. If it is unset, the machine zone is used as before, so existing setups are unaffected.

Testing: I don't have an environment with differing compute and Cinder zones, so this is covered by unit tests only and has not been verified end to end. The change is small and backwards compatible, so I'd like to get it merged and validated once it is available in a release.

[aaronfern]

Alright, thank you for the explanation. The changes look fine now.
Thanks for the PR!
/lgtm

[gardener-prow]

LGTM label has been added.

Details

Git tree hash: 984ca63bce92b24db57ca744afc28daf57203fbe

[gardener-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AndreasBurger

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [AndreasBurger]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment