Skip to content

Commit

Permalink
Merge pull request kubernetes#59548 from linyouchong/lyc-201802082
Browse files Browse the repository at this point in the history 
Automatic merge from submit-queue (batch tested with PRs 59466, 58912, 59605, 59548). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

fix incorrect logic in canSupport

**What this PR does / why we need it**:
if `spec.PersistentVolume` is nil or `spec.Volume` is nil, func `canSupport` should return false.

**Release note**:
```
NONE
```
/release-note-none
/sig storage
  • Loading branch information
Kubernetes Submit Queue authored Feb 9, 2018
2 parents afcb0bf + e92f6eb commit aee2cff
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 15 deletions.
6 changes: 1 addition & 5 deletions pkg/volume/fc/fc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,7 @@ func (plugin *fcPlugin) GetVolumeName(spec *volume.Spec) (string, error) {
}

func (plugin *fcPlugin) CanSupport(spec *volume.Spec) bool {
if (spec.Volume != nil && spec.Volume.FC == nil) || (spec.PersistentVolume != nil && spec.PersistentVolume.Spec.FC == nil) {
return false
}

return true
return (spec.Volume != nil && spec.Volume.FC != nil) || (spec.PersistentVolume != nil && spec.PersistentVolume.Spec.FC != nil)
}

func (plugin *fcPlugin) RequiresRemount() bool {
Expand Down
15 changes: 15 additions & 0 deletions pkg/volume/fc/fc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,24 @@ func TestCanSupport(t *testing.T) {
if plug.GetPluginName() != "kubernetes.io/fc" {
t.Errorf("Wrong name: %s", plug.GetPluginName())
}
if plug.CanSupport(&volume.Spec{}) {
t.Errorf("Expected false")
}
if plug.CanSupport(&volume.Spec{Volume: &v1.Volume{VolumeSource: v1.VolumeSource{}}}) {
t.Errorf("Expected false")
}
if !plug.CanSupport(&volume.Spec{Volume: &v1.Volume{VolumeSource: v1.VolumeSource{FC: &v1.FCVolumeSource{}}}}) {
t.Errorf("Expected true")
}
if plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{}}}) {
t.Errorf("Expected false")
}
if plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{PersistentVolumeSource: v1.PersistentVolumeSource{}}}}) {
t.Errorf("Expected false")
}
if !plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{PersistentVolumeSource: v1.PersistentVolumeSource{FC: &v1.FCVolumeSource{}}}}}) {
t.Errorf("Expected true")
}
}

func TestGetAccessModes(t *testing.T) {
Expand Down
6 changes: 1 addition & 5 deletions pkg/volume/iscsi/iscsi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,7 @@ func (plugin *iscsiPlugin) GetVolumeName(spec *volume.Spec) (string, error) {
}

func (plugin *iscsiPlugin) CanSupport(spec *volume.Spec) bool {
if (spec.Volume != nil && spec.Volume.ISCSI == nil) || (spec.PersistentVolume != nil && spec.PersistentVolume.Spec.ISCSI == nil) {
return false
}

return true
return (spec.Volume != nil && spec.Volume.ISCSI != nil) || (spec.PersistentVolume != nil && spec.PersistentVolume.Spec.ISCSI != nil)
}

func (plugin *iscsiPlugin) RequiresRemount() bool {
Expand Down
15 changes: 15 additions & 0 deletions pkg/volume/iscsi/iscsi_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,21 @@ func TestCanSupport(t *testing.T) {
if plug.CanSupport(&volume.Spec{Volume: &v1.Volume{VolumeSource: v1.VolumeSource{}}}) {
t.Errorf("Expected false")
}
if plug.CanSupport(&volume.Spec{}) {
t.Errorf("Expected false")
}
if !plug.CanSupport(&volume.Spec{Volume: &v1.Volume{VolumeSource: v1.VolumeSource{ISCSI: &v1.ISCSIVolumeSource{}}}}) {
t.Errorf("Expected true")
}
if plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{}}}) {
t.Errorf("Expected false")
}
if plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{PersistentVolumeSource: v1.PersistentVolumeSource{}}}}) {
t.Errorf("Expected false")
}
if !plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{PersistentVolumeSource: v1.PersistentVolumeSource{ISCSI: &v1.ISCSIPersistentVolumeSource{}}}}}) {
t.Errorf("Expected true")
}
}

func TestGetAccessModes(t *testing.T) {
Expand Down
6 changes: 1 addition & 5 deletions pkg/volume/rbd/rbd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,7 @@ func (plugin *rbdPlugin) GetVolumeName(spec *volume.Spec) (string, error) {
}

func (plugin *rbdPlugin) CanSupport(spec *volume.Spec) bool {
if (spec.Volume != nil && spec.Volume.RBD == nil) || (spec.PersistentVolume != nil && spec.PersistentVolume.Spec.RBD == nil) {
return false
}

return true
return (spec.Volume != nil && spec.Volume.RBD != nil) || (spec.PersistentVolume != nil && spec.PersistentVolume.Spec.RBD != nil)
}

func (plugin *rbdPlugin) RequiresRemount() bool {
Expand Down
15 changes: 15 additions & 0 deletions pkg/volume/rbd/rbd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,24 @@ func TestCanSupport(t *testing.T) {
if plug.GetPluginName() != "kubernetes.io/rbd" {
t.Errorf("Wrong name: %s", plug.GetPluginName())
}
if plug.CanSupport(&volume.Spec{}) {
t.Errorf("Expected false")
}
if plug.CanSupport(&volume.Spec{Volume: &v1.Volume{VolumeSource: v1.VolumeSource{}}}) {
t.Errorf("Expected false")
}
if !plug.CanSupport(&volume.Spec{Volume: &v1.Volume{VolumeSource: v1.VolumeSource{RBD: &v1.RBDVolumeSource{}}}}) {
t.Errorf("Expected true")
}
if plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{}}}) {
t.Errorf("Expected false")
}
if plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{PersistentVolumeSource: v1.PersistentVolumeSource{}}}}) {
t.Errorf("Expected false")
}
if !plug.CanSupport(&volume.Spec{PersistentVolume: &v1.PersistentVolume{Spec: v1.PersistentVolumeSpec{PersistentVolumeSource: v1.PersistentVolumeSource{RBD: &v1.RBDPersistentVolumeSource{}}}}}) {
t.Errorf("Expected true")
}
}

type fakeDiskManager struct {
Expand Down

0 comments on commit aee2cff

Please sign in to comment.