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

UPSTREAM: 47281: Update devicepath with filepath.Glob result #14565

Merged
merged 2 commits into from
Jun 17, 2017

Conversation

mtanino
Copy link
Contributor

@mtanino mtanino commented Jun 10, 2017

If iscsiTransport is not tcp, iSCSI plugin tries to
find devicepath using filepath.Glob but never updates
devicepath with the filepath.Glob result.

This patch fixes the problem.

If iscsiTransport is not tcp, iSCSI plugin tries to
find devicepath using filepath.Glob but never updates
devicepath with the filepath.Glob result.

This patch fixes the problem.
@mtanino
Copy link
Contributor Author

mtanino commented Jun 10, 2017

@rootfs
Copy link
Member

rootfs commented Jun 12, 2017

lgtm
[test]

@mtanino
Copy link
Contributor Author

mtanino commented Jun 13, 2017

[test]

@mtanino
Copy link
Contributor Author

mtanino commented Jun 13, 2017

[test]
Network related test failed. not related to this fix.

[ERROR] PID 23302: test/extended/networking-minimal.sh:6: `NETWORKING_E2E_MINIMAL=1 "${OS_ROOT}/test/extended/networking.sh"` exited with status 1.

@mtanino
Copy link
Contributor Author

mtanino commented Jun 13, 2017

@rootfs
Could you kick test again? Seems I can't do it.

@mtanino
Copy link
Contributor Author

mtanino commented Jun 15, 2017

/cc @rootfs

@mtanino
Copy link
Contributor Author

mtanino commented Jun 15, 2017

ping,
@childsb @eparis

@deads2k
Copy link
Contributor

deads2k commented Jun 15, 2017

[test]

@rootfs

@rootfs
Copy link
Member

rootfs commented Jun 15, 2017

[merge]

@rootfs
Copy link
Member

rootfs commented Jun 15, 2017

@childsb can you merge?

@eparis
Copy link
Member

eparis commented Jun 15, 2017

[merge]

if err == nil {
return true
}
if err != nil && !os.IsNotExist(err) {
Copy link
Member

Choose a reason for hiding this comment

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

How did no one point out this err != nil is redundant? Don't have to fix it here, but we should fix this upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll push to remove that redundant "error != nil" check to upstream.

@smarterclayton
Copy link
Contributor

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to cf75600

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2292/) (Base Commit: d1e5be5) (PR Branch Commit: cf75600)

@smarterclayton
Copy link
Contributor

[severity:bug][merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to cf75600

@openshift-bot
Copy link
Contributor

openshift-bot commented Jun 17, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/1032/) (Base Commit: 7bee838) (PR Branch Commit: cf75600) (Extended Tests: bug) (Image: devenv-rhel7_6374)

@openshift-bot openshift-bot merged commit 640b50e into openshift:master Jun 17, 2017
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request Jun 29, 2017
Automatic merge from submit-queue (batch tested with PRs 47619, 47951, 46260, 48277)

iSCSi plugin: Remove redundant nil check

**What this PR does / why we need it**:

This patch is for cleanup of redundant nil check in iSCSI plugin.
This was mentioned at the code review on origin github thread.

openshift/origin#14565

**Which issue this PR fixes** 

**Special notes for your reviewer**:

**Release note**:

```
NONE
```
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.

6 participants