Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fileshare dock discovery code #735

Merged
merged 2 commits into from
May 20, 2019
Merged

Fileshare dock discovery code #735

merged 2 commits into from
May 20, 2019

Conversation

Shruthi-1MN
Copy link
Contributor

What this PR does / why we need it:
Dock was discovering always volume driver, to discover fileshare driver this piece of code is required.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

@Shruthi-1MN
Copy link
Contributor Author

@wisererik @xing-yang @leonwanghui @skdwriting

@coveralls
Copy link

coveralls commented May 13, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.03%) to 28.371% when pulling 31c8335 on Shruthi-1MN:fileshare-dockcodefix into 4c9bbbf on opensds:development.

wisererik
wisererik requested changes May 13, 2019
View reviewed changes
pkg/dock/discovery/discovery.go Show resolved Hide resolved
pkg/dock/discovery/discovery.go Outdated Show resolved Hide resolved
@leonwanghui leonwanghui added the feature there is a huge framework change or feature addition label May 14, 2019
leonwanghui
leonwanghui requested changes May 14, 2019
View reviewed changes
Copy link
Collaborator

@leonwanghui leonwanghui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use goimports to format the code style, you can try make goimports to do that.

@Shruthi-1MN Shruthi-1MN changed the title Fileshare dock discovery code [WIP]Fileshare dock discovery code May 14, 2019
@Shruthi-1MN Shruthi-1MN force-pushed the fileshare-dockcodefix branch 2 times, most recently from d4d842b to d341c11 Compare May 15, 2019 16:34
@Shruthi-1MN Shruthi-1MN changed the title [WIP]Fileshare dock discovery code Fileshare dock discovery code May 16, 2019
wisererik
wisererik requested changes May 16, 2019
View reviewed changes
pkg/controller/selector/selector.go Show resolved Hide resolved
pkg/controller/controller.go Outdated Show resolved Hide resolved
pkg/db/db.go Show resolved Hide resolved
pkg/utils/utils.go Outdated Show resolved Hide resolved
testutils/collection/data.go Outdated Show resolved Hide resolved
@Shruthi-1MN Shruthi-1MN force-pushed the fileshare-dockcodefix branch 9 times, most recently from fc57655 to 8860af5 Compare May 16, 2019 15:17
leonwanghui
leonwanghui requested changes May 17, 2019
View reviewed changes
pkg/model/proto/model.proto Outdated Show resolved Hide resolved
@Shruthi-1MN Shruthi-1MN force-pushed the fileshare-dockcodefix branch 6 times, most recently from ae813c8 to 1d50786 Compare May 17, 2019 13:33
@Shruthi-1MN Shruthi-1MN force-pushed the fileshare-dockcodefix branch 6 times, most recently from 02b4535 to c5e12ed Compare May 18, 2019 04:57
leonwanghui
leonwanghui requested changes May 19, 2019
View reviewed changes
Copy link
Collaborator

@leonwanghui leonwanghui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use make goimports to format your code style.

wisererik
wisererik requested changes May 20, 2019
View reviewed changes
Copy link
Collaborator

@wisererik wisererik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please format your code

pkg/controller/controller.go Outdated Show resolved Hide resolved
@Shruthi-1MN Shruthi-1MN force-pushed the fileshare-dockcodefix branch 2 times, most recently from dd7ee70 to 2001fb4 Compare May 20, 2019 04:47
wisererik
wisererik approved these changes May 20, 2019
View reviewed changes
Copy link
Collaborator

@wisererik wisererik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

leonwanghui
leonwanghui requested changes May 20, 2019
View reviewed changes
pkg/model/proto/model.proto Outdated
@@ -735,6 +735,7 @@ message CreateFileShareOpts {
message DeleteFileShareOpts {
// The uuid of the fileshare, required.
string id = 1;
string profileId = 2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I said, there is no need to add profileId here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry forgot to do the same for delete, now i did

leonwanghui
leonwanghui requested changes May 20, 2019
View reviewed changes
Copy link
Collaborator

@leonwanghui leonwanghui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the error in Line 922 that result.ProfileId should not equal with opt.GetProfile().

xxwjj
xxwjj reviewed May 20, 2019
View reviewed changes
pkg/controller/controller.go Outdated
@@ -154,6 +153,8 @@ func (c *Controller) CreateVolume(contx context.Context, opt *pb.CreateVolumeOpt
opt.PoolId = polInfo.Id
opt.PoolName = polInfo.Name

log.V(8).Infof("select pool %v and poolinfo : %v for volume %+v", opt.PoolId, opt.PoolName, vol)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For debuging log.V(5) is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@Shruthi-1MN Shruthi-1MN force-pushed the fileshare-dockcodefix branch 5 times, most recently from 1c541a1 to 92735a6 Compare May 20, 2019 07:25
@Shruthi-1MN
Copy link
Contributor Author

Please fix the error in Line 922 that result.ProfileId should not equal with opt.GetProfile().

Did the changes as per our discussion and tested.

leonwanghui
leonwanghui requested changes May 20, 2019
View reviewed changes
pkg/model/proto/model.pb.go Outdated Show resolved Hide resolved
@Shruthi-1MN Shruthi-1MN force-pushed the fileshare-dockcodefix branch 2 times, most recently from 4affe24 to 8d8448d Compare May 20, 2019 09:50
leonwanghui
leonwanghui approved these changes May 20, 2019
View reviewed changes
Copy link
Collaborator

@leonwanghui leonwanghui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@leonwanghui
Copy link
Collaborator

Please update the code

@wisererik wisererik merged commit 7c9ba74 into sodafoundation:development May 20, 2019
@Shruthi-1MN Shruthi-1MN deleted the fileshare-dockcodefix branch July 8, 2019 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xxwjj xxwjj xxwjj left review comments

@wisererik wisererik wisererik approved these changes

@leonwanghui leonwanghui leonwanghui approved these changes

Assignees

@Shruthi-1MN Shruthi-1MN

Labels
feature there is a huge framework change or feature addition
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Shruthi-1MN @coveralls @leonwanghui @xxwjj @wisererik