KarlJJonsson
commented
8 months ago Current workaround is to add the following configuration in addition to the clouds.yaml:
configuration:
extraEnvVars:
OS_REGION_NAME: <myRegion>Closed KarlJJonsson closed 8 months ago
KarlJJonsson
commented
8 months ago Current workaround is to add the following configuration in addition to the clouds.yaml:
configuration:
extraEnvVars:
OS_REGION_NAME: <myRegion>
Lirt
commented
8 months ago Hello @KarlJJonsson,
Checking the code I think we don't give Openstack config a chance to be respected :confused:. We try to override region first by ENV var OS_REGION_NAME and then by config["region"] (see the code). I will need to perform some tests to prove that and if that's right I will try to update the code to respect when the region_name is configured in clouds.yaml.
The latter one I mentioned (config["region"]) is something which could simplify your config and is officially supported (if you are not happy about current workaround until I check this topic). Cloud and region are configurable in VSL but not mentioned in the docs (I will also improve this part of documentation.)
---
apiVersion: velero.io/v1
kind: VolumeSnapshotLocation
metadata:
name: <NAME>
namespace: <NAMESPACE>
spec:
provider: community.openstack.org/openstack-cinder
config:
cloud: openstack
region: myRegion
...Also note OS_CLOUD can be replaced by config["cloud"] in the VSL (as seen above).
Lirt
commented
8 months ago After some research I see this:
This is the part of the code that decides which endpoint from a list of endpoints in Provider object to use (gophercloud code):
...
if (opts.Availability == gophercloud.Availability(endpoint.Interface)) &&
(opts.Region == "" || endpoint.Region == opts.Region || endpoint.RegionID == opts.Region) {
endpoints = append(endpoints, endpoint)
}
...It means that it searches for region if it is specified (non-empty). If region is empty, it picks the first found with "public" availability (default).
What we do in the code is this:
region, ok := os.LookupEnv("OS_REGION_NAME")
if !ok {
if config["region"] != "" {
region = config["region"]
} else {
region = "RegionOne"
}
}
b.client, err = openstack.NewBlockStorageV3(b.provider, gophercloud.EndpointOpts{
Region: region,
})So we default region to RegionOne. If there is only 1 CinderV3 endpoint, this doesn't matter. If there is more than 1 endpoint, it will prefer the one that matches region: "RegionOne".
Changing region to default to empty string "" would solve your issue. There could be possible side-effects:
RegionOne.RegionOne. In this case the plugin will pick the first one.On the other hand it might also be undesirable to prefer RegionOne in situation where there are more block storage regions.
Checking back in commit history I see that before v0.4.0 we defaulted region to empty string. The change to use RegionOne as default was added in v0.4.0. It makes sense for me to revert this back to empty string.
cc. @kayrus do you see this change breaking backwards compatibility in real-life scenarios?
PR draft: https://github.com/Lirt/velero-plugin-for-openstack/pull/110
KarlJJonsson
commented
8 months ago Hi @Lirt, sorry for late response. Easter got in the way.
The config section for VSL would be a good and clear enough solution for my part, I will go ahead and use this instead of the ENV var.
I don't have much insight into openstack or cinder, so I'm not aware of any standard defaults as RegionOne for example. Hence the following is just some thoughts from a outsider looking in:
When using the clouds.yaml and specifying the region_name while not specifying any region anywhere else, it feels intuitive that this would be the region picked by the plugin by default. Any additional configuration such as the config["region"] or OS_REGION_NAME would feel natural to take precedence since it's more fine grained or explicit.
This also looks slightly tricky as region in clouds.yaml doesn't neccesarilly mean that that region should be configured in Cinder.
From what I understand the regions in openstack and cinder might differ, which is why picking the clouds.yaml region_one might not be preferred? I think I would personally still prefer it to pick my specified region from clouds.yaml and have the plugin fail instead of defaulting to something unknown in the background.
This would also align with the openstack CLI as far as I can tell. If I set region_name: "NonExistingFakeRegion" in my clouds.yaml and run openstack volume list I get public endpoint for volumev3 service in NonExistingFakeRegion region not found.
KarlJJonsson
commented
8 months ago And to my understanding defaulting to an empty region would correspond to using the openstack CLI with a clouds.yaml without a region_name?
I think this is more what I had in mind:
region, ok := os.LookupEnv("OS_REGION_NAME")
if !ok {
if config["region"] != "" {
region = config["region"]
} else if {
// get region from clouds.yaml region_name
} else {
region = ""
}
}
b.client, err = openstack.NewBlockStorageV3(b.provider, gophercloud.EndpointOpts{
Region: region,
})When using the clouds.yaml and specifying the region_name while not specifying any region anywhere else, it feels intuitive that this would be the region picked by the plugin by default. Any additional configuration such as the config["region"] or OS_REGION_NAME would feel natural to take precedence since it's more fine grained or explicit.
I completely agree.
Regarding the region selection part of the code it's more complicated. Basically the whole code to pick the config from clouds.yaml comes from gophercloud functions clientconfig.AuthOptions(&clientOpts) and openstack.Authenticate() plus there are more modifications to support swiftstack and and some additional options.
There isn't much flexibility regarding authentication process without possibility to break some case, so I'd like to not modify this standardized part much (reacting to this code part):
} else if {
// get region from clouds.yaml region_nameSimplified, the code will automatically parse OS_ variables if set and try to use them or if not pick what matches in clouds.yaml. Then if region is empty, it will pick first found.
This would also align with the openstack CLI as far as I can tell. If I set region_name: "NonExistingFakeRegion" in my clouds.yaml and run openstack volume list I get public endpoint for volumev3 service in NonExistingFakeRegion region not found.
This scenario is handled :+1: .
And if you try it with empty --os-region - openstack volume list --os-region "" - it will work as well which tells me that to match Openstack default behavior we shouldn't force region to be RegionOne, but let it be empty as last resort (default behavior).
KarlJJonsson
commented
8 months ago Did alot of digging in the codebase and I think I'm onboard on how it works now :) I agree that the else if would just complicate things further.
What you're saying is how I'd imagine it to work before writing the issue, so it sounds great
Lirt
commented
8 months ago Great!
I will review my change in a few days and maybe add more tests. Then when it's merged I'll release as new minor version and note there potentially breaking change.
Lirt
commented
8 months ago Hello again,
Just FYI, I will update some dependencies and release this change soon as v0.7.0.
Thank you for report, discussion and patience :+1: .
Describe the bug When authenticating towards openstack using a clouds.yaml file the region_name is not correctly read and results in the following error:
failed to create cinder storage client: No suitable endpoint could be found in the service catalog.when backing up cinder backed persistent volumes. Currently using AWS provider plugin to connect to object storage so cannot speak for how this would affect backups of other resources.Setting the variable
OS_REGION_NAMEin the velero pod to the correct region solves the problem.Steps to reproduce the behavior
Deploy configmap with clouds.yaml:
deploy velero with clouds.yaml mounted as volume using the following snippet:
backup-test: trueExpected behavior Velero would create a backup in openstack of persistent volumes backed by cinder in the cluster by using the declared region in the clouds.yaml file. Just as it does if i add ENV var
OS_REGION_NAME: myRegion.Used versions
velero version): v1.13.0kubectl describe pod velero-...): lirt/velero-plugin-for-openstack:v0.6.1kubectl version): Server Version: v1.25.3+rke2r1openstack --version): openstack 6.3.0Link to velero or backup log
velero backup log demo-backup-3Log snippet around error:
Full log from backup: