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.

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

Factor mount utility code out gce_pd volume plugin #5036

Merged
merged 1 commit into from Mar 5, 2015

Conversation

pmorie
Copy link
Member

@pmorie pmorie commented Mar 4, 2015

Currently the gce_pd volume plugin contains a significant amount of utility code that should be consolidated under pkg/util/mount.

@thockin @markturansky

@googlebot
Copy link

Thanks for your pull request.

It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits.

Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name.

@pmorie
Copy link
Member Author

pmorie commented Mar 4, 2015

@calfonso @rootfs I believe this is relevant to your interests

@pmorie
Copy link
Member Author

pmorie commented Mar 4, 2015

#4612 #4601

@@ -47,7 +47,8 @@ var _ = Describe("PD", func() {

nodes, err := c.Nodes().List()
expectNoError(err, "Failed to list nodes for e2e cluster.")
Expect(len(nodes.Items) >= 2)
fmt.Printf("Nodes: %v", len(nodes.Items))
Copy link
Contributor

Choose a reason for hiding this comment

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

Debug statement?

@pmorie
Copy link
Member Author

pmorie commented Mar 4, 2015

@thockin I'm ready for review on this once you have some bandwidth.

@@ -49,3 +49,30 @@ type MountPoint struct {
Freq int
Pass int
}

// Examines /proc/mounts to find all other references to the device referenced
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just create a mount_util.go?

Copy link
Member Author

Choose a reason for hiding this comment

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

@rootfs things are in the files they are to ensure that the build flags for
each function / type didn't change across this commit.
On Wed, Mar 4, 2015 at 7:58 PM Huamin Chen notifications@github.com wrote:

In pkg/util/mount/mount.go
#5036 (comment)
:

@@ -49,3 +49,30 @@ type MountPoint struct {
Freq int
Pass int
}
+
+// Examines /proc/mounts to find all other references to the device referenced

why not just create a mount_util.go?


Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/5036/files#r25830927
.

@smarterclayton
Copy link
Contributor

One comment, otherwise LGTM

@pmorie
Copy link
Member Author

pmorie commented Mar 5, 2015

@smarterclayton nit fixed, I had missed that when I copied the header in.

smarterclayton added a commit that referenced this pull request Mar 5, 2015
Factor mount utility code out gce_pd volume plugin
@smarterclayton smarterclayton merged commit ce24a7b into kubernetes:master Mar 5, 2015

// FakeMounter implements mount.Interface.
type FakeMounter struct {
mountPoints []MountPoint
Copy link
Member

Choose a reason for hiding this comment

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

How is one supposed to set the fake mount points if it is a private field?

@thockin
Copy link
Member

thockin commented Mar 6, 2015

just a couple nits - sorry for late review, send me a followup PR and I'll fast-track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants