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

Support specifying StorageClass while creating volumes #11451

Merged
merged 2 commits into from
Oct 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions contrib/completions/bash/oc
Original file line number Diff line number Diff line change
Expand Up @@ -13768,6 +13768,8 @@ _oc_set_volumes()
local_nonpersistent_flags+=("--add")
flags+=("--all")
local_nonpersistent_flags+=("--all")
flags+=("--claim-class=")
local_nonpersistent_flags+=("--claim-class=")
flags+=("--claim-mode=")
local_nonpersistent_flags+=("--claim-mode=")
flags+=("--claim-name=")
Expand Down Expand Up @@ -14181,6 +14183,8 @@ _oc_volumes()
local_nonpersistent_flags+=("--add")
flags+=("--all")
local_nonpersistent_flags+=("--all")
flags+=("--claim-class=")
local_nonpersistent_flags+=("--claim-class=")
flags+=("--claim-mode=")
local_nonpersistent_flags+=("--claim-mode=")
flags+=("--claim-name=")
Expand Down
4 changes: 4 additions & 0 deletions contrib/completions/bash/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -18443,6 +18443,8 @@ _openshift_cli_set_volumes()
local_nonpersistent_flags+=("--add")
flags+=("--all")
local_nonpersistent_flags+=("--all")
flags+=("--claim-class=")
local_nonpersistent_flags+=("--claim-class=")
flags+=("--claim-mode=")
local_nonpersistent_flags+=("--claim-mode=")
flags+=("--claim-name=")
Expand Down Expand Up @@ -18819,6 +18821,8 @@ _openshift_cli_volumes()
local_nonpersistent_flags+=("--add")
flags+=("--all")
local_nonpersistent_flags+=("--all")
flags+=("--claim-class=")
local_nonpersistent_flags+=("--claim-class=")
flags+=("--claim-mode=")
local_nonpersistent_flags+=("--claim-mode=")
flags+=("--claim-name=")
Expand Down
4 changes: 4 additions & 0 deletions contrib/completions/zsh/oc
Original file line number Diff line number Diff line change
Expand Up @@ -13929,6 +13929,8 @@ _oc_set_volumes()
local_nonpersistent_flags+=("--add")
flags+=("--all")
local_nonpersistent_flags+=("--all")
flags+=("--claim-class=")
local_nonpersistent_flags+=("--claim-class=")
flags+=("--claim-mode=")
local_nonpersistent_flags+=("--claim-mode=")
flags+=("--claim-name=")
Expand Down Expand Up @@ -14342,6 +14344,8 @@ _oc_volumes()
local_nonpersistent_flags+=("--add")
flags+=("--all")
local_nonpersistent_flags+=("--all")
flags+=("--claim-class=")
local_nonpersistent_flags+=("--claim-class=")
flags+=("--claim-mode=")
local_nonpersistent_flags+=("--claim-mode=")
flags+=("--claim-name=")
Expand Down
4 changes: 4 additions & 0 deletions contrib/completions/zsh/openshift
Original file line number Diff line number Diff line change
Expand Up @@ -18604,6 +18604,8 @@ _openshift_cli_set_volumes()
local_nonpersistent_flags+=("--add")
flags+=("--all")
local_nonpersistent_flags+=("--all")
flags+=("--claim-class=")
local_nonpersistent_flags+=("--claim-class=")
flags+=("--claim-mode=")
local_nonpersistent_flags+=("--claim-mode=")
flags+=("--claim-name=")
Expand Down Expand Up @@ -18980,6 +18982,8 @@ _openshift_cli_volumes()
local_nonpersistent_flags+=("--add")
flags+=("--all")
local_nonpersistent_flags+=("--all")
flags+=("--claim-class=")
local_nonpersistent_flags+=("--claim-class=")
flags+=("--claim-mode=")
local_nonpersistent_flags+=("--claim-mode=")
flags+=("--claim-name=")
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/oc-set-volumes.1
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ For descriptions on other volume types, see
\fB\-\-all\fP=false
select all resources in the namespace of the specified resource types

.PP
\fB\-\-claim\-class\fP=""
StorageClass to use for the persistent volume claim

.PP
\fB\-\-claim\-mode\fP="ReadWriteOnce"
Set the access mode of the claim to be created. Valid values are ReadWriteOnce (rwo), ReadWriteMany (rwm), or ReadOnlyMany (rom)
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/oc-volumes.1
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ DEPRECATED: This command has been moved to "oc set volume"
\fB\-\-all\fP=false
select all resources in the namespace of the specified resource types

.PP
\fB\-\-claim\-class\fP=""
StorageClass to use for the persistent volume claim

.PP
\fB\-\-claim\-mode\fP="ReadWriteOnce"
Set the access mode of the claim to be created. Valid values are ReadWriteOnce (rwo), ReadWriteMany (rwm), or ReadOnlyMany (rom)
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/openshift-cli-set-volumes.1
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ For descriptions on other volume types, see
\fB\-\-all\fP=false
select all resources in the namespace of the specified resource types

.PP
\fB\-\-claim\-class\fP=""
StorageClass to use for the persistent volume claim

.PP
\fB\-\-claim\-mode\fP="ReadWriteOnce"
Set the access mode of the claim to be created. Valid values are ReadWriteOnce (rwo), ReadWriteMany (rwm), or ReadOnlyMany (rom)
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/openshift-cli-volumes.1
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ DEPRECATED: This command has been moved to "openshift cli set volume"
\fB\-\-all\fP=false
select all resources in the namespace of the specified resource types

.PP
\fB\-\-claim\-class\fP=""
StorageClass to use for the persistent volume claim

.PP
\fB\-\-claim\-mode\fP="ReadWriteOnce"
Set the access mode of the claim to be created. Valid values are ReadWriteOnce (rwo), ReadWriteMany (rwm), or ReadOnlyMany (rom)
Expand Down
24 changes: 22 additions & 2 deletions pkg/cmd/cli/cmd/set/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ import (
"github.com/openshift/origin/pkg/cmd/util/clientcmd"
)

const volumePrefix = "volume-"
const (
volumePrefix = "volume-"
storageAnnClass = "volume.beta.kubernetes.io/storage-class"
)

var (
volumeLong = templates.LongDesc(`
Expand Down Expand Up @@ -136,6 +139,7 @@ type AddVolumeOptions struct {
ClaimName string
ClaimSize string
ClaimMode string
ClaimClass string

TypeChanged bool
}
Expand Down Expand Up @@ -186,6 +190,7 @@ func NewCmdVolume(fullName string, f *clientcmd.Factory, out, errOut io.Writer)
cmd.Flags().StringVar(&addOpts.ConfigMapName, "configmap-name", "", "Name of the persisted config map. Must be provided for configmap volume type")
cmd.Flags().StringVar(&addOpts.SecretName, "secret-name", "", "Name of the persisted secret. Must be provided for secret volume type")
cmd.Flags().StringVar(&addOpts.ClaimName, "claim-name", "", "Persistent volume claim name. Must be provided for persistentVolumeClaim volume type")
cmd.Flags().StringVar(&addOpts.ClaimClass, "claim-class", "", "StorageClass to use for the persistent volume claim")
cmd.Flags().StringVar(&addOpts.ClaimSize, "claim-size", "", "If specified along with a persistent volume type, create a new claim with the given size in bytes. Accepts SI notation: 10, 10G, 10Gi")
cmd.Flags().StringVar(&addOpts.ClaimMode, "claim-mode", "ReadWriteOnce", "Set the access mode of the claim to be created. Valid values are ReadWriteOnce (rwo), ReadWriteMany (rwm), or ReadOnlyMany (rom)")
cmd.Flags().StringVar(&addOpts.Source, "source", "", "Details of volume source as json string. This can be used if the required volume type is not supported by --type option. (e.g.: '{\"gitRepo\": {\"repository\": <git-url>, \"revision\": <commit-hash>}}')")
Expand Down Expand Up @@ -314,6 +319,15 @@ func (a *AddVolumeOptions) Validate(isAddOp bool) error {
return err
}
}
if len(a.ClaimClass) > 0 {
selectedLowerType := strings.ToLower(a.Type)
if selectedLowerType != "persistentvolumeclaim" && selectedLowerType != "pvc" {
return errors.New("must provide --type as persistentVolumeClaim")
}
if len(a.ClaimSize) == 0 {
return errors.New("must provide --claim-size to create new pvc with claim-class")
}
}
} else if len(a.Source) > 0 || len(a.Path) > 0 || len(a.SecretName) > 0 || len(a.ConfigMapName) > 0 || len(a.ClaimName) > 0 || a.Overwrite {
return errors.New("--type|--path|--configmap-name|--secret-name|--claim-name|--source|--overwrite are only valid for --add operation")
}
Expand Down Expand Up @@ -563,7 +577,7 @@ func (v *VolumeOptions) printVolumes(infos []*resource.Info) []error {
}

func (v *AddVolumeOptions) createClaim() *kapi.PersistentVolumeClaim {
return &kapi.PersistentVolumeClaim{
pvc := &kapi.PersistentVolumeClaim{
ObjectMeta: kapi.ObjectMeta{
Name: v.ClaimName,
},
Expand All @@ -576,6 +590,12 @@ func (v *AddVolumeOptions) createClaim() *kapi.PersistentVolumeClaim {
},
},
}
if len(v.ClaimClass) > 0 {
pvc.Annotations = map[string]string{
storageAnnClass: v.ClaimClass,
}
}
return pvc
}

func (v *VolumeOptions) setVolumeSource(kv *kapi.Volume) error {
Expand Down
70 changes: 70 additions & 0 deletions pkg/cmd/cli/cmd/set/volume_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package set

import (
"errors"
"net/http"
"testing"

Expand Down Expand Up @@ -161,3 +162,72 @@ func TestAddVolume(t *testing.T) {
t.Error(patchError)
}
}

func TestCreateClaim(t *testing.T) {
addOpts := &AddVolumeOptions{
Type: "persistentVolumeClaim",
ClaimClass: "foobar",
ClaimName: "foo-vol",
ClaimSize: "5G",
MountPath: "/sandbox",
}

pvc := addOpts.createClaim()
if len(pvc.Annotations) == 0 {
t.Errorf("Expected storage class annotation")
}

if pvc.Annotations[storageAnnClass] != "foobar" {
t.Errorf("Expected storage annotated class to be %s", addOpts.ClaimClass)
}
}

func TestValidateAddOptions(t *testing.T) {
tests := []struct {
name string
addOpts *AddVolumeOptions
expectedError error
}{
{
"using existing pvc",
&AddVolumeOptions{Type: "persistentVolumeClaim"},
errors.New("must provide --claim-name or --claim-size (to create a new claim) for --type=pvc"),
},
{
"creating new pvc",
&AddVolumeOptions{Type: "persistentVolumeClaim", ClaimName: "sandbox-pvc", ClaimSize: "5G"},
nil,
},
{
"error creating pvc with storage class",
&AddVolumeOptions{Type: "persistentVolumeClaim", ClaimName: "sandbox-pvc", ClaimClass: "slow"},
errors.New("must provide --claim-size to create new pvc with claim-class"),
},
{
"creating pvc with storage class",
&AddVolumeOptions{Type: "persistentVolumeClaim", ClaimName: "sandbox-pvc", ClaimClass: "slow", ClaimSize: "5G"},
nil,
},
}

for _, testCase := range tests {
addOpts := testCase.addOpts
err := addOpts.Validate(true)
if testCase.expectedError == nil && err != nil {
t.Errorf("Expected nil error for %s got %s", testCase.name, err)
continue
}

if testCase.expectedError != nil {
if err == nil {
t.Errorf("Expected %s, got nil", testCase.expectedError)
continue
}

if testCase.expectedError.Error() != err.Error() {
t.Errorf("Expected %s, got %s", testCase.expectedError, err)
}
}

}
}