-
Notifications
You must be signed in to change notification settings - Fork 510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Manila: add List for share-access-rules API #2512
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR looks good apart from the failing test.
t.Fatalf("Unable to grant access to share %s: %v", share.ID, err) | ||
} | ||
|
||
accessRules, err := ShareAccessRuleList(t, client, shareAccessRight.ID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you pass share.ID
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, yes, copy-paste mishap. Thanks!
ab84de7
to
243666f
Compare
I think the acceptance test needs to wait until the access rule is ready. |
You can make use of the WaitFor() helper function. See for example how the compute acceptance tests use it. |
243666f
to
f5f5a59
Compare
e07c5e6
to
db07949
Compare
// AccessRightToShareAccess is a helper function that converts | ||
// shares.AccessRight into shareaccessrules.ShareAccess struct. | ||
func AccessRightToShareAccess(accessRight *shares.AccessRight) *shareaccessrules.ShareAccess { | ||
return &shareaccessrules.ShareAccess{ | ||
ShareID: accessRight.ShareID, | ||
AccessType: accessRight.AccessType, | ||
AccessTo: accessRight.AccessTo, | ||
AccessKey: accessRight.AccessKey, | ||
AccessLevel: accessRight.AccessLevel, | ||
State: accessRight.State, | ||
ID: accessRight.ID, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit confusing -- we have shares.AccessRight
and shareaccessrules.ShareAccess
. shareaccessrules
describes a new API, so technically it ought to be its own package and struct, but in the end it describes the same information as shares.AccessRight
does. I'm just pointing this out, but if reviewers are ok with this split, we can move on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me. We can always split that into a separate package later if that bothers us.
db07949
to
3d1e981
Compare
https://github.com/gophercloud/gophercloud/actions/runs/4182887756/jobs/7246548686 failed on OpenStack install:
PTAL @mandre |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
// AccessRightToShareAccess is a helper function that converts | ||
// shares.AccessRight into shareaccessrules.ShareAccess struct. | ||
func AccessRightToShareAccess(accessRight *shares.AccessRight) *shareaccessrules.ShareAccess { | ||
return &shareaccessrules.ShareAccess{ | ||
ShareID: accessRight.ShareID, | ||
AccessType: accessRight.AccessType, | ||
AccessTo: accessRight.AccessTo, | ||
AccessKey: accessRight.AccessKey, | ||
AccessLevel: accessRight.AccessLevel, | ||
State: accessRight.State, | ||
ID: accessRight.ID, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me. We can always split that into a separate package later if that bothers us.
This PR adds
List
forshare-access-rules
Manila API.Part of #2495
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
https://github.com/openstack/manila/blob/stable/yoga/manila/db/sqlalchemy/api.py#L2621-L2636
https://github.com/openstack/manila/blob/stable/yoga/manila/api/v2/share_accesses.py#L54-L77
https://github.com/openstack/manila/blob/stable/yoga/manila/api/views/share_accesses.py#L32-L35