Skip to content

Commit

Permalink
Set default region back to empty string (#110)
Browse files Browse the repository at this point in the history
For all plugins - block, file share and object storage the default region name 
will be `""` and plugin will respect setting of `region_name` configured in clouds.yaml.

For non-documented reason we switched from having default value for
region to be empty string to be value `RegionOne` (in tag `v0.4.0`). `RegionOne` is usually default region name
in Openstack, but this doesn't justify leaving region to default to this value.

Having region as empty string makes the gophercloud library to pick
first found region in the catalog. This can cause potential backwards
compatibility issues, which are however very unlikely. The release notes will mention that.

Negative impact of having region default to `RegionOne` is that it makes
the plugin to not work in case user relies on region specified in
clouds.yaml.

Signed-off-by: Ondrej Vasko <[email protected]>
  • Loading branch information
Lirt authored Apr 7, 2024
1 parent a665700 commit 3c925de
Show file tree
Hide file tree
Showing 13 changed files with 576 additions and 4 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*.so
*.dylib

velero-plugin-for-openstack

# Test binary, built with `go test -c`
*.test

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ Recommended way of using this plugin with restic is to use authentication with e

```bash
# test and build code
go test -v ./...
go test -v -count 1 ./...
go mod tidy
go build

Expand Down
16 changes: 16 additions & 0 deletions docs/installation-using-cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ metadata:
spec:
provider: community.openstack.org/openstack-cinder
config:
# optional Cloud:
# in case clouds.yaml is used as authentication method, cloud allows
# user to select which cloud from the clouds.yaml to use for volume backups
cloud: ""
# optional Region:
# in case multiple regions exist in a single cloud, select which region
# will be used for cinder volume backups.
region: ""
# optional snapshot method:
# * "snapshot" is a default cinder snapshot method
# * "clone" is for a full volume clone instead of a snapshot allowing the
Expand Down Expand Up @@ -89,6 +97,14 @@ metadata:
spec:
provider: community.openstack.org/openstack-manila
config:
# optional Cloud:
# in case clouds.yaml is used as authentication method, cloud allows user
# to select which cloud from the clouds.yaml to use for manila share backups
cloud: ""
# optional Region:
# in case multiple regions exist in a single cloud, select which region
# will be used for manila share backups.
region: ""
# optional snapshot method:
# * "snapshot" is a default manila snapshot method
# * "clone" is for a full share clone instead of a snapshot allowing the
Expand Down
16 changes: 16 additions & 0 deletions docs/installation-using-helm.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ configuration:
- name: cinder
provider: community.openstack.org/openstack-cinder
config:
# optional Cloud:
# in case clouds.yaml is used as authentication method, cloud allows
# user to select which cloud from the clouds.yaml to use for volume backups
cloud: ""
# optional Region:
# in case multiple regions exist in a single cloud, select which region
# will be used for cinder volume backups.
region: ""
# optional snapshot method:
# * "snapshot" is a default cinder snapshot method
# * "clone" is for a full volume clone instead of a snapshot allowing the
Expand Down Expand Up @@ -61,6 +69,14 @@ configuration:
- name: manila
provider: community.openstack.org/openstack-manila
config:
# optional Cloud:
# in case clouds.yaml is used as authentication method, cloud allows user
# to select which cloud from the clouds.yaml to use for manila share backups
cloud: ""
# optional Region:
# in case multiple regions exist in a single cloud, select which region
# will be used for manila share backups.
region: ""
# optional snapshot method:
# * "snapshot" is a default manila snapshot method
# * "clone" is for a full share clone instead of a snapshot allowing the
Expand Down
2 changes: 1 addition & 1 deletion src/cinder/block_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (b *BlockStore) Init(config map[string]string) error {
if config["region"] != "" {
region = config["region"]
} else {
region = "RegionOne"
region = ""
}
}
b.client, err = openstack.NewBlockStorageV3(b.provider, gophercloud.EndpointOpts{
Expand Down
171 changes: 171 additions & 0 deletions src/cinder/block_store_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
package cinder

import (
"fmt"
"net/http"
"testing"

th "github.com/gophercloud/gophercloud/testhelper"
fakeClient "github.com/gophercloud/gophercloud/testhelper/client"
"github.com/sirupsen/logrus"
)

const ID = "0123456789"
const tokenResp = `{
"token": {
"audit_ids": ["VcxU2JYqT8OzfUVvrjEITQ", "qNUTIJntTzO1-XUk5STybw"],
"catalog": [
{
"endpoints": [
{
"id": "796186fced4611ee9e2c9cb6d0fbac9d",
"interface": "public",
"region": "RegionOne",
"url": "http://localhost:5000"
},
{
"id": "7c2bb2cced4611ee90c09cb6d0fbac9d",
"interface": "internal",
"region": "RegionOne",
"url": "http://localhost:5000"
},
{
"id": "8080e7b6ed4611ee88be9cb6d0fbac9d",
"interface": "admin",
"region": "RegionOne",
"url": "http://localhost:35357"
}
],
"id": "854d03ceed4611ee82b09cb6d0fbac9d",
"type": "identity",
"name": "keystone"
},
{
"endpoints": [
{
"id": "5fb3e04cc47345079bcccfa5a78d4de6",
"interface": "internal",
"region_id": "myRegion",
"url": "http://localhost:8776/v3/955f0136ed4611ee9f489cb6d0fbac9d",
"region": "myRegion"
},
{
"id": "d48c520ef7b941c692100f24a1437864",
"interface": "public",
"region_id": "myRegion",
"url": "https://localhost:8776/v3/955f0136ed4611ee9f489cb6d0fbac9d",
"region": "myRegion"
},
{
"id": "da15876d31f24af3afc3a69cb918c45f",
"interface": "admin",
"region_id": "myRegion",
"url": "https://localhost:8776/v3/955f0136ed4611ee9f489cb6d0fbac9d",
"region": "myRegion"
}
],
"id": "439e9f0d9d224b88a9b01774a9948e5e",
"type": "volumev3",
"name": "cinderv3"
},
{
"endpoints": [
{
"id": "2bed9ab4ed4111eeb4229cb6d0fbac9d",
"interface": "internal",
"region_id": "secondRegion",
"url": "http://localhost2:8776/v3/4c30519aed4111eeab909cb6d0fbac9d",
"region": "secondRegion"
},
{
"id": "3bd7f8caed4111eeb77a9cb6d0fbac9d",
"interface": "public",
"region_id": "secondRegion",
"url": "https://localhost2:8776/v3/4c30519aed4111eeab909cb6d0fbac9d",
"region": "secondRegion"
},
{
"id": "46474c98ed4111eeb2839cb6d0fbac9d",
"interface": "admin",
"region_id": "secondRegion",
"url": "https://localhost2:8776/v3/4c30519aed4111eeab909cb6d0fbac9d",
"region": "secondRegion"
}
],
"id": "4c30519aed4111eeab909cb6d0fbac9d",
"type": "volumev3",
"name": "cinderv3"
}
],
"expires_at": "2025-02-27T18:30:59.999999Z",
"is_domain": false,
"issued_at": "2025-02-27T16:30:59.999999Z",
"methods": [
"password"
],
"project": {
"domain": {
"id": "8789d1",
"name": "domain"
},
"id": "04982538-f42b-11ee-a412-9cb6d0fbac9d",
"name": "project"
},
"roles": [
{
"id": "86e72a",
"name": "admin"
},
{
"id": "e4f392",
"name": "member"
}
],
"user": {
"domain": {
"id": "8789d1",
"name": "domain"
},
"id": "cf78e694-f42a-11ee-bfcc-9cb6d0fbac9d",
"name": "user",
"password_expires_at": "2026-11-06T15:32:17.000000"
}
}
}`

// TestInit performs standard block store initialization
// which includes creation of auth client, authentication and
// creation of block storage client.
// In this test we use simple clouds.yaml and not override
// any option.
func TestSimpleBlockStorageInit(t *testing.T) {
// Basic structs
log := logrus.New()
config := map[string]string{
"cloud": "myCloud",
}
bs := NewBlockStore(log)

// Create fake provider client for authentication,
// prepare handler for authentication and redirect
// provider endpoint to fake client.
th.SetupPersistentPortHTTP(t, 32498)
defer th.TeardownHTTP()
fakeClient.ServiceClient()
bs.provider = fakeClient.ServiceClient().ProviderClient
bs.provider.IdentityEndpoint = th.Endpoint()

th.Mux.HandleFunc("/v3/auth/tokens",
func(w http.ResponseWriter, r *http.Request) {
w.Header().Add("X-Subject-Token", ID)

w.WriteHeader(http.StatusCreated)
fmt.Fprint(w, tokenResp)
},
)

// Try to Init block storage. This involves authentication.
if err := bs.Init(config); err != nil {
t.Error(err)
}
}
12 changes: 12 additions & 0 deletions src/cinder/clouds.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# This file is only to perform unit test with clouds.yaml for which the path cannot be changed
clouds:
myCloud:
auth:
user_domain_name: domain
auth_url: http://127.0.0.1:32498/v3
username: user
password: pass
project_name: project
project_domain_name: domain
# region_name: myRegion
identity_api_version: 3
12 changes: 12 additions & 0 deletions src/manila/clouds.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# This file is only to perform unit test with clouds.yaml for which the path cannot be changed
clouds:
myCloud:
auth:
user_domain_name: users
auth_url: http://127.0.0.1:32499/v3
username: user
password: pass
project_name: project
project_domain_name: domain
# region_name: myRegion
identity_api_version: 3
2 changes: 1 addition & 1 deletion src/manila/fs_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func (b *FSStore) Init(config map[string]string) error {
if config["region"] != "" {
region = config["region"]
} else {
region = "RegionOne"
region = ""
}
}
b.client, err = openstack.NewSharedFileSystemV2(b.provider, gophercloud.EndpointOpts{
Expand Down
Loading

0 comments on commit 3c925de

Please sign in to comment.