Skip to content

Commit

Permalink
Exit when invalid cmd options are set
Browse files Browse the repository at this point in the history
Exit with an error when both --volume-attach-limit and
--reserved-volume-attachments are specified on the command line.
  • Loading branch information
jsafrane committed Feb 9, 2024
1 parent de76efb commit 35e6ece
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 0 deletions.
8 changes: 8 additions & 0 deletions cmd/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ func GetOptions(fs *flag.FlagSet) *Options {
klog.ErrorS(err, "failed to validate and apply logging configuration")
}

if mode != driver.ControllerMode {
// nodeOptions must have been populated from the cmdline, validate them.
if err := nodeOptions.Validate(); err != nil {
klog.Error(err.Error())
klog.FlushAndExit(klog.ExitFlushTimeout, 1)
}
}

if *version {
versionInfo, err := driver.GetVersionJSON()
if err != nil {
Expand Down
9 changes: 9 additions & 0 deletions cmd/options/node_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package options

import (
"fmt"

flag "github.com/spf13/pflag"
)

Expand Down Expand Up @@ -44,3 +46,10 @@ func (o *NodeOptions) AddFlags(fs *flag.FlagSet) {
fs.Int64Var(&o.VolumeAttachLimit, "volume-attach-limit", -1, "Value for the maximum number of volumes attachable per node. If specified, the limit applies to all nodes and overrides --reserved-volume-attachments. If not specified, the value is approximated from the instance type.")
fs.IntVar(&o.ReservedVolumeAttachments, "reserved-volume-attachments", -1, "Number of volume attachments reserved for system use. Not used when --volume-attach-limit is specified. The total amount of volume attachments for a node is computed as: <nr. of attachments for corresponding instance type> - <number of NICs, if relevant to the instance type> - <reserved-volume-attachments value>. When -1, the amount of reserved attachments is loaded from instance metadata that captured state at node boot and may include not only system disks but also CSI volumes.")
}

func (o *NodeOptions) Validate() error {
if o.VolumeAttachLimit != -1 && o.ReservedVolumeAttachments != -1 {
return fmt.Errorf("only one of --volume-attach-limit and --reserved-volume-attachments may be specified")
}
return nil
}
53 changes: 53 additions & 0 deletions cmd/options/node_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,56 @@ func TestNodeOptions(t *testing.T) {
})
}
}

func TestValidate(t *testing.T) {
testCases := []struct {
name string
options *NodeOptions
expectError bool
}{
{
name: "valid VolumeAttachLimit",
options: &NodeOptions{
VolumeAttachLimit: 42,
ReservedVolumeAttachments: -1,
},
expectError: false,
},
{
name: "valid ReservedVolumeAttachments",
options: &NodeOptions{
VolumeAttachLimit: -1,
ReservedVolumeAttachments: 42,
},
expectError: false,
},
{
name: "default options",
options: &NodeOptions{
VolumeAttachLimit: -1,
ReservedVolumeAttachments: -1,
},
expectError: false,
},
{
name: "both options set",
options: &NodeOptions{
VolumeAttachLimit: 1,
ReservedVolumeAttachments: 1,
},
expectError: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := tc.options.Validate()
if tc.expectError && err == nil {
t.Errorf("Expected error, got nil")
}
if !tc.expectError && err != nil {
t.Errorf("Unexpected error: %v", err)
}
})
}
}
34 changes: 34 additions & 0 deletions cmd/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"testing"

flag "github.com/spf13/pflag"
"k8s.io/klog/v2"

"github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver"
)
Expand Down Expand Up @@ -221,6 +222,39 @@ func TestGetOptions(t *testing.T) {
}
},
},
{
name: "both volume-attach-limit and reserved-volume-attachments specified",
testFunc: func(t *testing.T) {
oldOSExit := klog.OsExit
defer func() { klog.OsExit = oldOSExit }()

var exitCode int
calledExit := false
testExit := func(code int) {
exitCode = code
calledExit = true
}
klog.OsExit = testExit

oldArgs := os.Args
defer func() { os.Args = oldArgs }()
os.Args = []string{
"aws-ebs-csi-driver",
"--volume-attach-limit=10",
"--reserved-volume-attachments=10",
}

flagSet := flag.NewFlagSet("test-flagset", flag.ContinueOnError)
_ = GetOptions(flagSet)

if exitCode != 1 {
t.Fatalf("expected exit code 1 but got %d", exitCode)
}
if !calledExit {
t.Fatalf("expect osExit to be called, but wasn't")
}
},
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 35e6ece

Please sign in to comment.