[manila]: Add Share Replicas support by kayrus · Pull Request #2635 · gophercloud/gophercloud

kayrus · 2023-05-26T16:08:22Z

Looks like devstack doesn't support replicas. Here is the output of local tests:

$ go test -run TestReplica -v ./*.go
=== RUN   TestReplicaCreate
    tools.go:80: {
          "id": "125d64dd-aab9-4e24-8ccb-39dd455d1ba7",
          "availability_zone": "zone-1b",
          "cast_rules_to_readonly": false,
          "host": "",
          "share_id": "72a37fa6-20c9-43dc-a463-b802e96044d0",
          "share_network_id": "ca0163c8-3941-4420-8b01-41517e19e366",
          "share_server_id": "ed15ae66-af5d-4d72-8db2-a1f90a93a260",
          "status": "available",
          "replica_state": "out_of_sync"
        }
    replicas.go:54: Deleted replica: 125d64dd-aab9-4e24-8ccb-39dd455d1ba7
    shares.go:103: Deleted share: 72a37fa6-20c9-43dc-a463-b802e96044d0
--- PASS: TestReplicaCreate (66.88s)
=== RUN   TestReplicaPromote
    tools.go:80: {
          "id": "6a39a458-b2ee-468f-b1d9-261db50eec8c",
          "availability_zone": "zone-1b",
          "cast_rules_to_readonly": false,
          "host": "",
          "share_id": "fe52148b-3a24-4a19-84aa-08f1b2eeafd8",
          "share_network_id": "ca0163c8-3941-4420-8b01-41517e19e366",
          "share_server_id": "ed15ae66-af5d-4d72-8db2-a1f90a93a260",
          "status": "available",
          "replica_state": "out_of_sync"
        }
    replicas.go:54: Deleted replica: 6a39a458-b2ee-468f-b1d9-261db50eec8c
    shares.go:103: Deleted share: fe52148b-3a24-4a19-84aa-08f1b2eeafd8
--- PASS: TestReplicaPromote (126.09s)
=== RUN   TestReplicaExportLocations
    tools.go:80: []
    tools.go:80: [
          {
            "id": "c0cee115-28fc-43f0-bb42-5eba7f09d5dc",
            "path": "10.180.66.129:/share_5b891aba_b9a9_4582_8fb3_e14ab0dc2b6c",
            "share_instance_id": "",
            "is_admin_only": false,
            "preferred": true,
            "availability_zone": "zone-1b",
            "replica_state": "active"
          },
          {
            "id": "97bcdc70-ca37-45e9-b02b-3918b035c69b",
            "path": "10.180.66.22:/share_5b891aba_b9a9_4582_8fb3_e14ab0dc2b6c",
            "share_instance_id": "",
            "is_admin_only": false,
            "preferred": false,
            "availability_zone": "zone-1b",
            "replica_state": "active"
          }
        ]
    tools.go:80: {
          "id": "c0cee115-28fc-43f0-bb42-5eba7f09d5dc",
          "path": "10.180.66.129:/share_5b891aba_b9a9_4582_8fb3_e14ab0dc2b6c",
          "share_instance_id": "",
          "is_admin_only": false,
          "preferred": true,
          "availability_zone": "zone-1b",
          "replica_state": "active"
        }
    replicas.go:54: Deleted replica: 53661add-3fa1-49a2-b482-45c8c16a3e0a
    shares.go:103: Deleted share: 66c839d2-b083-4cb7-a4f8-eab71e43c908
--- PASS: TestReplicaExportLocations (54.36s)
=== RUN   TestReplicaListDetail
    tools.go:80: {
          "id": "65fe1244-ac6f-4e73-80e4-597a95ec6520",
          "availability_zone": "",
          "cast_rules_to_readonly": false,
          "host": "",
          "share_id": "e836fc36-62ef-4c82-b873-7c9fac061819",
          "share_network_id": "",
          "share_server_id": "",
          "status": "available",
          "replica_state": "out_of_sync"
        }
    tools.go:80: {
          "id": "f2e24517-87e5-4b35-b594-f69c2bb09ab4",
          "availability_zone": "",
          "cast_rules_to_readonly": false,
          "host": "",
          "share_id": "e836fc36-62ef-4c82-b873-7c9fac061819",
          "share_network_id": "",
          "share_server_id": "",
          "status": "available",
          "replica_state": "active"
        }
    replicas.go:54: Deleted replica: 65fe1244-ac6f-4e73-80e4-597a95ec6520
    shares.go:103: Deleted share: e836fc36-62ef-4c82-b873-7c9fac061819
--- PASS: TestReplicaListDetail (66.38s)
=== RUN   TestReplicaResetStatus
    replicas_test.go:278: Replica bf6d605c-3773-4178-8ae1-3b177540ffd0 status successfuly reset
    replicas.go:54: Deleted replica: bf6d605c-3773-4178-8ae1-3b177540ffd0
    shares.go:103: Deleted share: ef9fe330-4a4f-4703-ba97-a853d70a8314
--- PASS: TestReplicaResetStatus (80.83s)
=== RUN   TestReplicaForceDelete
    replicas_test.go:23: Skipping cloud admin tests.
--- SKIP: TestReplicaForceDelete (0.00s)
PASS
ok      command-line-arguments  394.539s

coveralls · 2023-05-26T16:15:47Z

coverage: 79.063% (+0.02%) from 79.039% when pulling 14d31d3 on kayrus:manila-replicas into 3fec48d on gophercloud:master.

kayrus · 2023-05-27T07:08:35Z

@mandre @pierreprinetti ready for review

kayrus · 2023-05-31T10:21:15Z

@mandre @pierreprinetti kindly ping

kayrus · 2023-06-07T23:41:35Z

any updates on this PR?

mandre · 2023-06-09T08:11:54Z

any updates on this PR?

About to take a look. Stay tuned :)

mandre · 2023-06-09T09:46:59Z

openstack/sharedfilesystems/v2/replicas/requests.go

+	// The availability zone of the share
+	AvailabilityZone string `json:"availability_zone,omitempty"`
+	// One or more scheduler_hints key and value pairs as a dictionary of strings
+	SchedulerHints map[string]string `json:"scheduler_hints,omitempty"`


Scheduler hints were added in the 2.67 microversion API (yoga). Perhaps add that as a comment so that users know not to use it unless they have a recent OpenStack.

mandre · 2023-06-09T11:36:07Z

openstack/sharedfilesystems/v2/replicas/results.go

+	// The availability zone of the share replica
+	AvailabilityZone string `json:"availability_zone"`
+	// Indicates whether existing access rules will be cast to read/only
+	CastRulesToReadonly bool `json:"cast_rules_to_readonly"`


Wouldn't it be better to have a separate type for brief and detailed results? As it is now, I can see it be quite confusing, especially for boolean fields, where it can returns false on a List() call, while returning true on a ListDetail() one.

This is a broader topic that we'll eventually need to tackle as part of the List() function consolidation (#2554). Please share your thoughts.

IMHO, adding an extra struct type would add even more confusion, especially when the results may be used within a single scope. BTW, the same bool issue can be faced with the is_admin_only field in the ExportLocation struct:

This parameter is only available to users with an “administrator” role, and cannot be controlled via policy .json.

Maybe it makes sense to use pointers to indicate whether the bool value is set?

Also, listing replica export locations doesn't return created/updated timestamps, while getting a single export location does. I don't think that we need to define an extra struct for this case as well.

mandre · 2023-06-09T11:43:37Z

acceptance/openstack/sharedfilesystems/v2/replicas_test.go

+// otherwise we need to set "X-OpenStack-Manila-API-Experimental: true"
+const replicasMicroversion = "2.60"
+
+func skipReplicaTests(t *testing.T) {


Please provide a comment explaining why these tests are skipped.

Have you tried changing the devstack config to support replication? This is how we configure it for our manila jobs.

Alternatively, perhaps we could skip the test when manila returns a 400 error with "Replication not supported for share", as in https://github.com/gophercloud/gophercloud/actions/runs/5097431904/jobs/9163951294#step:5:978 instead of disabling them altogether?

I suspect adding replication_type=readable to MANILA_DEFAULT_SHARE_TYPE_EXTRA_SPECS might do the trick.

https://github.com/gophercloud/gophercloud/blob/master/.github/workflows/functional-sharedfilesystems.yaml#LL54C13-L54C50

Unfortunately I haven't managed to enable the replication (which requires ZFS on Linux) support. I disabled it again.

I added an OS_MANILA_REPLICAS env check to run manila replicas acceptance tests.

kayrus · 2023-06-14T07:54:23Z

@mandre is there anything else I can do?

mandre · 2023-06-14T08:48:14Z

@mandre is there anything else I can do?

No, it's waiting on me, sorry. Just haven't had the chance to get back to reviewing your PR yet.

~~In your latest change, what sets OS_MANILA_REPLICAS?~~

edit: forget my question, I now understand we need to set OS_MANILA_REPLICAS variable if the env supports Manila replicas, which our job currently does not.

kayrus · 2023-06-14T09:34:16Z

@mandre we can try to setup the env with zfs that supports replicas later, but IMO it won't be easy. Locally I test with NetApp backend, it works great.

kayrus force-pushed the manila-replicas branch 2 times, most recently from 3e9e672 to 2dbc38a Compare May 27, 2023 06:22

kayrus mentioned this pull request Jun 2, 2023

[manila]: implement share and snapshot instances API #2638

Open

mandre reviewed Jun 9, 2023

View reviewed changes

kayrus force-pushed the manila-replicas branch 4 times, most recently from 8260be7 to e18cf83 Compare June 12, 2023 15:18

kayrus force-pushed the manila-replicas branch from e18cf83 to ecb7daa Compare June 14, 2023 08:30

[manila]: Add Share Replicas support

14d31d3

kayrus force-pushed the manila-replicas branch from ecb7daa to 14d31d3 Compare June 14, 2023 08:32

mandre added the semver:minor label Jun 14, 2023

mandre added this to the v1.5.0 milestone Jun 14, 2023

mandre approved these changes Jun 14, 2023

View reviewed changes

mandre merged commit fa800fc into gophercloud:master Jun 14, 2023

kayrus deleted the manila-replicas branch June 14, 2023 09:33

kayrus mentioned this pull request Jun 15, 2023

[manila-csi-plugin]: add support for replicas + share migration between AZs kubernetes/cloud-provider-openstack#2276

Closed

Conversation

kayrus commented May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kayrus commented May 27, 2023

Uh oh!

kayrus commented May 31, 2023

Uh oh!

kayrus commented Jun 7, 2023

Uh oh!

mandre commented Jun 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kayrus commented Jun 14, 2023

Uh oh!

mandre commented Jun 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kayrus commented Jun 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kayrus commented May 26, 2023 •

edited

Loading

coveralls commented May 26, 2023 •

edited

Loading

mandre commented Jun 14, 2023 •

edited

Loading