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

Check presence of /dev/disk/by-id before reading it in ScaleIO #66174

Merged
merged 1 commit into from Jul 18, 2018

Conversation

@ddebroy
Copy link
Member

commented Jul 13, 2018

What this PR does / why we need it:
In certain OS environments, like RHEL 7.4 using Xen PV driver xen_blkfront for boot and data disks, the path /dev/disk/by-id does not exist in the OS post boot. When trying to dynamically provision PVs from a ScaleIO storageclass in such an environment, ScaleIO fails when trying to create the PV with:

E0711 08:02:50.441964   25246 sio_client.go:342] scaleio: failed to ReadDir /dev/disk/by-id: open /dev/disk/by-id: no such file or directory
E0711 08:02:50.441989   25246 sio_volume.go:123] scaleio: setup of volume k8svol-282bd4fa84d:  open /dev/disk/by-id: no such file or directory

This PR avoids the above failure by first checking for the presence of /dev/disk/by-id before attempting to read from it as done in other drivers like gce_pd.

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

Special notes for your reviewer:

Release note:

Allow ScaleIO volumes to be provisioned without having to first manually create /dev/disk/by-id path on each kubernetes node (if not already present)

/sig storage

@ddebroy

This comment has been minimized.

Copy link
Member Author

commented Jul 13, 2018

/assign @jsafrane

@ddebroy ddebroy changed the title Check presence of /dev/disk/by-id before reading it Check presence of /dev/disk/by-id before reading it in ScaleIO Jul 13, 2018

@ddebroy ddebroy force-pushed the ddebroy:scaleio1 branch 2 times, most recently from 1e21a73 to 8636d81 Jul 13, 2018

@abhi

This comment has been minimized.

Copy link
Member

commented Jul 13, 2018

/ok-to-test

@abhi
abhi approved these changes Jul 13, 2018
if !sioDiskIDPathExists {
return result, nil
}
// proceed to read sioDiskIDPath since we know it exists
files, err := ioutil.ReadDir(sioDiskIDPath)
if err != nil {

This comment has been minimized.

Copy link
@jsafrane

jsafrane Jul 16, 2018

Member

Can you just check ReadDir err here?

files, err := ioutil.ReadDir(sioDiskIDPath)
if err != nil {
    if !os.IsNotExists(err) {
        return []os.FileInfo{}, nil
    } else {
        return nil, err
}

This comment has been minimized.

Copy link
@ddebroy

ddebroy Jul 16, 2018

Author Member

Sure ... I have changed the PR to follow above.

@ddebroy

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2018

/test pull-kubernetes-verify

@jsafrane

This comment has been minimized.

Copy link
Member

commented Jul 17, 2018

lgmt-ish, please squash the commits

Check presence of sioDiskIDPath before reading it
Signed-off-by: Deep Debroy <ddebroy@docker.com>

@ddebroy ddebroy force-pushed the ddebroy:scaleio1 branch from 94cedbf to 7392ed4 Jul 17, 2018

@ddebroy

This comment has been minimized.

Copy link
Member Author

commented Jul 17, 2018

Commits squashed

@jsafrane

This comment has been minimized.

Copy link
Member

commented Jul 18, 2018

Thanks!

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 18, 2018

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ddebroy, jsafrane

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2018

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2018

Automatic merge from submit-queue (batch tested with PRs 64690, 66174). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-github-robot k8s-github-robot merged commit 78082a6 into kubernetes:master Jul 18, 2018

15 of 17 checks passed

Submit Queue Required Github CI test is not green: pull-kubernetes-kubemark-e2e-gce-big
Details
pull-kubernetes-kubemark-e2e-gce-big Job triggered.
Details
cla/linuxfoundation ddebroy authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
k8s-github-robot pushed a commit that referenced this pull request Jul 25, 2018
Kubernetes Submit Queue
Merge pull request #66347 from ddebroy/automated-cherry-pick-of-#6617…
…4-upstream-release-1.11

Automatic merge from submit-queue.

Automated cherry pick of #66174: Check presence of sioDiskIDPath before reading it

Cherry pick of #66174 on release-1.11.

#66174: Check presence of sioDiskIDPath before reading it

```release-note
Allow ScaleIO volumes to be provisioned without having to first manually create /dev/disk/by-id path on each kubernetes node (if not already present)
```
@ddebroy ddebroy referenced this pull request Aug 28, 2018
6 of 6 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.