diff --git a/Makefile b/Makefile index b85a2045f4..2d45031232 100644 --- a/Makefile +++ b/Makefile @@ -83,9 +83,12 @@ test/coverage: go tool cover -html=filtered_cover.out -o coverage.html rm cover.out filtered_cover.out -.PHONY: test-sanity -test-sanity: - go test -v -race ./tests/sanity/... +# TODO: Re-enable sanity tests +# sanity tests have been disabled with the removal of NewFakeDriver, which was previously created to instantiate a fake driver utilized for testing. +# to re-enable tests, implement sanity tests creating a new driver instance by injecting mocked dependencies. +#.PHONY: test-sanity +#test-sanity: +# go test -v -race ./tests/sanity/... .PHONY: tools tools: bin/aws bin/ct bin/eksctl bin/ginkgo bin/golangci-lint bin/helm bin/kops bin/kubetest2 bin/mockgen bin/shfmt diff --git a/cmd/main.go b/cmd/main.go index 5424c912eb..a25b14338d 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors. +Copyright 2024 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -18,31 +18,102 @@ package main import ( "context" + "fmt" + "os" + "strings" "time" - flag "github.com/spf13/pflag" - + "github.com/kubernetes-sigs/aws-ebs-csi-driver/cmd/hooks" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud/metadata" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/metrics" + flag "github.com/spf13/pflag" + "k8s.io/component-base/featuregate" logsapi "k8s.io/component-base/logs/api/v1" json "k8s.io/component-base/logs/json" - "k8s.io/klog/v2" ) +var ( + featureGate = featuregate.NewFeatureGate() +) + func main() { fs := flag.NewFlagSet("aws-ebs-csi-driver", flag.ExitOnError) - if err := logsapi.RegisterLogFormat(logsapi.JSONLogFormat, json.Factory{}, logsapi.LoggingBetaOptions); err != nil { klog.ErrorS(err, "failed to register JSON log format") } - options := GetOptions(fs) + var ( + version = fs.Bool("version", false, "Print the version and exit.") + toStderr = fs.Bool("logtostderr", false, "log to standard error instead of files. DEPRECATED: will be removed in a future release.") + args = os.Args[1:] + cmd = string(driver.AllMode) + options = driver.Options{} + ) + + c := logsapi.NewLoggingConfiguration() + err := logsapi.AddFeatureGates(featureGate) + if err != nil { + klog.ErrorS(err, "failed to add feature gates") + } + logsapi.AddFlags(c, fs) + + if len(os.Args) > 1 && !strings.HasPrefix(os.Args[1], "-") { + cmd = os.Args[1] + args = os.Args[2:] + } + + switch cmd { + case "pre-stop-hook": + clientset, clientErr := metadata.DefaultKubernetesAPIClient() + if clientErr != nil { + klog.ErrorS(err, "unable to communicate with k8s API") + } else { + err = hooks.PreStop(clientset) + if err != nil { + klog.ErrorS(err, "failed to execute PreStop lifecycle hook") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) + } + } + klog.FlushAndExit(klog.ExitFlushTimeout, 0) + case string(driver.ControllerMode), string(driver.NodeMode), string(driver.AllMode): + options.Mode = driver.Mode(cmd) + default: + klog.Errorf("Unknown driver mode %s: Expected %s, %s, %s, or pre-stop-hook", cmd, driver.ControllerMode, driver.NodeMode, driver.AllMode) + klog.FlushAndExit(klog.ExitFlushTimeout, 0) + } + + options.AddFlags(fs) + + if err = fs.Parse(args); err != nil { + panic(err) + } + + err = logsapi.ValidateAndApply(c, featureGate) + if err != nil { + klog.ErrorS(err, "failed to validate and apply logging configuration") + } + + if *version { + versionInfo, versionErr := driver.GetVersionJSON() + if versionErr != nil { + klog.ErrorS(err, "failed to get version") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) + } + fmt.Println(versionInfo) + os.Exit(0) + } + + if *toStderr { + klog.SetOutput(os.Stderr) + } // Start tracing as soon as possible - if options.ServerOptions.EnableOtelTracing { - exporter, err := driver.InitOtelTracing() - if err != nil { + if options.EnableOtelTracing { + exporter, exporterErr := driver.InitOtelTracing() + if exporterErr != nil { klog.ErrorS(err, "failed to initialize otel tracing") klog.FlushAndExit(klog.ExitFlushTimeout, 1) } @@ -50,32 +121,39 @@ func main() { defer func() { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - if err := exporter.Shutdown(ctx); err != nil { - klog.ErrorS(err, "could not shutdown otel exporter") + if shutdownErr := exporter.Shutdown(ctx); shutdownErr != nil { + klog.ErrorS(exporterErr, "could not shutdown otel exporter") } }() } - if options.ServerOptions.HttpEndpoint != "" { + if options.HttpEndpoint != "" { r := metrics.InitializeRecorder() - r.InitializeMetricsHandler(options.ServerOptions.HttpEndpoint, "/metrics") + r.InitializeMetricsHandler(options.HttpEndpoint, "/metrics") } - drv, err := driver.NewDriver( - driver.WithEndpoint(options.ServerOptions.Endpoint), - driver.WithExtraTags(options.ControllerOptions.ExtraTags), - driver.WithExtraVolumeTags(options.ControllerOptions.ExtraVolumeTags), - driver.WithMode(options.DriverMode), - driver.WithVolumeAttachLimit(options.NodeOptions.VolumeAttachLimit), - driver.WithReservedVolumeAttachments(options.NodeOptions.ReservedVolumeAttachments), - driver.WithKubernetesClusterID(options.ControllerOptions.KubernetesClusterID), - driver.WithAwsSdkDebugLog(options.ControllerOptions.AwsSdkDebugLog), - driver.WithWarnOnInvalidTag(options.ControllerOptions.WarnOnInvalidTag), - driver.WithUserAgentExtra(options.ControllerOptions.UserAgentExtra), - driver.WithOtelTracing(options.ServerOptions.EnableOtelTracing), - driver.WithBatching(options.ControllerOptions.Batching), - driver.WithModifyVolumeRequestHandlerTimeout(options.ControllerOptions.ModifyVolumeRequestHandlerTimeout), - ) + region := os.Getenv("AWS_REGION") + if region == "" { + klog.V(5).InfoS("[Debug] Retrieving region from metadata service") + cfg := metadata.MetadataServiceConfig{ + EC2MetadataClient: metadata.DefaultEC2MetadataClient, + K8sAPIClient: metadata.DefaultKubernetesAPIClient, + } + metadata, metadataErr := metadata.NewMetadataService(cfg, region) + if metadataErr != nil { + klog.ErrorS(metadataErr, "Could not determine region from any metadata service. The region can be manually supplied via the AWS_REGION environment variable.") + panic(err) + } + region = metadata.GetRegion() + } + + cloud, err := cloud.NewCloud(region, options.AwsSdkDebugLog, options.UserAgentExtra, options.Batching) + if err != nil { + klog.ErrorS(err, "failed to create cloud service") + klog.FlushAndExit(klog.ExitFlushTimeout, 1) + } + + drv, err := driver.NewDriver(cloud, &options) if err != nil { klog.ErrorS(err, "failed to create driver") klog.FlushAndExit(klog.ExitFlushTimeout, 1) diff --git a/cmd/options.go b/cmd/options.go deleted file mode 100644 index b05e01bb46..0000000000 --- a/cmd/options.go +++ /dev/null @@ -1,148 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "fmt" - "os" - "strings" - - flag "github.com/spf13/pflag" - - "github.com/kubernetes-sigs/aws-ebs-csi-driver/cmd/hooks" - "github.com/kubernetes-sigs/aws-ebs-csi-driver/cmd/options" - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver" - - "k8s.io/component-base/featuregate" - logsapi "k8s.io/component-base/logs/api/v1" - "k8s.io/klog/v2" -) - -// Options is the combined set of options for all operating modes. -type Options struct { - DriverMode driver.Mode - - *options.ServerOptions - *options.ControllerOptions - *options.NodeOptions -} - -// used for testing -var osExit = os.Exit - -var featureGate = featuregate.NewFeatureGate() - -// GetOptions parses the command line options and returns a struct that contains -// the parsed options. -func GetOptions(fs *flag.FlagSet) *Options { - var ( - version = fs.Bool("version", false, "Print the version and exit.") - toStderr = fs.Bool("logtostderr", false, "log to standard error instead of files. DEPRECATED: will be removed in a future release.") - - args = os.Args[1:] - cmd = string(driver.AllMode) - - serverOptions = options.ServerOptions{} - controllerOptions = options.ControllerOptions{} - nodeOptions = options.NodeOptions{} - ) - - serverOptions.AddFlags(fs) - - c := logsapi.NewLoggingConfiguration() - - err := logsapi.AddFeatureGates(featureGate) - if err != nil { - klog.ErrorS(err, "failed to add feature gates") - } - - logsapi.AddFlags(c, fs) - - if len(os.Args) > 1 && !strings.HasPrefix(os.Args[1], "-") { - cmd = os.Args[1] - args = os.Args[2:] - } - - switch cmd { - case "pre-stop-hook": - clientset, clientErr := cloud.DefaultKubernetesAPIClient() - if clientErr != nil { - klog.ErrorS(err, "unable to communicate with k8s API") - } else { - err = hooks.PreStop(clientset) - if err != nil { - klog.ErrorS(err, "failed to execute PreStop lifecycle hook") - klog.FlushAndExit(klog.ExitFlushTimeout, 1) - } - } - klog.FlushAndExit(klog.ExitFlushTimeout, 0) - - case string(driver.ControllerMode): - controllerOptions.AddFlags(fs) - - case string(driver.NodeMode): - nodeOptions.AddFlags(fs) - - case string(driver.AllMode): - controllerOptions.AddFlags(fs) - nodeOptions.AddFlags(fs) - - default: - klog.Errorf("Unknown driver mode %s: Expected %s, %s, %s, or pre-stop-hook", cmd, driver.ControllerMode, driver.NodeMode, driver.AllMode) - klog.FlushAndExit(klog.ExitFlushTimeout, 0) - } - - if err = fs.Parse(args); err != nil { - panic(err) - } - - err = logsapi.ValidateAndApply(c, featureGate) - if err != nil { - klog.ErrorS(err, "failed to validate and apply logging configuration") - } - - if cmd != string(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 { - klog.ErrorS(err, "failed to get version") - klog.FlushAndExit(klog.ExitFlushTimeout, 1) - } - fmt.Println(versionInfo) - osExit(0) - } - - if *toStderr { - klog.SetOutput(os.Stderr) - } - - return &Options{ - DriverMode: driver.Mode(cmd), - - ServerOptions: &serverOptions, - ControllerOptions: &controllerOptions, - NodeOptions: &nodeOptions, - } -} diff --git a/cmd/options/controller_options.go b/cmd/options/controller_options.go deleted file mode 100644 index 4599d4b5f5..0000000000 --- a/cmd/options/controller_options.go +++ /dev/null @@ -1,61 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package options - -import ( - "time" - - flag "github.com/spf13/pflag" - - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver" - cliflag "k8s.io/component-base/cli/flag" -) - -// ControllerOptions contains options and configuration settings for the controller service. -type ControllerOptions struct { - // ExtraTags is a map of tags that will be attached to each dynamically provisioned - // resource. - ExtraTags map[string]string - // ExtraVolumeTags is a map of tags that will be attached to each dynamically provisioned - // volume. - // DEPRECATED: Use ExtraTags instead. - ExtraVolumeTags map[string]string - // ID of the kubernetes cluster. - KubernetesClusterID string - // flag to enable sdk debug log - AwsSdkDebugLog bool - // flag to warn on invalid tag, instead of returning an error - WarnOnInvalidTag bool - // flag to set user agent - UserAgentExtra string - // flag to enable batching of API calls - Batching bool - // flag to set the timeout for volume modification requests to be coalesced into a single - // volume modification call to AWS. - ModifyVolumeRequestHandlerTimeout time.Duration -} - -func (s *ControllerOptions) AddFlags(fs *flag.FlagSet) { - fs.Var(cliflag.NewMapStringString(&s.ExtraTags), "extra-tags", "Extra tags to attach to each dynamically provisioned resource. It is a comma separated list of key value pairs like '=,='") - fs.Var(cliflag.NewMapStringString(&s.ExtraVolumeTags), "extra-volume-tags", "DEPRECATED: Please use --extra-tags instead. Extra volume tags to attach to each dynamically provisioned volume. It is a comma separated list of key value pairs like '=,='") - fs.StringVar(&s.KubernetesClusterID, "k8s-tag-cluster-id", "", "ID of the Kubernetes cluster used for tagging provisioned EBS volumes (optional).") - fs.BoolVar(&s.AwsSdkDebugLog, "aws-sdk-debug-log", false, "To enable the aws sdk debug log level (default to false).") - fs.BoolVar(&s.WarnOnInvalidTag, "warn-on-invalid-tag", false, "To warn on invalid tags, instead of returning an error") - fs.StringVar(&s.UserAgentExtra, "user-agent-extra", "", "Extra string appended to user agent.") - fs.BoolVar(&s.Batching, "batching", false, "To enable batching of API calls. This is especially helpful for improving performance in workloads that are sensitive to EC2 rate limits.") - fs.DurationVar(&s.ModifyVolumeRequestHandlerTimeout, "modify-volume-request-handler-timeout", driver.DefaultModifyVolumeRequestHandlerTimeout, "Timeout for the window in which volume modification calls must be received in order for them to coalesce into a single volume modification call to AWS. This must be lower than the csi-resizer and volumemodifier timeouts") -} diff --git a/cmd/options/controller_options_test.go b/cmd/options/controller_options_test.go deleted file mode 100644 index 02919db846..0000000000 --- a/cmd/options/controller_options_test.go +++ /dev/null @@ -1,82 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package options - -import ( - "testing" - - flag "github.com/spf13/pflag" -) - -func TestControllerOptions(t *testing.T) { - testCases := []struct { - name string - flag string - found bool - }{ - { - name: "lookup desired flag", - flag: "extra-volume-tags", - found: true, - }, - { - name: "lookup k8s-tag-cluster-id", - flag: "k8s-tag-cluster-id", - found: true, - }, - { - name: "lookup aws-sdk-debug-log", - flag: "aws-sdk-debug-log", - found: true, - }, - { - name: "lookup batching", - flag: "batching", - found: true, - }, - { - name: "lookup user-agent-extra", - flag: "user-agent-extra", - found: true, - }, - { - name: "lookup modify-volume-request-handler-timeout", - flag: "modify-volume-request-handler-timeout", - found: true, - }, - { - name: "fail for non-desired flag", - flag: "some-other-flag", - found: false, - }, - } - - for _, tc := range testCases { - flagSet := flag.NewFlagSet("test-flagset", flag.ContinueOnError) - controllerOptions := &ControllerOptions{} - - t.Run(tc.name, func(t *testing.T) { - controllerOptions.AddFlags(flagSet) - - flag := flagSet.Lookup(tc.flag) - found := flag != nil - if found != tc.found { - t.Fatalf("result not equal\ngot:\n%v\nexpected:\n%v", found, tc.found) - } - }) - } -} diff --git a/cmd/options/node_options.go b/cmd/options/node_options.go deleted file mode 100644 index a66a2a9ab9..0000000000 --- a/cmd/options/node_options.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package options - -import ( - "fmt" - - flag "github.com/spf13/pflag" -) - -// NodeOptions contains options and configuration settings for the node service. -type NodeOptions struct { - // VolumeAttachLimit specifies the value that shall be reported as "maximum number of attachable volumes" - // in CSINode objects. It is similar to https://kubernetes.io/docs/concepts/storage/storage-limits/#custom-limits - // which allowed administrators to specify custom volume limits by configuring the kube-scheduler. Also, each AWS - // machine type has different volume limits. By default, the EBS CSI driver parses the machine type name and then - // decides the volume limit. However, this is only a rough approximation and not good enough in most cases. - // Specifying the volume attach limit via command line is the alternative until a more sophisticated solution presents - // itself (dynamically discovering the maximum number of attachable volume per EC2 machine type, see also - // https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/347). - VolumeAttachLimit int64 - - // ReservedVolumeAttachments specifies number of volume attachments reserved for system use. - // Typically 1 for the root disk, but may be larger when more system disks are attached to nodes. - // This option is not used when --volume-attach-limit is specified. - // 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 (and therefore it may be wrong). - ReservedVolumeAttachments int -} - -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: - - . 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 -} diff --git a/cmd/options/node_options_test.go b/cmd/options/node_options_test.go deleted file mode 100644 index 44aa021836..0000000000 --- a/cmd/options/node_options_test.go +++ /dev/null @@ -1,110 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package options - -import ( - "testing" - - flag "github.com/spf13/pflag" -) - -func TestNodeOptions(t *testing.T) { - testCases := []struct { - name string - flag string - found bool - }{ - { - name: "lookup desired flag", - flag: "volume-attach-limit", - found: true, - }, - { - name: "fail for non-desired flag", - flag: "some-flag", - found: false, - }, - } - - for _, tc := range testCases { - flagSet := flag.NewFlagSet("test-flagset", flag.ContinueOnError) - nodeOptions := &NodeOptions{} - - t.Run(tc.name, func(t *testing.T) { - nodeOptions.AddFlags(flagSet) - - flag := flagSet.Lookup(tc.flag) - found := flag != nil - if found != tc.found { - t.Fatalf("result not equal\ngot:\n%v\nexpected:\n%v", found, tc.found) - } - }) - } -} - -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) - } - }) - } -} diff --git a/cmd/options/server_options.go b/cmd/options/server_options.go deleted file mode 100644 index 52d6d7b49e..0000000000 --- a/cmd/options/server_options.go +++ /dev/null @@ -1,39 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package options - -import ( - flag "github.com/spf13/pflag" - - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver" -) - -// ServerOptions contains options and configuration settings for the driver server. -type ServerOptions struct { - // Endpoint is the endpoint that the driver server should listen on. - Endpoint string - // HttpEndpoint is the endpoint that the HTTP server for metrics should listen on. - HttpEndpoint string - // EnableOtelTracing enables opentelemetry tracing. - EnableOtelTracing bool -} - -func (s *ServerOptions) AddFlags(fs *flag.FlagSet) { - fs.StringVar(&s.Endpoint, "endpoint", driver.DefaultCSIEndpoint, "Endpoint for the CSI driver server") - fs.StringVar(&s.HttpEndpoint, "http-endpoint", "", "The TCP network address where the HTTP server for metrics will listen (example: `:8080`). The default is empty string, which means the server is disabled.") - fs.BoolVar(&s.EnableOtelTracing, "enable-otel-tracing", false, "To enable opentelemetry tracing for the driver. The tracing is disabled by default. Configure the exporter endpoint with OTEL_EXPORTER_OTLP_ENDPOINT and other env variables, see https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration.") -} diff --git a/cmd/options/server_options_test.go b/cmd/options/server_options_test.go deleted file mode 100644 index ade4cd073c..0000000000 --- a/cmd/options/server_options_test.go +++ /dev/null @@ -1,57 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package options - -import ( - "testing" - - flag "github.com/spf13/pflag" -) - -func TestServerOptions(t *testing.T) { - testCases := []struct { - name string - flag string - found bool - }{ - { - name: "lookup desired flag", - flag: "endpoint", - found: true, - }, - { - name: "fail for non-desired flag", - flag: "some-other-flag", - found: false, - }, - } - - for _, tc := range testCases { - flagSet := flag.NewFlagSet("test-flagset", flag.ContinueOnError) - serverOptions := &ServerOptions{} - - t.Run(tc.name, func(t *testing.T) { - serverOptions.AddFlags(flagSet) - - flag := flagSet.Lookup(tc.flag) - found := flag != nil - if found != tc.found { - t.Fatalf("result not equal\ngot:\n%v\nexpected:\n%v", found, tc.found) - } - }) - } -} diff --git a/cmd/options_test.go b/cmd/options_test.go deleted file mode 100644 index 4ee1951dc8..0000000000 --- a/cmd/options_test.go +++ /dev/null @@ -1,273 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package main - -import ( - "os" - "reflect" - "strconv" - "testing" - - flag "github.com/spf13/pflag" - "k8s.io/klog/v2" - - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver" -) - -func TestGetOptions(t *testing.T) { - testFunc := func( - t *testing.T, - additionalArgs []string, - withServerOptions bool, - withControllerOptions bool, - withNodeOptions bool, - ) *Options { - flagSet := flag.NewFlagSet("test-flagset", flag.ContinueOnError) - - endpointFlagName := "endpoint" - endpoint := "foo" - - extraTagsFlagName := "extra-tags" - extraTagKey := "bar" - extraTagValue := "baz" - extraTags := map[string]string{ - extraTagKey: extraTagValue, - } - - awsSdkDebugFlagName := "aws-sdk-debug-log" - awsSdkDebugFlagValue := true - VolumeAttachLimitFlagName := "volume-attach-limit" - var VolumeAttachLimit int64 = 42 - reservedVolumeAttachmentsFlagName := "reserved-volume-attachments" - reservedVolumeAttachments := -1 - - userAgentExtraFlag := "user-agent-extra" - userAgentExtraFlagValue := "test" - otelTracingFlagName := "enable-otel-tracing" - otelTracingFlagValue := true - batchingFlagName := "batching" - batchingFlagValue := true - - args := append([]string{ - "aws-ebs-csi-driver", - }, additionalArgs...) - - if withServerOptions { - args = append(args, "--"+endpointFlagName+"="+endpoint) - args = append(args, "--"+otelTracingFlagName+"="+strconv.FormatBool(otelTracingFlagValue)) - } - if withControllerOptions { - args = append(args, "--"+extraTagsFlagName+"="+extraTagKey+"="+extraTagValue) - args = append(args, "--"+awsSdkDebugFlagName+"="+strconv.FormatBool(awsSdkDebugFlagValue)) - args = append(args, "--"+userAgentExtraFlag+"="+userAgentExtraFlagValue) - args = append(args, "--"+batchingFlagName+"="+strconv.FormatBool(batchingFlagValue)) - } - if withNodeOptions { - args = append(args, "--"+VolumeAttachLimitFlagName+"="+strconv.FormatInt(VolumeAttachLimit, 10)) - } - - oldArgs := os.Args - defer func() { os.Args = oldArgs }() - os.Args = args - options := GetOptions(flagSet) - - if withServerOptions { - endpointFlag := flagSet.Lookup(endpointFlagName) - if endpointFlag == nil { - t.Fatalf("expected %q flag to be added but it is not", endpointFlagName) - } - if options.ServerOptions.Endpoint != endpoint { - t.Fatalf("expected endpoint to be %q but it is %q", endpoint, options.ServerOptions.Endpoint) - } - otelTracingFlag := flagSet.Lookup(otelTracingFlagName) - if otelTracingFlag == nil { - t.Fatalf("expected %q flag to be added but it is not", otelTracingFlagName) - } - } - - if withControllerOptions { - extraTagsFlag := flagSet.Lookup(extraTagsFlagName) - if extraTagsFlag == nil { - t.Fatalf("expected %q flag to be added but it is not", extraTagsFlagName) - } - if !reflect.DeepEqual(options.ControllerOptions.ExtraTags, extraTags) { - t.Fatalf("expected extra tags to be %q but it is %q", extraTags, options.ControllerOptions.ExtraTags) - } - awsDebugLogFlag := flagSet.Lookup(awsSdkDebugFlagName) - if awsDebugLogFlag == nil { - t.Fatalf("expected %q flag to be added but it is not", awsSdkDebugFlagName) - } - if options.ControllerOptions.AwsSdkDebugLog != awsSdkDebugFlagValue { - t.Fatalf("expected sdk debug flag to be %v but it is %v", awsSdkDebugFlagValue, options.ControllerOptions.AwsSdkDebugLog) - } - if options.ControllerOptions.UserAgentExtra != userAgentExtraFlagValue { - t.Fatalf("expected user agent string to be %q but it is %q", userAgentExtraFlagValue, options.ControllerOptions.UserAgentExtra) - } - batchingFlag := flagSet.Lookup(batchingFlagName) - if batchingFlag == nil { - t.Fatalf("expected %q flag to be added but it is not", batchingFlagName) - } - if options.ControllerOptions.Batching != batchingFlagValue { - t.Fatalf("expected sdk debug flag to be %v but it is %v", batchingFlagValue, options.ControllerOptions.Batching) - } - } - - if withNodeOptions { - VolumeAttachLimitFlag := flagSet.Lookup(VolumeAttachLimitFlagName) - if VolumeAttachLimitFlag == nil { - t.Fatalf("expected %q flag to be added but it is not", VolumeAttachLimitFlagName) - } - if options.NodeOptions.VolumeAttachLimit != VolumeAttachLimit { - t.Fatalf("expected VolumeAttachLimit to be %d but it is %d", VolumeAttachLimit, options.NodeOptions.VolumeAttachLimit) - } - reservedVolumeAttachmentsFlag := flagSet.Lookup(reservedVolumeAttachmentsFlagName) - if reservedVolumeAttachmentsFlag == nil { - t.Fatalf("expected %q flag to be added but it is not", reservedVolumeAttachmentsFlagName) - } - if options.NodeOptions.ReservedVolumeAttachments != reservedVolumeAttachments { - t.Fatalf("expected reservedVolumeAttachmentsFlagName to be %d but it is %d", reservedVolumeAttachments, options.NodeOptions.ReservedVolumeAttachments) - } - } - - return options - } - - testCases := []struct { - name string - testFunc func(t *testing.T) - }{ - { - name: "no controller mode given - expect all mode", - testFunc: func(t *testing.T) { - options := testFunc(t, nil, true, true, true) - - if options.DriverMode != driver.AllMode { - t.Fatalf("expected driver mode to be %q but it is %q", driver.AllMode, options.DriverMode) - } - }, - }, - { - name: "no options at all - expect all mode", - testFunc: func(t *testing.T) { - options := testFunc(t, nil, false, false, false) - - if options.DriverMode != driver.AllMode { - t.Fatalf("expected driver mode to be %q but it is %q", driver.AllMode, options.DriverMode) - } - }, - }, - { - name: "all mode given - expect all mode", - testFunc: func(t *testing.T) { - options := testFunc(t, []string{"all"}, true, true, true) - - if options.DriverMode != driver.AllMode { - t.Fatalf("expected driver mode to be %q but it is %q", driver.AllMode, options.DriverMode) - } - }, - }, - { - name: "controller mode given - expect controller mode", - testFunc: func(t *testing.T) { - options := testFunc(t, []string{"controller"}, true, true, false) - - if options.DriverMode != driver.ControllerMode { - t.Fatalf("expected driver mode to be %q but it is %q", driver.ControllerMode, options.DriverMode) - } - }, - }, - { - name: "node mode given - expect node mode", - testFunc: func(t *testing.T) { - options := testFunc(t, []string{"node"}, true, false, true) - - if options.DriverMode != driver.NodeMode { - t.Fatalf("expected driver mode to be %q but it is %q", driver.NodeMode, options.DriverMode) - } - }, - }, - { - name: "version flag specified", - testFunc: func(t *testing.T) { - oldOSExit := osExit - defer func() { osExit = oldOSExit }() - - var exitCode int - calledExit := false - testExit := func(code int) { - exitCode = code - calledExit = true - } - osExit = testExit - - oldArgs := os.Args - defer func() { os.Args = oldArgs }() - os.Args = []string{ - "aws-ebs-csi-driver", - "--version", - } - - flagSet := flag.NewFlagSet("test-flagset", flag.ContinueOnError) - _ = GetOptions(flagSet) - - if exitCode != 0 { - t.Fatalf("expected exit code 0 but got %d", exitCode) - } - if !calledExit { - t.Fatalf("expect osExit to be called, but wasn'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 { - t.Run(tc.name, tc.testFunc) - } -} diff --git a/docs/options.md b/docs/options.md index fd1ef08063..4a7d5f2060 100644 --- a/docs/options.md +++ b/docs/options.md @@ -5,6 +5,7 @@ There are a couple of driver options that can be passed as arguments when starti | Option argument | value sample | default | Description | |-----------------------------|---------------------------------------------------|-----------------------------------------------------|---------------------| | endpoint | tcp://127.0.0.1:10000/ | unix:///var/lib/csi/sockets/pluginproxy/csi.sock | The socket on which the driver will listen for CSI RPCs| +| http-endpoint | :8080 | | The TCP network address where the HTTP server for metrics will listen (example: `:8080`). The default is empty string, which means the server is disabled.| | volume-attach-limit | 1,2,3 ... | -1 | Value for the maximum number of volumes attachable per node. If specified, the limit applies to all nodes. If not specified, the value is approximated from the instance type| | extra-tags | key1=value1,key2=value2 | | Tags attached to each dynamically provisioned resource| | k8s-tag-cluster-id | aws-cluster-id-1 | | ID of the Kubernetes cluster used for tagging provisioned EBS volumes| @@ -14,3 +15,5 @@ There are a couple of driver options that can be passed as arguments when starti | enable-otel-tracing | true | false | If set to true, the driver will enable opentelemetry tracing. Might need [additional env variables](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration) to export the traces to the right collector| | batching | true | true | If set to true, the driver will enable batching of API calls. This is especially helpful for improving performance in workloads that are sensitive to EC2 rate limits at the cost of a small increase to worst-case latency| | modify-volume-request-handler-timeout | 10s | 2s | Timeout for the window in which volume modification calls must be received in order for them to coalesce into a single volume modification call to AWS. If changing this, be aware that the ebs-csi-controller's csi-resizer and volumemodifier containers both have timeouts on the calls they make, if this value exceeds those timeouts it will cause them to always fail and fall into a retry loop, so adjust those values accordingly. +| warn-on-invalid-tag | true | false | To warn on invalid tags, instead of returning an error| +|reserved-volume-attachments | 2 | -1 | Number of volume attachments reserved for system use. Not used when --volume-attach-limit is specified. 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.| diff --git a/go.mod b/go.mod index 315d2470f5..88e79e19ff 100644 --- a/go.mod +++ b/go.mod @@ -93,15 +93,12 @@ require ( github.com/jmespath/go-jmespath v0.4.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect - github.com/kubernetes-csi/csi-test/v4 v4.4.0 github.com/mailru/easyjson v0.7.7 // indirect github.com/moby/spdystream v0.2.0 // indirect github.com/moby/sys/mountinfo v0.7.1 // indirect github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/nxadm/tail v1.4.8 // indirect - github.com/onsi/ginkgo v1.16.5 // indirect github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/selinux v1.11.0 // indirect github.com/pkg/errors v0.9.1 // indirect @@ -137,7 +134,6 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240318140521-94a12d6c2237 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/natefinch/lumberjack.v2 v2.2.1 // indirect - gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiextensions-apiserver v0.29.2 // indirect diff --git a/go.sum b/go.sum index 0b2dbda57b..4eb20e251d 100644 --- a/go.sum +++ b/go.sum @@ -691,7 +691,6 @@ github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWH github.com/cncf/xds/go v0.0.0-20220314180256-7f1daf1720fc/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cncf/xds/go v0.0.0-20230105202645-06c439db220b/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cncf/xds/go v0.0.0-20230310173818-32f1caf87195/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= -github.com/container-storage-interface/spec v1.6.0/go.mod h1:8K96oQNkJ7pFcC2R9Z1ynGGBB1I93kcS6PGg3SsOk8s= github.com/container-storage-interface/spec v1.9.0 h1:zKtX4STsq31Knz3gciCYCi1SXtO2HJDecIjDVboYavY= github.com/container-storage-interface/spec v1.9.0/go.mod h1:ZfDu+3ZRyeVqxZM0Ds19MVLkN2d1XJ5MAfi1L3VjlT0= github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr4= @@ -968,8 +967,6 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kubernetes-csi/csi-proxy/client v1.1.3 h1:FdGU7NtxGhQX2wTfnuscmThG920hq0OaVVpuJW9t2k0= github.com/kubernetes-csi/csi-proxy/client v1.1.3/go.mod h1:SfK4HVKQdMH5KrffivddAWgX5hl3P5KmnuOTBbDNboU= -github.com/kubernetes-csi/csi-test/v4 v4.4.0 h1:r0mnAwDURI24Vw3a/LyA/ga11yD5ZGuU7+REO35Na9s= -github.com/kubernetes-csi/csi-test/v4 v4.4.0/go.mod h1:t1RzseMZJKy313nezI/d7TolbbiKpUZM3SXQvXxOX0w= github.com/kubernetes-csi/external-snapshotter/client/v4 v4.2.0 h1:nHHjmvjitIiyPlUHk/ofpgvBcNcawJLtf4PYHORLjAA= github.com/kubernetes-csi/external-snapshotter/client/v4 v4.2.0/go.mod h1:YBCo4DoEeDndqvAn6eeu0vWM7QdXmHEeI9cFWplmBys= github.com/lyft/protoc-gen-star v0.6.0/go.mod h1:TGAoBVkt8w7MPG72TrKIu85MIdXwDuzJYeZuUPFPNwA= @@ -999,13 +996,10 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8m github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= -github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk= github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0= -github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= -github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c= github.com/onsi/ginkgo/v2 v2.1.4/go.mod h1:um6tUpWM/cxCK3/FK8BXqEiUMUwRgSM4JXG47RKZmLU= github.com/onsi/ginkgo/v2 v2.1.6/go.mod h1:MEH45j8TBi6u9BMogfbp0stKC5cdGjumZj5Y7AG4VIk= @@ -1734,7 +1728,6 @@ google.golang.org/genproto v0.0.0-20200825200019-8632dd797987/go.mod h1:FWY/as6D google.golang.org/genproto v0.0.0-20200904004341-0bd0a958aa1d/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20201109203340-2640f1f9cdfb/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20201201144952-b05cb90ed32e/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= -google.golang.org/genproto v0.0.0-20201209185603-f92720507ed4/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20201210142538-e3217bee35cc/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20201214200347-8c77b98c765d/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/genproto v0.0.0-20210108203827-ffc7fda8c3d7/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= @@ -1927,7 +1920,6 @@ gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc= gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw= gopkg.in/natefinch/lumberjack.v2 v2.2.1 h1:bBRl1b0OH9s/DuPhuXpNl+VtCaJXFZ5/uEFST95x9zc= gopkg.in/natefinch/lumberjack.v2 v2.2.1/go.mod h1:YD8tP3GAjkrDg1eZH7EGmyESg/lsYskCTPBJVb9jqSc= -gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.3/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= @@ -1972,7 +1964,6 @@ k8s.io/csi-translation-lib v0.29.2/go.mod h1:vbSYY4c6mVPwTHAvb5V3CHlq/dmQFIZC1SJ k8s.io/gengo v0.0.0-20230829151522-9cce18d56c01/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= -k8s.io/klog/v2 v2.60.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/klog/v2 v2.110.1/go.mod h1:YGtd1984u+GgbuZ7e08/yBuAfKLSO0+uR1Fhi6ExXjo= k8s.io/klog/v2 v2.120.1 h1:QXU6cPEOIslTGvZaXvFWiP9VKyeet3sawzTOvdXb4Vw= diff --git a/hack/update-mockgen.sh b/hack/update-mockgen.sh index 2a5e926e3e..0e35f5b205 100755 --- a/hack/update-mockgen.sh +++ b/hack/update-mockgen.sh @@ -19,8 +19,8 @@ set -euo pipefail BIN="$(dirname "$(realpath "${BASH_SOURCE[0]}")")/../bin" # Source-based mocking for internal interfaces -"${BIN}/mockgen" -package cloud -destination=./pkg/cloud/mock_cloud.go -source pkg/cloud/cloud_interface.go -"${BIN}/mockgen" -package cloud -destination=./pkg/cloud/mock_metadata.go -source pkg/cloud/metadata_interface.go +"${BIN}/mockgen" -package cloud -destination=./pkg/cloud/mock_cloud.go -source pkg/cloud/interface.go +"${BIN}/mockgen" -package metadata -destination=./pkg/cloud/metadata/mock_metadata.go -source pkg/cloud/metadata/interface.go "${BIN}/mockgen" -package driver -destination=./pkg/driver/mock_mount.go -source pkg/driver/mount.go "${BIN}/mockgen" -package mounter -destination=./pkg/mounter/mock_mount_windows.go -source pkg/mounter/safe_mounter_windows.go "${BIN}/mockgen" -package cloud -destination=./pkg/cloud/mock_ec2.go -source pkg/cloud/ec2_interface.go EC2API diff --git a/pkg/cloud/cloud_interface.go b/pkg/cloud/interface.go similarity index 100% rename from pkg/cloud/cloud_interface.go rename to pkg/cloud/interface.go diff --git a/pkg/cloud/metadata_ec2.go b/pkg/cloud/metadata/ec2.go similarity index 99% rename from pkg/cloud/metadata_ec2.go rename to pkg/cloud/metadata/ec2.go index 258cdd691d..168ca7fc9c 100644 --- a/pkg/cloud/metadata_ec2.go +++ b/pkg/cloud/metadata/ec2.go @@ -1,4 +1,4 @@ -package cloud +package metadata import ( "context" diff --git a/pkg/cloud/metadata_interface.go b/pkg/cloud/metadata/interface.go similarity index 98% rename from pkg/cloud/metadata_interface.go rename to pkg/cloud/metadata/interface.go index d1e3017821..7f8c2d4092 100644 --- a/pkg/cloud/metadata_interface.go +++ b/pkg/cloud/metadata/interface.go @@ -1,4 +1,4 @@ -package cloud +package metadata import ( "context" diff --git a/pkg/cloud/metadata_k8s.go b/pkg/cloud/metadata/k8s.go similarity index 99% rename from pkg/cloud/metadata_k8s.go rename to pkg/cloud/metadata/k8s.go index 1ba84a0990..c288f634ca 100644 --- a/pkg/cloud/metadata_k8s.go +++ b/pkg/cloud/metadata/k8s.go @@ -1,4 +1,4 @@ -package cloud +package metadata import ( "context" diff --git a/pkg/cloud/metadata.go b/pkg/cloud/metadata/metadata.go similarity index 99% rename from pkg/cloud/metadata.go rename to pkg/cloud/metadata/metadata.go index 64e0a8996d..2b81e9e445 100644 --- a/pkg/cloud/metadata.go +++ b/pkg/cloud/metadata/metadata.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cloud +package metadata import ( "fmt" diff --git a/pkg/cloud/metadata_test.go b/pkg/cloud/metadata/metadata_test.go similarity index 99% rename from pkg/cloud/metadata_test.go rename to pkg/cloud/metadata/metadata_test.go index c6bc8dcef3..31fbe7a013 100644 --- a/pkg/cloud/metadata_test.go +++ b/pkg/cloud/metadata/metadata_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cloud +package metadata import ( "errors" diff --git a/pkg/cloud/mock_metadata.go b/pkg/cloud/metadata/mock_metadata.go similarity index 98% rename from pkg/cloud/mock_metadata.go rename to pkg/cloud/metadata/mock_metadata.go index 4e79bcfa0f..d7ae84fd91 100644 --- a/pkg/cloud/mock_metadata.go +++ b/pkg/cloud/metadata/mock_metadata.go @@ -1,8 +1,8 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: pkg/cloud/metadata_interface.go +// Source: pkg/cloud/metadata/interface.go -// Package cloud is a generated GoMock package. -package cloud +// Package metadata is a generated GoMock package. +package metadata import ( context "context" diff --git a/pkg/cloud/mock_cloud.go b/pkg/cloud/mock_cloud.go index 8c8a28710d..e13521874d 100644 --- a/pkg/cloud/mock_cloud.go +++ b/pkg/cloud/mock_cloud.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: pkg/cloud/cloud_interface.go +// Source: pkg/cloud/interface.go // Package cloud is a generated GoMock package. package cloud diff --git a/pkg/driver/controller.go b/pkg/driver/controller.go index df028a00f6..71ecaa47eb 100644 --- a/pkg/driver/controller.go +++ b/pkg/driver/controller.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "os" "strconv" "strings" @@ -57,59 +56,26 @@ var ( const isManagedByDriver = "true" -// controllerService represents the controller service of CSI driver -type controllerService struct { +// ControllerService represents the controller service of CSI driver +type ControllerService struct { cloud cloud.Cloud inFlight *internal.InFlight - driverOptions *DriverOptions + options *Options modifyVolumeManager *modifyVolumeManager - rpc.UnimplementedModifyServer } -var ( - // NewMetadataFunc is a variable for the cloud.NewMetadata function that can - // be overwritten in unit tests. - NewMetadataFunc = cloud.NewMetadataService - // NewCloudFunc is a variable for the cloud.NewCloud function that can - // be overwritten in unit tests. - NewCloudFunc = cloud.NewCloud -) - -// newControllerService creates a new controller service -// it panics if failed to create the service -func newControllerService(driverOptions *DriverOptions) controllerService { - region := os.Getenv("AWS_REGION") - if region == "" { - klog.V(5).InfoS("[Debug] Retrieving region from metadata service") - - cfg := cloud.MetadataServiceConfig{ - EC2MetadataClient: cloud.DefaultEC2MetadataClient, - K8sAPIClient: cloud.DefaultKubernetesAPIClient, - } - metadata, err := NewMetadataFunc(cfg, region) - if err != nil { - klog.ErrorS(err, "Could not determine region from any metadata service. The region can be manually supplied via the AWS_REGION environment variable.") - panic(err) - } - region = metadata.GetRegion() - } - - klog.InfoS("batching", "status", driverOptions.batching) - cloudSrv, err := NewCloudFunc(region, driverOptions.awsSdkDebugLog, driverOptions.userAgentExtra, driverOptions.batching) - if err != nil { - panic(err) - } - - return controllerService{ - cloud: cloudSrv, +// NewControllerService creates a new controller service +func NewControllerService(c cloud.Cloud, o *Options) *ControllerService { + return &ControllerService{ + cloud: c, + options: o, inFlight: internal.NewInFlight(), - driverOptions: driverOptions, modifyVolumeManager: newModifyVolumeManager(), } } -func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) { +func (d *ControllerService) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) (*csi.CreateVolumeResponse, error) { klog.V(4).InfoS("CreateVolume: called", "args", *req) if err := validateCreateVolumeRequest(req); err != nil { return nil, err @@ -325,22 +291,22 @@ func (d *controllerService) CreateVolume(ctx context.Context, req *csi.CreateVol outpostArn := getOutpostArn(req.GetAccessibilityRequirements()) // fill volume tags - if d.driverOptions.kubernetesClusterID != "" { - resourceLifecycleTag := ResourceLifecycleTagPrefix + d.driverOptions.kubernetesClusterID + if d.options.KubernetesClusterID != "" { + resourceLifecycleTag := ResourceLifecycleTagPrefix + d.options.KubernetesClusterID volumeTags[resourceLifecycleTag] = ResourceLifecycleOwned - volumeTags[NameTag] = d.driverOptions.kubernetesClusterID + "-dynamic-" + volName - volumeTags[KubernetesClusterTag] = d.driverOptions.kubernetesClusterID + volumeTags[NameTag] = d.options.KubernetesClusterID + "-dynamic-" + volName + volumeTags[KubernetesClusterTag] = d.options.KubernetesClusterID } - for k, v := range d.driverOptions.extraTags { + for k, v := range d.options.ExtraTags { volumeTags[k] = v } - addTags, err := template.Evaluate(scTags, tProps, d.driverOptions.warnOnInvalidTag) + addTags, err := template.Evaluate(scTags, tProps, d.options.WarnOnInvalidTag) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "Error interpolating the tag value: %v", err) } - if err = validateExtraTags(addTags, d.driverOptions.warnOnInvalidTag); err != nil { + if err = validateExtraTags(addTags, d.options.WarnOnInvalidTag); err != nil { return nil, status.Errorf(codes.InvalidArgument, "Invalid tag value: %v", err) } @@ -398,7 +364,7 @@ func validateCreateVolumeRequest(req *csi.CreateVolumeRequest) error { return nil } -func (d *controllerService) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { +func (d *ControllerService) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { klog.V(4).InfoS("DeleteVolume: called", "args", *req) if err := validateDeleteVolumeRequest(req); err != nil { return nil, err @@ -430,7 +396,7 @@ func validateDeleteVolumeRequest(req *csi.DeleteVolumeRequest) error { return nil } -func (d *controllerService) ControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) { +func (d *ControllerService) ControllerPublishVolume(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (*csi.ControllerPublishVolumeResponse, error) { klog.V(4).InfoS("ControllerPublishVolume: called", "args", *req) if err := validateControllerPublishVolumeRequest(req); err != nil { return nil, err @@ -479,7 +445,7 @@ func validateControllerPublishVolumeRequest(req *csi.ControllerPublishVolumeRequ return nil } -func (d *controllerService) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) { +func (d *ControllerService) ControllerUnpublishVolume(ctx context.Context, req *csi.ControllerUnpublishVolumeRequest) (*csi.ControllerUnpublishVolumeResponse, error) { klog.V(4).InfoS("ControllerUnpublishVolume: called", "args", *req) if err := validateControllerUnpublishVolumeRequest(req); err != nil { @@ -519,7 +485,7 @@ func validateControllerUnpublishVolumeRequest(req *csi.ControllerUnpublishVolume return nil } -func (d *controllerService) ControllerGetCapabilities(ctx context.Context, req *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { +func (d *ControllerService) ControllerGetCapabilities(ctx context.Context, req *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) { klog.V(4).InfoS("ControllerGetCapabilities: called", "args", *req) var caps []*csi.ControllerServiceCapability for _, cap := range controllerCaps { @@ -535,17 +501,17 @@ func (d *controllerService) ControllerGetCapabilities(ctx context.Context, req * return &csi.ControllerGetCapabilitiesResponse{Capabilities: caps}, nil } -func (d *controllerService) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest) (*csi.GetCapacityResponse, error) { +func (d *ControllerService) GetCapacity(ctx context.Context, req *csi.GetCapacityRequest) (*csi.GetCapacityResponse, error) { klog.V(4).InfoS("GetCapacity: called", "args", *req) return nil, status.Error(codes.Unimplemented, "") } -func (d *controllerService) ListVolumes(ctx context.Context, req *csi.ListVolumesRequest) (*csi.ListVolumesResponse, error) { +func (d *ControllerService) ListVolumes(ctx context.Context, req *csi.ListVolumesRequest) (*csi.ListVolumesResponse, error) { klog.V(4).InfoS("ListVolumes: called", "args", *req) return nil, status.Error(codes.Unimplemented, "") } -func (d *controllerService) ValidateVolumeCapabilities(ctx context.Context, req *csi.ValidateVolumeCapabilitiesRequest) (*csi.ValidateVolumeCapabilitiesResponse, error) { +func (d *ControllerService) ValidateVolumeCapabilities(ctx context.Context, req *csi.ValidateVolumeCapabilitiesRequest) (*csi.ValidateVolumeCapabilitiesResponse, error) { klog.V(4).InfoS("ValidateVolumeCapabilities: called", "args", *req) volumeID := req.GetVolumeId() if len(volumeID) == 0 { @@ -573,7 +539,7 @@ func (d *controllerService) ValidateVolumeCapabilities(ctx context.Context, req }, nil } -func (d *controllerService) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) { +func (d *ControllerService) ControllerExpandVolume(ctx context.Context, req *csi.ControllerExpandVolumeRequest) (*csi.ControllerExpandVolumeResponse, error) { klog.V(4).InfoS("ControllerExpandVolume: called", "args", *req) volumeID := req.GetVolumeId() if len(volumeID) == 0 { @@ -626,7 +592,7 @@ func (d *controllerService) ControllerExpandVolume(ctx context.Context, req *csi }, nil } -func (d *controllerService) ControllerModifyVolume(ctx context.Context, req *csi.ControllerModifyVolumeRequest) (*csi.ControllerModifyVolumeResponse, error) { +func (d *ControllerService) ControllerModifyVolume(ctx context.Context, req *csi.ControllerModifyVolumeRequest) (*csi.ControllerModifyVolumeResponse, error) { klog.V(4).InfoS("ControllerModifyVolume: called", "args", *req) volumeID := req.GetVolumeId() @@ -647,7 +613,7 @@ func (d *controllerService) ControllerModifyVolume(ctx context.Context, req *csi return &csi.ControllerModifyVolumeResponse{}, nil } -func (d *controllerService) ControllerGetVolume(ctx context.Context, req *csi.ControllerGetVolumeRequest) (*csi.ControllerGetVolumeResponse, error) { +func (d *ControllerService) ControllerGetVolume(ctx context.Context, req *csi.ControllerGetVolumeRequest) (*csi.ControllerGetVolumeResponse, error) { klog.V(4).InfoS("ControllerGetVolume: called", "args", *req) return nil, status.Error(codes.Unimplemented, "") } @@ -705,7 +671,7 @@ func isValidVolumeContext(volContext map[string]string) bool { return true } -func (d *controllerService) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequest) (*csi.CreateSnapshotResponse, error) { +func (d *ControllerService) CreateSnapshot(ctx context.Context, req *csi.CreateSnapshotRequest) (*csi.CreateSnapshotResponse, error) { klog.V(4).InfoS("CreateSnapshot: called", "args", req) if err := validateCreateSnapshotRequest(req); err != nil { return nil, err @@ -762,21 +728,21 @@ func (d *controllerService) CreateSnapshot(ctx context.Context, req *csi.CreateS } } - addTags, err := template.Evaluate(vscTags, vsProps, d.driverOptions.warnOnInvalidTag) + addTags, err := template.Evaluate(vscTags, vsProps, d.options.WarnOnInvalidTag) if err != nil { return nil, status.Errorf(codes.InvalidArgument, "Error interpolating the tag value: %v", err) } - if err = validateExtraTags(addTags, d.driverOptions.warnOnInvalidTag); err != nil { + if err = validateExtraTags(addTags, d.options.WarnOnInvalidTag); err != nil { return nil, status.Errorf(codes.InvalidArgument, "Invalid tag value: %v", err) } - if d.driverOptions.kubernetesClusterID != "" { - resourceLifecycleTag := ResourceLifecycleTagPrefix + d.driverOptions.kubernetesClusterID + if d.options.KubernetesClusterID != "" { + resourceLifecycleTag := ResourceLifecycleTagPrefix + d.options.KubernetesClusterID snapshotTags[resourceLifecycleTag] = ResourceLifecycleOwned - snapshotTags[NameTag] = d.driverOptions.kubernetesClusterID + "-dynamic-" + snapshotName + snapshotTags[NameTag] = d.options.KubernetesClusterID + "-dynamic-" + snapshotName } - for k, v := range d.driverOptions.extraTags { + for k, v := range d.options.ExtraTags { snapshotTags[k] = v } @@ -834,7 +800,7 @@ func validateCreateSnapshotRequest(req *csi.CreateSnapshotRequest) error { return nil } -func (d *controllerService) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) { +func (d *ControllerService) DeleteSnapshot(ctx context.Context, req *csi.DeleteSnapshotRequest) (*csi.DeleteSnapshotResponse, error) { klog.V(4).InfoS("DeleteSnapshot: called", "args", req) if err := validateDeleteSnapshotRequest(req); err != nil { return nil, err @@ -867,7 +833,7 @@ func validateDeleteSnapshotRequest(req *csi.DeleteSnapshotRequest) error { return nil } -func (d *controllerService) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) { +func (d *ControllerService) ListSnapshots(ctx context.Context, req *csi.ListSnapshotsRequest) (*csi.ListSnapshotsResponse, error) { klog.V(4).InfoS("ListSnapshots: called", "args", req) var snapshots []*cloud.Snapshot diff --git a/pkg/driver/controller_modify_volume.go b/pkg/driver/controller_modify_volume.go index ee49171503..9e033f39df 100644 --- a/pkg/driver/controller_modify_volume.go +++ b/pkg/driver/controller_modify_volume.go @@ -120,7 +120,7 @@ func (h *modifyVolumeRequestHandler) mergeModifyVolumeRequest(r *modifyVolumeReq // the ec2 API call to the CSI Driver main thread via response channels. // This method receives requests from CSI driver main thread via the request channel. When a new request is received from the request channel, we first // validate the new request. If the new request is acceptable, it will be merged with the existing request for the volume. -func (d *controllerService) processModifyVolumeRequests(h *modifyVolumeRequestHandler, responseChans []chan modifyVolumeResponse) { +func (d *ControllerService) processModifyVolumeRequests(h *modifyVolumeRequestHandler, responseChans []chan modifyVolumeResponse) { klog.V(4).InfoS("Start processing ModifyVolumeRequest for ", "volume ID", h.volumeID) process := func(req *modifyVolumeRequest) { if err := h.validateModifyVolumeRequest(req); err != nil { @@ -135,7 +135,7 @@ func (d *controllerService) processModifyVolumeRequests(h *modifyVolumeRequestHa select { case req := <-h.requestChan: process(req) - case <-time.After(d.driverOptions.modifyVolumeRequestHandlerTimeout): + case <-time.After(d.options.ModifyVolumeRequestHandlerTimeout): d.modifyVolumeManager.requestHandlerMap.Delete(h.volumeID) // At this point, no new requests can come in on the request channel because it has been removed from the map // However, the request channel may still have requests waiting on it @@ -167,7 +167,7 @@ func (d *controllerService) processModifyVolumeRequests(h *modifyVolumeRequestHa // If there’s ModifyVolumeRequestHandler for the volume, meaning that there is inflight request(s) for the volume, we will send the new request // to the goroutine for the volume via the receiving channel. // Note that each volume with inflight requests has their own goroutine which follows timeout schedule of their own. -func (d *controllerService) addModifyVolumeRequest(volumeID string, r *modifyVolumeRequest) { +func (d *ControllerService) addModifyVolumeRequest(volumeID string, r *modifyVolumeRequest) { requestHandler := newModifyVolumeRequestHandler(volumeID, r) handler, loaded := d.modifyVolumeManager.requestHandlerMap.LoadOrStore(volumeID, requestHandler) if loaded { @@ -179,7 +179,7 @@ func (d *controllerService) addModifyVolumeRequest(volumeID string, r *modifyVol } } -func (d *controllerService) executeModifyVolumeRequest(volumeID string, req *modifyVolumeRequest) (int32, error) { +func (d *ControllerService) executeModifyVolumeRequest(volumeID string, req *modifyVolumeRequest) (int32, error) { ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) defer cancel() actualSizeGiB, err := d.cloud.ResizeOrModifyDisk(ctx, volumeID, req.newSize, &req.modifyDiskOptions) @@ -190,14 +190,14 @@ func (d *controllerService) executeModifyVolumeRequest(volumeID string, req *mod } } -func (d *controllerService) GetCSIDriverModificationCapability( +func (d *ControllerService) GetCSIDriverModificationCapability( _ context.Context, _ *rpc.GetCSIDriverModificationCapabilityRequest, ) (*rpc.GetCSIDriverModificationCapabilityResponse, error) { return &rpc.GetCSIDriverModificationCapabilityResponse{}, nil } -func (d *controllerService) ModifyVolumeProperties( +func (d *ControllerService) ModifyVolumeProperties( ctx context.Context, req *rpc.ModifyVolumePropertiesRequest, ) (*rpc.ModifyVolumePropertiesResponse, error) { @@ -260,7 +260,7 @@ func parseModifyVolumeParameters(params map[string]string) (*cloud.ModifyDiskOpt return &options, nil } -func (d *controllerService) modifyVolumeWithCoalescing(ctx context.Context, volume string, options *cloud.ModifyDiskOptions) error { +func (d *ControllerService) modifyVolumeWithCoalescing(ctx context.Context, volume string, options *cloud.ModifyDiskOptions) error { responseChan := make(chan modifyVolumeResponse) request := modifyVolumeRequest{ modifyDiskOptions: *options, diff --git a/pkg/driver/controller_test.go b/pkg/driver/controller_test.go index 880c00e89c..9ae985b639 100644 --- a/pkg/driver/controller_test.go +++ b/pkg/driver/controller_test.go @@ -21,7 +21,6 @@ import ( "errors" "fmt" "math/rand" - "os" "reflect" "strings" "testing" @@ -48,107 +47,6 @@ const ( expDevicePath = "/dev/xvda" ) -func TestNewControllerService(t *testing.T) { - - var ( - cloudObj cloud.Cloud - testErr = errors.New("test error") - testRegion = "test-region" - - getNewCloudFunc = func(expectedRegion string, _ bool) func(region string, awsSdkDebugLog bool, userAgentExtra string, batching bool) (cloud.Cloud, error) { - return func(region string, awsSdkDebugLog bool, userAgentExtra string, batching bool) (cloud.Cloud, error) { - if region != expectedRegion { - t.Fatalf("expected region %q but got %q", expectedRegion, region) - } - return cloudObj, nil - } - } - ) - - testCases := []struct { - name string - region string - newCloudFunc func(string, bool, string, bool) (cloud.Cloud, error) - newMetadataFuncErrors bool - expectPanic bool - }{ - { - name: "AWS_REGION variable set, newCloud does not error", - region: "foo", - newCloudFunc: getNewCloudFunc("foo", false), - }, - { - name: "AWS_REGION variable set, newCloud errors", - region: "foo", - newCloudFunc: func(region string, awsSdkDebugLog bool, userAgentExtra string, batching bool) (cloud.Cloud, error) { - return nil, testErr - }, - expectPanic: true, - }, - { - name: "AWS_REGION variable not set, newMetadata does not error", - newCloudFunc: getNewCloudFunc(testRegion, false), - }, - { - name: "AWS_REGION variable not set, newMetadata errors", - newCloudFunc: getNewCloudFunc(testRegion, false), - newMetadataFuncErrors: true, - expectPanic: true, - }, - } - - driverOptions := &DriverOptions{ - endpoint: "test", - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - oldNewCloudFunc := NewCloudFunc - defer func() { NewCloudFunc = oldNewCloudFunc }() - NewCloudFunc = tc.newCloudFunc - - if tc.region == "" { - mockCtl := gomock.NewController(t) - defer mockCtl.Finish() - mockMetadataService := cloud.NewMockMetadataService(mockCtl) - - oldNewMetadataFunc := NewMetadataFunc - defer func() { NewMetadataFunc = oldNewMetadataFunc }() - NewMetadataFunc = func(cfg cloud.MetadataServiceConfig, region string) (cloud.MetadataService, error) { - if tc.newMetadataFuncErrors { - return nil, testErr - } - return mockMetadataService, nil - } - - if !tc.newMetadataFuncErrors { - mockMetadataService.EXPECT().GetRegion().Return(testRegion) - } - } else { - os.Setenv("AWS_REGION", tc.region) - defer os.Unsetenv("AWS_REGION") - } - - if tc.expectPanic { - defer func() { - if r := recover(); r == nil { - t.Errorf("The code did not panic") - } - }() - } - - controllerSvc := newControllerService(driverOptions) - - if controllerSvc.cloud != cloudObj { - t.Fatalf("expected cloud attribute to be equal to instantiated cloud object") - } - if !reflect.DeepEqual(controllerSvc.driverOptions, driverOptions) { - t.Fatalf("expected driverOptions attribute to be equal to input") - } - }) - } -} - func TestCreateVolume(t *testing.T) { stdVolCap := []*csi.VolumeCapability{ { @@ -221,10 +119,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -291,10 +189,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateVolume(ctx, req) @@ -358,10 +256,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } rsp, err := awsDriver.CreateVolume(ctx, req) @@ -414,10 +312,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } rsp, err := awsDriver.CreateVolume(ctx, req) @@ -463,10 +361,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(nil, cloud.ErrIdempotentParameterMismatch) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -490,10 +388,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -544,10 +442,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -630,10 +528,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err = awsDriver.CreateVolume(ctx, req) @@ -688,10 +586,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateVolume(ctx, req) @@ -749,10 +647,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateVolume(ctx, req) @@ -805,10 +703,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -847,10 +745,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -889,10 +787,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -931,10 +829,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -973,10 +871,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -1014,10 +912,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -1055,10 +953,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -1096,10 +994,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -1138,10 +1036,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -1187,10 +1085,10 @@ func TestCreateVolume(t *testing.T) { assert.Equal(t, int32(4000), diskOptions.IOPS) return mockDisk, nil }) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -1223,10 +1121,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -1263,10 +1161,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -1303,10 +1201,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -1377,10 +1275,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -1458,11 +1356,11 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Eq(diskOptions)).Return(mockDisk, nil) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - extraTags: map[string]string{ + options: &Options{ + ExtraTags: map[string]string{ extraVolumeTagKey: extraVolumeTagValue, }, }, @@ -1523,11 +1421,11 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Eq(diskOptions)).Return(mockDisk, nil) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - kubernetesClusterID: clusterID, + options: &Options{ + KubernetesClusterID: clusterID, }, } @@ -1589,10 +1487,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Eq(diskOptions)).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -1626,10 +1524,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -1667,10 +1565,10 @@ func TestCreateVolume(t *testing.T) { inFlight.Insert(req.GetName()) defer inFlight.Delete(req.GetName()) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: inFlight, - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: inFlight, + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -1695,10 +1593,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(nil, cloud.ErrIdempotentParameterMismatch) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -1729,10 +1627,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().CreateDisk(gomock.Eq(ctx), gomock.Eq(req.GetName()), gomock.Any()).Return(mockDisk, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateVolume(ctx, req); err != nil { @@ -1760,10 +1658,10 @@ func TestCreateVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateVolume(ctx, req) @@ -1921,10 +1819,10 @@ func TestCreateVolumeWithFormattingParameters(t *testing.T) { defer mockCtl.Finish() } - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } response, err := awsDriver.CreateVolume(ctx, req) @@ -1969,10 +1867,10 @@ func TestDeleteVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().DeleteDisk(gomock.Eq(ctx), gomock.Eq(req.GetVolumeId())).Return(true, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.DeleteVolume(ctx, req) if err != nil { @@ -2001,10 +1899,10 @@ func TestDeleteVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().DeleteDisk(gomock.Eq(ctx), gomock.Eq(req.GetVolumeId())).Return(false, cloud.ErrNotFound) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.DeleteVolume(ctx, req) if err != nil { @@ -2032,10 +1930,10 @@ func TestDeleteVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().DeleteDisk(gomock.Eq(ctx), gomock.Eq(req.GetVolumeId())).Return(false, fmt.Errorf("DeleteDisk could not delete volume")) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.DeleteVolume(ctx, req) if err != nil { @@ -2070,10 +1968,10 @@ func TestDeleteVolume(t *testing.T) { inFlight := internal.NewInFlight() inFlight.Insert(req.GetVolumeId()) defer inFlight.Delete(req.GetVolumeId()) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: inFlight, - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: inFlight, + options: &Options{}, } _, err := awsDriver.DeleteVolume(ctx, req) @@ -2344,10 +2242,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Any()).Return(mockSnapshot, nil) mockCloud.EXPECT().GetSnapshotByName(gomock.Eq(ctx), gomock.Eq(req.GetName())).Return(nil, cloud.ErrNotFound) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateSnapshot(context.Background(), req) if err != nil { @@ -2401,11 +2299,11 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Eq(snapshotOptions)).Return(mockSnapshot, nil) mockCloud.EXPECT().GetSnapshotByName(gomock.Eq(ctx), gomock.Eq(req.GetName())).Return(nil, cloud.ErrNotFound) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - kubernetesClusterID: clusterID, + options: &Options{ + KubernetesClusterID: clusterID, }, } resp, err := awsDriver.CreateSnapshot(context.Background(), req) @@ -2456,11 +2354,11 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Eq(snapshotOptions)).Return(mockSnapshot, nil) mockCloud.EXPECT().GetSnapshotByName(gomock.Eq(ctx), gomock.Eq(req.GetName())).Return(nil, cloud.ErrNotFound) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - extraTags: map[string]string{ + options: &Options{ + ExtraTags: map[string]string{ extraVolumeTagKey: extraVolumeTagValue, }, }, @@ -2488,10 +2386,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.CreateSnapshot(context.Background(), req); err != nil { srvErr, ok := status.FromError(err) @@ -2537,10 +2435,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().GetSnapshotByName(gomock.Eq(ctx), gomock.Eq(req.GetName())).Return(nil, cloud.ErrNotFound) mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Any()).Return(mockSnapshot, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateSnapshot(context.Background(), req) if err != nil { @@ -2604,10 +2502,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().GetSnapshotByName(gomock.Eq(ctx), gomock.Eq(req.GetName())).Return(nil, cloud.ErrNotFound) mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Any()).Return(mockSnapshot, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateSnapshot(context.Background(), req) if err != nil { @@ -2643,10 +2541,10 @@ func TestCreateSnapshot(t *testing.T) { inFlight.Insert(req.GetName()) defer inFlight.Delete(req.GetName()) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: inFlight, - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: inFlight, + options: &Options{}, } _, err := awsDriver.CreateSnapshot(context.Background(), req) @@ -2695,10 +2593,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Eq(snapshotOptions)).Return(mockSnapshot, nil) mockCloud.EXPECT().GetSnapshotByName(gomock.Eq(ctx), gomock.Eq(req.GetName())).Return(nil, cloud.ErrNotFound) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateSnapshot(context.Background(), req) if err != nil { @@ -2753,10 +2651,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Eq(snapshotOptions)).Return(mockSnapshot, nil) mockCloud.EXPECT().GetSnapshotByName(gomock.Eq(ctx), gomock.Eq(req.GetName())).Return(nil, cloud.ErrNotFound) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{kubernetesClusterID: clusterId}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{KubernetesClusterID: clusterId}, } resp, err := awsDriver.CreateSnapshot(context.Background(), req) if err != nil { @@ -2817,10 +2715,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Eq(snapshotOptions)).Return(mockSnapshot, nil).AnyTimes() mockCloud.EXPECT().EnableFastSnapshotRestores(gomock.Eq(ctx), gomock.Eq([]string{"us-east-1a", "us-east-1f"}), gomock.Eq(mockSnapshot.SnapshotID)).Return(expOutput, nil).AnyTimes() - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateSnapshot(context.Background(), req) @@ -2881,10 +2779,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().CreateSnapshot(gomock.Eq(ctx), gomock.Eq(req.GetSourceVolumeId()), gomock.Eq(snapshotOptions)).Return(mockSnapshot, nil).AnyTimes() mockCloud.EXPECT().EnableFastSnapshotRestores(gomock.Eq(ctx), gomock.Eq([]string{"us-east-1a", "us-east-1f"}), gomock.Eq(mockSnapshot.SnapshotID)).Return(expOutput, nil).AnyTimes() - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.CreateSnapshot(context.Background(), req) @@ -2950,10 +2848,10 @@ func TestCreateSnapshot(t *testing.T) { Return(expOutput, fmt.Errorf("Failed to create Fast Snapshot Restores")).AnyTimes() mockCloud.EXPECT().DeleteSnapshot(gomock.Eq(ctx), gomock.Eq(mockSnapshot.SnapshotID)).Return(true, nil).AnyTimes() - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateSnapshot(context.Background(), req) @@ -2986,10 +2884,10 @@ func TestCreateSnapshot(t *testing.T) { mockCloud.EXPECT().AvailabilityZones(gomock.Eq(ctx)).Return(map[string]struct{}{ "us-east-1a": {}, "us-east-1b": {}}, nil).AnyTimes() - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateSnapshot(context.Background(), req) @@ -3039,10 +2937,10 @@ func TestCreateSnapshot(t *testing.T) { gomock.Eq(mockSnapshot.SnapshotID)).Return(nil, fmt.Errorf("error")).AnyTimes() mockCloud.EXPECT().DeleteSnapshot(gomock.Eq(ctx), gomock.Eq(mockSnapshot.SnapshotID)).Return(true, nil).AnyTimes() - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } _, err := awsDriver.CreateSnapshot(context.Background(), req) @@ -3072,10 +2970,10 @@ func TestDeleteSnapshot(t *testing.T) { defer mockCtl.Finish() mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } req := &csi.DeleteSnapshotRequest{ @@ -3097,10 +2995,10 @@ func TestDeleteSnapshot(t *testing.T) { defer mockCtl.Finish() mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } req := &csi.DeleteSnapshotRequest{ @@ -3130,10 +3028,10 @@ func TestDeleteSnapshot(t *testing.T) { inFlight.Insert(req.GetSnapshotId()) defer inFlight.Delete(req.GetSnapshotId()) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: inFlight, - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: inFlight, + options: &Options{}, } _, err := awsDriver.DeleteSnapshot(ctx, req) @@ -3182,10 +3080,10 @@ func TestListSnapshots(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int32(0)), gomock.Eq("")).Return(mockCloudSnapshotsResponse, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.ListSnapshots(context.Background(), req) @@ -3209,10 +3107,10 @@ func TestListSnapshots(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int32(0)), gomock.Eq("")).Return(nil, cloud.ErrNotFound) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.ListSnapshots(context.Background(), req) @@ -3245,10 +3143,10 @@ func TestListSnapshots(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().GetSnapshotByID(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(mockCloudSnapshotsResponse, nil) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.ListSnapshots(context.Background(), req) @@ -3275,10 +3173,10 @@ func TestListSnapshots(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().GetSnapshotByID(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(nil, cloud.ErrNotFound) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } resp, err := awsDriver.ListSnapshots(context.Background(), req) @@ -3305,10 +3203,10 @@ func TestListSnapshots(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().GetSnapshotByID(gomock.Eq(ctx), gomock.Eq("snapshot-1")).Return(nil, cloud.ErrMultiSnapshots) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.ListSnapshots(context.Background(), req); err != nil { @@ -3337,10 +3235,10 @@ func TestListSnapshots(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().ListSnapshots(gomock.Eq(ctx), gomock.Eq(""), gomock.Eq(int32(4)), gomock.Eq("")).Return(nil, cloud.ErrInvalidMaxResults) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } if _, err := awsDriver.ListSnapshots(context.Background(), req); err != nil { @@ -3381,7 +3279,7 @@ func TestControllerPublishVolume(t *testing.T) { mockAttach func(mockCloud *cloud.MockCloud, ctx context.Context, volumeId string, nodeId string) expResp *csi.ControllerPublishVolumeResponse errorCode codes.Code - setupFunc func(controllerService *controllerService) + setupFunc func(ControllerService *ControllerService) }{ { name: "AttachDisk successfully with valid volume ID, node ID, and volume capability", @@ -3479,8 +3377,8 @@ func TestControllerPublishVolume(t *testing.T) { mockAttach: func(mockCloud *cloud.MockCloud, ctx context.Context, volumeId string, nodeId string) { }, errorCode: codes.Aborted, - setupFunc: func(controllerService *controllerService) { - controllerService.inFlight.Insert("vol-test" + expInstanceID) + setupFunc: func(ControllerService *ControllerService) { + ControllerService.inFlight.Insert("vol-test" + expInstanceID) }, }, } @@ -3526,7 +3424,7 @@ func TestControllerUnpublishVolume(t *testing.T) { errorCode codes.Code mockDetach func(mockCloud *cloud.MockCloud, ctx context.Context, volumeId string, nodeId string) expResp *csi.ControllerUnpublishVolumeResponse - setupFunc func(driver *controllerService) + setupFunc func(driver *ControllerService) }{ { name: "DetachDisk successfully with valid volume ID and node ID", @@ -3575,7 +3473,7 @@ func TestControllerUnpublishVolume(t *testing.T) { volumeId: "vol-test", nodeId: expInstanceID, errorCode: codes.Aborted, - setupFunc: func(driver *controllerService) { + setupFunc: func(driver *ControllerService) { driver.inFlight.Insert("vol-test" + expInstanceID) }, }, @@ -3668,10 +3566,10 @@ func TestControllerExpandVolume(t *testing.T) { mockCloud := cloud.NewMockCloud(mockCtl) mockCloud.EXPECT().ResizeOrModifyDisk(gomock.Any(), gomock.Eq(tc.req.GetVolumeId()), gomock.Any(), gomock.Any()).Return(retSizeGiB, nil).AnyTimes() - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + options: &Options{}, modifyVolumeManager: newModifyVolumeManager(), } @@ -3713,14 +3611,14 @@ func checkExpectedErrorCode(t *testing.T, err error, expectedCode codes.Code) { } } -func createControllerService(t *testing.T) (controllerService, *gomock.Controller, *cloud.MockCloud) { +func createControllerService(t *testing.T) (ControllerService, *gomock.Controller, *cloud.MockCloud) { t.Helper() mockCtl := gomock.NewController(t) mockCloud := cloud.NewMockCloud(mockCtl) - awsDriver := controllerService{ - cloud: mockCloud, - inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + awsDriver := ControllerService{ + cloud: mockCloud, + inFlight: internal.NewInFlight(), + options: &Options{}, } return awsDriver, mockCtl, mockCloud } diff --git a/pkg/driver/driver.go b/pkg/driver/driver.go index 04756406aa..544f8e3fe7 100644 --- a/pkg/driver/driver.go +++ b/pkg/driver/driver.go @@ -20,12 +20,10 @@ import ( "context" "fmt" "net" - "time" "github.com/awslabs/volume-modifier-for-k8s/pkg/rpc" csi "github.com/container-storage-interface/spec/lib/go/csi" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver/internal" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/util" "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "google.golang.org/grpc" @@ -45,12 +43,11 @@ const ( ) const ( - DriverName = "ebs.csi.aws.com" - AwsPartitionKey = "topology." + DriverName + "/partition" - AwsAccountIDKey = "topology." + DriverName + "/account-id" - AwsRegionKey = "topology." + DriverName + "/region" - AwsOutpostIDKey = "topology." + DriverName + "/outpost-id" - + DriverName = "ebs.csi.aws.com" + AwsPartitionKey = "topology." + DriverName + "/partition" + AwsAccountIDKey = "topology." + DriverName + "/account-id" + AwsRegionKey = "topology." + DriverName + "/region" + AwsOutpostIDKey = "topology." + DriverName + "/outpost-id" WellKnownZoneTopologyKey = "topology.kubernetes.io/zone" // DEPRECATED Use the WellKnownZoneTopologyKey instead ZoneTopologyKey = "topology." + DriverName + "/zone" @@ -58,89 +55,40 @@ const ( ) type Driver struct { - controllerService + controller *ControllerService nodeService - srv *grpc.Server - options *DriverOptions -} - -type DriverOptions struct { - endpoint string - extraTags map[string]string - mode Mode - volumeAttachLimit int64 - reservedVolumeAttachments int - kubernetesClusterID string - awsSdkDebugLog bool - batching bool - warnOnInvalidTag bool - userAgentExtra string - otelTracing bool - modifyVolumeRequestHandlerTimeout time.Duration + options *Options } -func NewDriver(options ...func(*DriverOptions)) (*Driver, error) { +func NewDriver(c cloud.Cloud, o *Options) (*Driver, error) { klog.InfoS("Driver Information", "Driver", DriverName, "Version", driverVersion) - driverOptions := DriverOptions{ - endpoint: DefaultCSIEndpoint, - mode: AllMode, - modifyVolumeRequestHandlerTimeout: DefaultModifyVolumeRequestHandlerTimeout, - } - for _, option := range options { - option(&driverOptions) - } - - if err := ValidateDriverOptions(&driverOptions); err != nil { - return nil, fmt.Errorf("Invalid driver options: %w", err) + if err := ValidateDriverOptions(o); err != nil { + return nil, fmt.Errorf("invalid driver options: %w", err) } - driver := Driver{ - options: &driverOptions, + driver := &Driver{ + options: o, } - switch driverOptions.mode { + switch o.Mode { case ControllerMode: - driver.controllerService = newControllerService(&driverOptions) + driver.controller = NewControllerService(c, o) case NodeMode: - driver.nodeService = newNodeService(&driverOptions) + driver.nodeService = newNodeService(o) case AllMode: - driver.controllerService = newControllerService(&driverOptions) - driver.nodeService = newNodeService(&driverOptions) + driver.controller = NewControllerService(c, o) + driver.nodeService = newNodeService(o) default: - return nil, fmt.Errorf("unknown mode: %s", driverOptions.mode) + return nil, fmt.Errorf("unknown mode: %s", o.Mode) } - return &driver, nil -} - -func NewFakeDriver(e string, c cloud.Cloud, md *cloud.Metadata, m Mounter) (*Driver, error) { - driverOptions := &DriverOptions{ - endpoint: e, - mode: AllMode, - } - driver := Driver{ - options: driverOptions, - controllerService: controllerService{ - cloud: c, - inFlight: internal.NewInFlight(), - driverOptions: driverOptions, - modifyVolumeManager: newModifyVolumeManager(), - }, - nodeService: nodeService{ - metadata: md, - deviceIdentifier: newNodeDeviceIdentifier(), - inFlight: internal.NewInFlight(), - mounter: m, - driverOptions: driverOptions, - }, - } - return &driver, nil + return driver, nil } func (d *Driver) Run() error { - scheme, addr, err := util.ParseEndpoint(d.options.endpoint) + scheme, addr, err := util.ParseEndpoint(d.options.Endpoint) if err != nil { return err } @@ -161,25 +109,26 @@ func (d *Driver) Run() error { opts := []grpc.ServerOption{ grpc.UnaryInterceptor(logErr), } - if d.options.otelTracing { + + if d.options.EnableOtelTracing { opts = append(opts, grpc.StatsHandler(otelgrpc.NewServerHandler())) } - d.srv = grpc.NewServer(opts...) + d.srv = grpc.NewServer(opts...) csi.RegisterIdentityServer(d.srv, d) - switch d.options.mode { + switch d.options.Mode { case ControllerMode: - csi.RegisterControllerServer(d.srv, d) - rpc.RegisterModifyServer(d.srv, d) + csi.RegisterControllerServer(d.srv, d.controller) + rpc.RegisterModifyServer(d.srv, d.controller) case NodeMode: csi.RegisterNodeServer(d.srv, d) case AllMode: - csi.RegisterControllerServer(d.srv, d) + csi.RegisterControllerServer(d.srv, d.controller) csi.RegisterNodeServer(d.srv, d) - rpc.RegisterModifyServer(d.srv, d) + rpc.RegisterModifyServer(d.srv, d.controller) default: - return fmt.Errorf("unknown mode: %s", d.options.mode) + return fmt.Errorf("unknown mode: %s", d.options.Mode) } klog.V(4).InfoS("Listening for connections", "address", listener.Addr()) @@ -189,87 +138,3 @@ func (d *Driver) Run() error { func (d *Driver) Stop() { d.srv.Stop() } - -func WithEndpoint(endpoint string) func(*DriverOptions) { - return func(o *DriverOptions) { - o.endpoint = endpoint - } -} - -func WithExtraTags(extraTags map[string]string) func(*DriverOptions) { - return func(o *DriverOptions) { - o.extraTags = extraTags - } -} - -func WithExtraVolumeTags(extraVolumeTags map[string]string) func(*DriverOptions) { - return func(o *DriverOptions) { - if o.extraTags == nil && extraVolumeTags != nil { - klog.InfoS("DEPRECATION WARNING: --extra-volume-tags is deprecated, please use --extra-tags instead") - o.extraTags = extraVolumeTags - } - } -} - -func WithMode(mode Mode) func(*DriverOptions) { - return func(o *DriverOptions) { - o.mode = mode - } -} - -func WithVolumeAttachLimit(volumeAttachLimit int64) func(*DriverOptions) { - return func(o *DriverOptions) { - o.volumeAttachLimit = volumeAttachLimit - } -} - -func WithReservedVolumeAttachments(reservedVolumeAttachments int) func(*DriverOptions) { - return func(o *DriverOptions) { - o.reservedVolumeAttachments = reservedVolumeAttachments - } -} - -func WithBatching(enableBatching bool) func(*DriverOptions) { - return func(o *DriverOptions) { - o.batching = enableBatching - } -} - -func WithKubernetesClusterID(clusterID string) func(*DriverOptions) { - return func(o *DriverOptions) { - o.kubernetesClusterID = clusterID - } -} - -func WithAwsSdkDebugLog(enableSdkDebugLog bool) func(*DriverOptions) { - return func(o *DriverOptions) { - o.awsSdkDebugLog = enableSdkDebugLog - } -} - -func WithWarnOnInvalidTag(warnOnInvalidTag bool) func(*DriverOptions) { - return func(o *DriverOptions) { - o.warnOnInvalidTag = warnOnInvalidTag - } -} - -func WithUserAgentExtra(userAgentExtra string) func(*DriverOptions) { - return func(o *DriverOptions) { - o.userAgentExtra = userAgentExtra - } -} - -func WithOtelTracing(enableOtelTracing bool) func(*DriverOptions) { - return func(o *DriverOptions) { - o.otelTracing = enableOtelTracing - } -} - -func WithModifyVolumeRequestHandlerTimeout(timeout time.Duration) func(*DriverOptions) { - return func(o *DriverOptions) { - if timeout == 0 { - return - } - o.modifyVolumeRequestHandlerTimeout = timeout - } -} diff --git a/pkg/driver/driver_test.go b/pkg/driver/driver_test.go deleted file mode 100644 index 3527968c40..0000000000 --- a/pkg/driver/driver_test.go +++ /dev/null @@ -1,143 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package driver - -import ( - "reflect" - "testing" - "time" -) - -func TestWithEndpoint(t *testing.T) { - value := "endpoint" - options := &DriverOptions{} - WithEndpoint(value)(options) - if options.endpoint != value { - t.Fatalf("expected endpoint option got set to %q but is set to %q", value, options.endpoint) - } -} - -func TestWithExtraTags(t *testing.T) { - value := map[string]string{"foo": "bar"} - options := &DriverOptions{} - WithExtraTags(value)(options) - if !reflect.DeepEqual(options.extraTags, value) { - t.Fatalf("expected extraTags option got set to %+v but is set to %+v", value, options.extraTags) - } -} - -func TestWithExtraVolumeTags(t *testing.T) { - value := map[string]string{"foo": "bar"} - options := &DriverOptions{} - WithExtraVolumeTags(value)(options) - if !reflect.DeepEqual(options.extraTags, value) { - t.Fatalf("expected extraTags option got set to %+v but is set to %+v", value, options.extraTags) - } -} - -func TestWithExtraVolumeTagsNoOverwrite(t *testing.T) { - extraTagsValue := map[string]string{"foo": "bar"} - options := &DriverOptions{} - WithExtraTags(extraTagsValue)(options) - extraVolumeTagsValue := map[string]string{"baz": "qux"} - WithExtraVolumeTags(extraVolumeTagsValue)(options) - if !reflect.DeepEqual(options.extraTags, extraTagsValue) { - t.Fatalf("expected extraTags option got set to %+v but is set to %+v", extraTagsValue, options.extraTags) - } -} - -func TestWithMode(t *testing.T) { - value := Mode("mode") - options := &DriverOptions{} - WithMode(value)(options) - if options.mode != value { - t.Fatalf("expected mode option got set to %q but is set to %q", value, options.mode) - } -} - -func TestWithVolumeAttachLimit(t *testing.T) { - var value int64 = 42 - options := &DriverOptions{} - WithVolumeAttachLimit(value)(options) - if options.volumeAttachLimit != value { - t.Fatalf("expected volumeAttachLimit option got set to %d but is set to %d", value, options.volumeAttachLimit) - } -} - -func TestWithVolumeAttachLimitFromMetadata(t *testing.T) { - value := 10 - options := &DriverOptions{} - WithReservedVolumeAttachments(value)(options) - if options.reservedVolumeAttachments != value { - t.Fatalf("expected reservedVolumeAttachments option got set to %d but is set to %d", value, options.reservedVolumeAttachments) - } -} - -func TestWithClusterID(t *testing.T) { - var id string = "test-cluster-id" - options := &DriverOptions{} - WithKubernetesClusterID(id)(options) - if options.kubernetesClusterID != id { - t.Fatalf("expected kubernetesClusterID option got set to %s but is set to %s", id, options.kubernetesClusterID) - } -} - -func TestWithAwsSdkDebugLog(t *testing.T) { - var enableSdkDebugLog bool = true - options := &DriverOptions{} - WithAwsSdkDebugLog(enableSdkDebugLog)(options) - if options.awsSdkDebugLog != enableSdkDebugLog { - t.Fatalf("expected awsSdkDebugLog option got set to %v but is set to %v", enableSdkDebugLog, options.awsSdkDebugLog) - } -} - -func TestWithUserAgentExtra(t *testing.T) { - var userAgentExtra string = "test-user-agent" - options := &DriverOptions{} - WithUserAgentExtra(userAgentExtra)(options) - if options.userAgentExtra != userAgentExtra { - t.Fatalf("expected userAgentExtra option got set to %s but is set to %s", userAgentExtra, options.userAgentExtra) - } -} - -func TestWithOtelTracing(t *testing.T) { - var enableOtelTracing bool = true - options := &DriverOptions{} - WithOtelTracing(enableOtelTracing)(options) - if options.otelTracing != enableOtelTracing { - t.Fatalf("expected otelTracing option got set to %v but is set to %v", enableOtelTracing, options.otelTracing) - } -} - -func TestWithBatching(t *testing.T) { - var batching bool = true - options := &DriverOptions{} - WithBatching(batching)(options) - if options.batching != batching { - t.Fatalf("expected batching option got set to %v but is set to %v", batching, options.batching) - } -} - -func TestWithModifyVolumeRequestHandlerTimeout(t *testing.T) { - timeout := 15 * time.Second - options := &DriverOptions{} - WithModifyVolumeRequestHandlerTimeout(timeout)(options) - if options.modifyVolumeRequestHandlerTimeout != timeout { - t.Fatalf("expected modifyVolumeRequestHandlerTimeout option got set to %v but is set to %v", - timeout, options.modifyVolumeRequestHandlerTimeout) - } -} diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 1d25ade080..ff12ef3e2e 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -28,6 +28,7 @@ import ( csi "github.com/container-storage-interface/spec/lib/go/csi" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud/metadata" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver/internal" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/util" "google.golang.org/grpc/codes" @@ -82,26 +83,26 @@ var ( // nodeService represents the node service of CSI driver type nodeService struct { - metadata cloud.MetadataService + metadata metadata.MetadataService mounter Mounter deviceIdentifier DeviceIdentifier inFlight *internal.InFlight - driverOptions *DriverOptions + options *Options } // newNodeService creates a new node service // it panics if failed to create the service -func newNodeService(driverOptions *DriverOptions) nodeService { +func newNodeService(o *Options) nodeService { klog.V(5).InfoS("[Debug] Retrieving node info from metadata service") region := os.Getenv("AWS_REGION") klog.InfoS("regionFromSession Node service", "region", region) - cfg := cloud.MetadataServiceConfig{ - EC2MetadataClient: cloud.DefaultEC2MetadataClient, - K8sAPIClient: cloud.DefaultKubernetesAPIClient, + cfg := metadata.MetadataServiceConfig{ + EC2MetadataClient: metadata.DefaultEC2MetadataClient, + K8sAPIClient: metadata.DefaultKubernetesAPIClient, } - metadata, err := cloud.NewMetadataService(cfg, region) + metadata, err := metadata.NewMetadataService(cfg, region) if err != nil { panic(err) } @@ -114,7 +115,7 @@ func newNodeService(driverOptions *DriverOptions) nodeService { // Remove taint from node to indicate driver startup success // This is done at the last possible moment to prevent race conditions or false positive removals time.AfterFunc(taintRemovalInitialDelay, func() { - removeTaintInBackground(cloud.DefaultKubernetesAPIClient, removeNotReadyTaint) + removeTaintInBackground(cfg.K8sAPIClient, removeNotReadyTaint) }) return nodeService{ @@ -122,7 +123,7 @@ func newNodeService(driverOptions *DriverOptions) nodeService { mounter: nodeMounter, deviceIdentifier: newNodeDeviceIdentifier(), inFlight: internal.NewInFlight(), - driverOptions: driverOptions, + options: o, } } @@ -786,8 +787,8 @@ func (d *nodeService) nodePublishVolumeForFileSystem(req *csi.NodePublishVolumeR // getVolumesLimit returns the limit of volumes that the node supports func (d *nodeService) getVolumesLimit() int64 { - if d.driverOptions.volumeAttachLimit >= 0 { - return d.driverOptions.volumeAttachLimit + if d.options.VolumeAttachLimit >= 0 { + return d.options.VolumeAttachLimit } if util.IsSBE(d.metadata.GetRegion()) { @@ -799,7 +800,7 @@ func (d *nodeService) getVolumesLimit() int64 { isNitro := cloud.IsNitroInstanceType(instanceType) availableAttachments := cloud.GetMaxAttachments(isNitro) - reservedVolumeAttachments := d.driverOptions.reservedVolumeAttachments + reservedVolumeAttachments := d.options.ReservedVolumeAttachments if reservedVolumeAttachments == -1 { reservedVolumeAttachments = d.metadata.GetNumBlockDeviceMappings() + 1 // +1 for the root device } @@ -874,7 +875,7 @@ type JSONPatch struct { } // removeTaintInBackground is a goroutine that retries removeNotReadyTaint with exponential backoff -func removeTaintInBackground(k8sClient cloud.KubernetesAPIClient, removalFunc func(cloud.KubernetesAPIClient) error) { +func removeTaintInBackground(k8sClient metadata.KubernetesAPIClient, removalFunc func(metadata.KubernetesAPIClient) error) { backoffErr := wait.ExponentialBackoff(taintRemovalBackoff, func() (bool, error) { err := removalFunc(k8sClient) if err != nil { @@ -892,7 +893,7 @@ func removeTaintInBackground(k8sClient cloud.KubernetesAPIClient, removalFunc fu // removeNotReadyTaint removes the taint ebs.csi.aws.com/agent-not-ready from the local node // This taint can be optionally applied by users to prevent startup race conditions such as // https://github.com/kubernetes/kubernetes/issues/95911 -func removeNotReadyTaint(k8sClient cloud.KubernetesAPIClient) error { +func removeNotReadyTaint(k8sClient metadata.KubernetesAPIClient) error { nodeName := os.Getenv("CSI_NODE_NAME") if nodeName == "" { klog.V(4).InfoS("CSI_NODE_NAME missing, skipping taint removal") diff --git a/pkg/driver/node_linux_test.go b/pkg/driver/node_linux_test.go index ade833c614..00c787ec91 100644 --- a/pkg/driver/node_linux_test.go +++ b/pkg/driver/node_linux_test.go @@ -27,7 +27,7 @@ import ( "time" "github.com/golang/mock/gomock" - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud/metadata" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver/internal" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -160,20 +160,20 @@ func TestFindDevicePath(t *testing.T) { mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) nodeDriver := nodeService{ - metadata: &cloud.Metadata{}, + metadata: &metadata.Metadata{}, mounter: mockMounter, deviceIdentifier: mockDeviceIdentifier, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + options: &Options{}, } if tc.expectDevicePath == snowDevicePath+tc.partition { nodeDriver = nodeService{ - metadata: &cloud.Metadata{Region: "snow"}, + metadata: &metadata.Metadata{Region: "snow"}, mounter: mockMounter, deviceIdentifier: mockDeviceIdentifier, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{}, + options: &Options{}, } } diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index 91cd050606..e336cb5b15 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -34,6 +34,7 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" "github.com/golang/mock/gomock" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" + "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud/metadata" "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver/internal" "github.com/stretchr/testify/assert" "google.golang.org/grpc/codes" @@ -469,7 +470,7 @@ func TestNodeStageVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -513,7 +514,7 @@ func TestNodeUnstageVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -544,7 +545,7 @@ func TestNodeUnstageVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -573,7 +574,7 @@ func TestNodeUnstageVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -604,7 +605,7 @@ func TestNodeUnstageVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -629,7 +630,7 @@ func TestNodeUnstageVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -653,7 +654,7 @@ func TestNodeUnstageVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -681,7 +682,7 @@ func TestNodeUnstageVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -734,7 +735,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -769,7 +770,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -803,7 +804,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -838,7 +839,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -877,7 +878,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -921,7 +922,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -957,7 +958,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1005,7 +1006,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1053,7 +1054,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1100,7 +1101,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1147,7 +1148,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1200,7 +1201,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1249,7 +1250,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1298,7 +1299,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1335,7 +1336,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1371,7 +1372,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1410,7 +1411,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1439,7 +1440,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1468,7 +1469,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1497,7 +1498,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1525,7 +1526,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1559,7 +1560,7 @@ func TestNodePublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1601,7 +1602,7 @@ func TestNodeExpandVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1699,7 +1700,7 @@ func TestNodeUnpublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1728,7 +1729,7 @@ func TestNodeUnpublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1753,7 +1754,7 @@ func TestNodeUnpublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1778,7 +1779,7 @@ func TestNodeUnpublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1805,7 +1806,7 @@ func TestNodeUnpublishVolume(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -1844,7 +1845,7 @@ func TestNodeGetVolumeStats(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) VolumePath := "./test" @@ -1879,7 +1880,7 @@ func TestNodeGetVolumeStats(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) VolumePath := "/test" @@ -1907,7 +1908,7 @@ func TestNodeGetVolumeStats(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) VolumePath := "/test" @@ -1935,7 +1936,7 @@ func TestNodeGetVolumeStats(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) VolumePath := "/test" @@ -1969,7 +1970,7 @@ func TestNodeGetCapabilities(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) @@ -2285,15 +2286,15 @@ func TestNodeGetInfo(t *testing.T) { mockCtl := gomock.NewController(t) defer mockCtl.Finish() - driverOptions := &DriverOptions{ - volumeAttachLimit: tc.volumeAttachLimit, - reservedVolumeAttachments: tc.reservedVolumeAttachments, + options := &Options{ + VolumeAttachLimit: tc.volumeAttachLimit, + ReservedVolumeAttachments: tc.reservedVolumeAttachments, } mockMounter := NewMockMounter(mockCtl) mockDeviceIdentifier := NewMockDeviceIdentifier(mockCtl) - mockMetadata := cloud.NewMockMetadataService(mockCtl) + mockMetadata := metadata.NewMockMetadataService(mockCtl) mockMetadata.EXPECT().GetInstanceID().Return(tc.instanceID) mockMetadata.EXPECT().GetAvailabilityZone().Return(tc.availabilityZone) mockMetadata.EXPECT().GetOutpostArn().Return(tc.outpostArn) @@ -2314,7 +2315,7 @@ func TestNodeGetInfo(t *testing.T) { mounter: mockMounter, deviceIdentifier: mockDeviceIdentifier, inFlight: internal.NewInFlight(), - driverOptions: driverOptions, + options: options, } resp, err := awsDriver.NodeGetInfo(context.TODO(), &csi.NodeGetInfoRequest{}) @@ -2709,7 +2710,7 @@ func TestRemoveNotReadyTaint(t *testing.T) { func TestRemoveTaintInBackground(t *testing.T) { mockRemovalCount := 0 - mockRemovalFunc := func(_ cloud.KubernetesAPIClient) error { + mockRemovalFunc := func(_ metadata.KubernetesAPIClient) error { mockRemovalCount += 1 if mockRemovalCount == 3 { return nil diff --git a/pkg/driver/options.go b/pkg/driver/options.go new file mode 100644 index 0000000000..1768b95137 --- /dev/null +++ b/pkg/driver/options.go @@ -0,0 +1,111 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package driver + +import ( + "fmt" + "time" + + flag "github.com/spf13/pflag" + cliflag "k8s.io/component-base/cli/flag" +) + +// Options contains options and configuration settings for the driver. +type Options struct { + Mode Mode + + // #### Server options #### + + //Endpoint is the endpoint for the CSI driver server + Endpoint string + // HttpEndpoint is the TCP network address where the HTTP server for metrics will listen + HttpEndpoint string + // EnableOtelTracing is a flag to enable opentelemetry tracing for the driver + EnableOtelTracing bool + + // #### Controller options #### + + // ExtraTags is a map of tags that will be attached to each dynamically provisioned + // resource. + ExtraTags map[string]string + // ExtraVolumeTags is a map of tags that will be attached to each dynamically provisioned + // volume. + // DEPRECATED: Use ExtraTags instead. + ExtraVolumeTags map[string]string + // ID of the kubernetes cluster. + KubernetesClusterID string + // flag to enable sdk debug log + AwsSdkDebugLog bool + // flag to warn on invalid tag, instead of returning an error + WarnOnInvalidTag bool + // flag to set user agent + UserAgentExtra string + // flag to enable batching of API calls + Batching bool + // flag to set the timeout for volume modification requests to be coalesced into a single + // volume modification call to AWS. + ModifyVolumeRequestHandlerTimeout time.Duration + + // #### Node options ##### + + // VolumeAttachLimit specifies the value that shall be reported as "maximum number of attachable volumes" + // in CSINode objects. It is similar to https://kubernetes.io/docs/concepts/storage/storage-limits/#custom-limits + // which allowed administrators to specify custom volume limits by configuring the kube-scheduler. Also, each AWS + // machine type has different volume limits. By default, the EBS CSI driver parses the machine type name and then + // decides the volume limit. However, this is only a rough approximation and not good enough in most cases. + // Specifying the volume attach limit via command line is the alternative until a more sophisticated solution presents + // itself (dynamically discovering the maximum number of attachable volume per EC2 machine type, see also + // https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/347). + VolumeAttachLimit int64 + // ReservedVolumeAttachments specifies number of volume attachments reserved for system use. + // Typically 1 for the root disk, but may be larger when more system disks are attached to nodes. + // This option is not used when --volume-attach-limit is specified. + // 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 (and therefore it may be wrong). + ReservedVolumeAttachments int +} + +func (o *Options) AddFlags(f *flag.FlagSet) { + // Server options + f.StringVar(&o.Endpoint, "endpoint", DefaultCSIEndpoint, "Endpoint for the CSI driver server") + f.StringVar(&o.HttpEndpoint, "http-endpoint", "", "The TCP network address where the HTTP server for metrics will listen (example: `:8080`). The default is empty string, which means the server is disabled.") + f.BoolVar(&o.EnableOtelTracing, "enable-otel-tracing", false, "To enable opentelemetry tracing for the driver. The tracing is disabled by default. Configure the exporter endpoint with OTEL_EXPORTER_OTLP_ENDPOINT and other env variables, see https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration.") + + // Controller options + if o.Mode == AllMode || o.Mode == ControllerMode { + f.Var(cliflag.NewMapStringString(&o.ExtraTags), "extra-tags", "Extra tags to attach to each dynamically provisioned resource. It is a comma separated list of key value pairs like '=,='") + f.Var(cliflag.NewMapStringString(&o.ExtraVolumeTags), "extra-volume-tags", "DEPRECATED: Please use --extra-tags instead. Extra volume tags to attach to each dynamically provisioned volume. It is a comma separated list of key value pairs like '=,='") + f.StringVar(&o.KubernetesClusterID, "k8s-tag-cluster-id", "", "ID of the Kubernetes cluster used for tagging provisioned EBS volumes (optional).") + f.BoolVar(&o.AwsSdkDebugLog, "aws-sdk-debug-log", false, "To enable the aws sdk debug log level (default to false).") + f.BoolVar(&o.WarnOnInvalidTag, "warn-on-invalid-tag", false, "To warn on invalid tags, instead of returning an error") + f.StringVar(&o.UserAgentExtra, "user-agent-extra", "", "Extra string appended to user agent.") + f.BoolVar(&o.Batching, "batching", false, "To enable batching of API calls. This is especially helpful for improving performance in workloads that are sensitive to EC2 rate limits.") + f.DurationVar(&o.ModifyVolumeRequestHandlerTimeout, "modify-volume-request-handler-timeout", DefaultModifyVolumeRequestHandlerTimeout, "Timeout for the window in which volume modification calls must be received in order for them to coalesce into a single volume modification call to AWS. This must be lower than the csi-resizer and volumemodifier timeouts") + } + // Node options + if o.Mode == AllMode || o.Mode == NodeMode { + f.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.") + f.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: - - . 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 *Options) 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 +} diff --git a/pkg/driver/options_test.go b/pkg/driver/options_test.go new file mode 100644 index 0000000000..c5fc111a04 --- /dev/null +++ b/pkg/driver/options_test.go @@ -0,0 +1,144 @@ +package driver + +import ( + "testing" + "time" + + flag "github.com/spf13/pflag" +) + +func TestAddFlags(t *testing.T) { + o := &Options{} + o.Mode = AllMode + + f := flag.NewFlagSet("test", flag.ExitOnError) + o.AddFlags(f) + + if err := f.Set("endpoint", "custom-endpoint"); err != nil { + t.Errorf("error setting endpoint: %v", err) + } + if err := f.Set("http-endpoint", ":8080"); err != nil { + t.Errorf("error setting http-endpoint: %v", err) + } + if err := f.Set("enable-otel-tracing", "true"); err != nil { + t.Errorf("error setting enable-otel-tracing: %v", err) + } + if err := f.Set("extra-tags", "key1=value1,key2=value2"); err != nil { + t.Errorf("error setting extra-tags: %v", err) + } + if err := f.Set("k8s-tag-cluster-id", "cluster-123"); err != nil { + t.Errorf("error setting k8s-tag-cluster-id: %v", err) + } + if err := f.Set("aws-sdk-debug-log", "true"); err != nil { + t.Errorf("error setting aws-sdk-debug-log: %v", err) + } + if err := f.Set("warn-on-invalid-tag", "true"); err != nil { + t.Errorf("error setting warn-on-invalid-tag: %v", err) + } + if err := f.Set("user-agent-extra", "extra-info"); err != nil { + t.Errorf("error setting user-agent-extra: %v", err) + } + if err := f.Set("batching", "true"); err != nil { + t.Errorf("error setting batching: %v", err) + } + if err := f.Set("modify-volume-request-handler-timeout", "1m"); err != nil { + t.Errorf("error setting modify-volume-request-handler-timeout: %v", err) + } + if err := f.Set("volume-attach-limit", "10"); err != nil { + t.Errorf("error setting volume-attach-limit: %v", err) + } + if err := f.Set("reserved-volume-attachments", "5"); err != nil { + t.Errorf("error setting reserved-volume-attachments: %v", err) + } + + if o.Endpoint != "custom-endpoint" { + t.Errorf("unexpected Endpoint: got %s, want custom-endpoint", o.Endpoint) + } + if o.HttpEndpoint != ":8080" { + t.Errorf("unexpected HttpEndpoint: got %s, want :8080", o.HttpEndpoint) + } + if !o.EnableOtelTracing { + t.Error("unexpected EnableOtelTracing: got false, want true") + } + if len(o.ExtraTags) != 2 || o.ExtraTags["key1"] != "value1" || o.ExtraTags["key2"] != "value2" { + t.Errorf("unexpected ExtraTags: got %v, want map[key1:value1 key2:value2]", o.ExtraTags) + } + if o.KubernetesClusterID != "cluster-123" { + t.Errorf("unexpected KubernetesClusterID: got %s, want cluster-123", o.KubernetesClusterID) + } + if !o.AwsSdkDebugLog { + t.Error("unexpected AwsSdkDebugLog: got false, want true") + } + if !o.WarnOnInvalidTag { + t.Error("unexpected WarnOnInvalidTag: got false, want true") + } + if o.UserAgentExtra != "extra-info" { + t.Errorf("unexpected UserAgentExtra: got %s, want extra-info", o.UserAgentExtra) + } + if !o.Batching { + t.Error("unexpected Batching: got false, want true") + } + if o.ModifyVolumeRequestHandlerTimeout != time.Minute { + t.Errorf("unexpected ModifyVolumeRequestHandlerTimeout: got %v, want 1m", o.ModifyVolumeRequestHandlerTimeout) + } + if o.VolumeAttachLimit != 10 { + t.Errorf("unexpected VolumeAttachLimit: got %d, want 10", o.VolumeAttachLimit) + } + if o.ReservedVolumeAttachments != 5 { + t.Errorf("unexpected ReservedVolumeAttachments: got %d, want 5", o.ReservedVolumeAttachments) + } +} + +func TestValidate(t *testing.T) { + tests := []struct { + name string + volumeAttachLimit int64 + reservedAttachments int + expectedErr bool + errMsg string + }{ + { + name: "both options not set", + volumeAttachLimit: -1, + reservedAttachments: -1, + expectedErr: false, + }, + { + name: "volumeAttachLimit set", + volumeAttachLimit: 10, + reservedAttachments: -1, + expectedErr: false, + }, + { + name: "reservedVolumeAttachments set", + volumeAttachLimit: -1, + reservedAttachments: 2, + expectedErr: false, + }, + { + name: "both options set", + volumeAttachLimit: 10, + reservedAttachments: 2, + expectedErr: true, + errMsg: "only one of --volume-attach-limit and --reserved-volume-attachments may be specified", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &Options{ + VolumeAttachLimit: tt.volumeAttachLimit, + ReservedVolumeAttachments: tt.reservedAttachments, + } + + err := o.Validate() + if (err != nil) != tt.expectedErr { + t.Errorf("Options.Validate() error = %v, wantErr %v", err, tt.expectedErr) + } + + if err != nil && err.Error() != tt.errMsg { + t.Errorf("Options.Validate() error message = %v, wantErrMsg %v", err.Error(), tt.errMsg) + } + }) + } +} diff --git a/pkg/driver/request_coalescing_test.go b/pkg/driver/request_coalescing_test.go index f14fe441bf..f44c578eb1 100644 --- a/pkg/driver/request_coalescing_test.go +++ b/pkg/driver/request_coalescing_test.go @@ -34,9 +34,9 @@ import ( "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" ) -type modifyVolumeExecutor func(ctx context.Context, driver controllerService, name string, params map[string]string) error +type modifyVolumeExecutor func(ctx context.Context, driver ControllerService, name string, params map[string]string) error -func externalResizerModifyVolume(ctx context.Context, driver controllerService, name string, params map[string]string) error { +func externalResizerModifyVolume(ctx context.Context, driver ControllerService, name string, params map[string]string) error { _, err := driver.ControllerModifyVolume(ctx, &csi.ControllerModifyVolumeRequest{ VolumeId: name, MutableParameters: params, @@ -44,7 +44,7 @@ func externalResizerModifyVolume(ctx context.Context, driver controllerService, return err } -func modifierForK8sModifyVolume(ctx context.Context, driver controllerService, name string, params map[string]string) error { +func modifierForK8sModifyVolume(ctx context.Context, driver ControllerService, name string, params map[string]string) error { _, err := driver.ModifyVolumeProperties(ctx, &rpc.ModifyVolumePropertiesRequest{ Name: name, Parameters: params, @@ -121,11 +121,11 @@ func testBasicRequestCoalescingSuccess(t *testing.T, executor modifyVolumeExecut return newSize, nil }) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - modifyVolumeRequestHandlerTimeout: 2 * time.Second, + options: &Options{ + ModifyVolumeRequestHandlerTimeout: 2 * time.Second, }, modifyVolumeManager: newModifyVolumeManager(), } @@ -175,11 +175,11 @@ func testRequestFail(t *testing.T, executor modifyVolumeExecutor) { return 0, fmt.Errorf("ResizeOrModifyDisk failed") }) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - modifyVolumeRequestHandlerTimeout: 2 * time.Second, + options: &Options{ + ModifyVolumeRequestHandlerTimeout: 2 * time.Second, }, modifyVolumeManager: newModifyVolumeManager(), } @@ -243,11 +243,11 @@ func testPartialFail(t *testing.T, executor modifyVolumeExecutor) { return newSize, nil }) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - modifyVolumeRequestHandlerTimeout: 2 * time.Second, + options: &Options{ + ModifyVolumeRequestHandlerTimeout: 2 * time.Second, }, modifyVolumeManager: newModifyVolumeManager(), } @@ -324,11 +324,11 @@ func testSequentialRequests(t *testing.T, executor modifyVolumeExecutor) { return newSize, nil }).Times(2) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - modifyVolumeRequestHandlerTimeout: 2 * time.Second, + options: &Options{ + ModifyVolumeRequestHandlerTimeout: 2 * time.Second, }, modifyVolumeManager: newModifyVolumeManager(), } @@ -381,11 +381,11 @@ func testDuplicateRequest(t *testing.T, executor modifyVolumeExecutor) { return newSize, nil }) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - modifyVolumeRequestHandlerTimeout: 2 * time.Second, + options: &Options{ + ModifyVolumeRequestHandlerTimeout: 2 * time.Second, }, modifyVolumeManager: newModifyVolumeManager(), } @@ -445,11 +445,11 @@ func testContextTimeout(t *testing.T, executor modifyVolumeExecutor) { return newSize, nil }) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - modifyVolumeRequestHandlerTimeout: 2 * time.Second, + options: &Options{ + ModifyVolumeRequestHandlerTimeout: 2 * time.Second, }, modifyVolumeManager: newModifyVolumeManager(), } @@ -510,11 +510,11 @@ func testResponseReturnTiming(t *testing.T, executor modifyVolumeExecutor) { return newSize, nil }) - awsDriver := controllerService{ + awsDriver := ControllerService{ cloud: mockCloud, inFlight: internal.NewInFlight(), - driverOptions: &DriverOptions{ - modifyVolumeRequestHandlerTimeout: 2 * time.Second, + options: &Options{ + ModifyVolumeRequestHandlerTimeout: 2 * time.Second, }, modifyVolumeManager: newModifyVolumeManager(), } diff --git a/pkg/driver/validation.go b/pkg/driver/validation.go index 7cce5a0e6e..99a08be67a 100644 --- a/pkg/driver/validation.go +++ b/pkg/driver/validation.go @@ -26,16 +26,16 @@ import ( "k8s.io/klog/v2" ) -func ValidateDriverOptions(options *DriverOptions) error { - if err := validateExtraTags(options.extraTags, false); err != nil { +func ValidateDriverOptions(options *Options) error { + if err := validateExtraTags(options.ExtraTags, false); err != nil { return fmt.Errorf("Invalid extra tags: %w", err) } - if err := validateMode(options.mode); err != nil { + if err := validateMode(options.Mode); err != nil { return fmt.Errorf("Invalid mode: %w", err) } - if options.modifyVolumeRequestHandlerTimeout == 0 { + if options.ModifyVolumeRequestHandlerTimeout == 0 && (options.Mode == ControllerMode || options.Mode == AllMode) { return errors.New("Invalid modifyVolumeRequestHandlerTimeout: Timeout cannot be zero") } diff --git a/pkg/driver/validation_test.go b/pkg/driver/validation_test.go index 4730a42ec8..1dfb685c96 100644 --- a/pkg/driver/validation_test.go +++ b/pkg/driver/validation_test.go @@ -195,10 +195,10 @@ func TestValidateDriverOptions(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - err := ValidateDriverOptions(&DriverOptions{ - extraTags: tc.extraVolumeTags, - mode: tc.mode, - modifyVolumeRequestHandlerTimeout: tc.modifyVolumeTimeout, + err := ValidateDriverOptions(&Options{ + ExtraTags: tc.extraVolumeTags, + Mode: tc.mode, + ModifyVolumeRequestHandlerTimeout: tc.modifyVolumeTimeout, }) if !reflect.DeepEqual(err, tc.expErr) { t.Fatalf("error not equal\ngot:\n%s\nexpected:\n%s", err, tc.expErr) diff --git a/tests/sanity/sanity_test.go b/tests/sanity/sanity_test.go deleted file mode 100644 index 82e9785ec6..0000000000 --- a/tests/sanity/sanity_test.go +++ /dev/null @@ -1,306 +0,0 @@ -//go:build linux -// +build linux - -package sanity - -import ( - "context" - "fmt" - "math/rand" - "os" - "path" - "strconv" - "testing" - "time" - - "github.com/golang/mock/gomock" - csisanity "github.com/kubernetes-csi/csi-test/v4/pkg/sanity" - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/cloud" - d "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/driver" - "github.com/kubernetes-sigs/aws-ebs-csi-driver/pkg/util" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" -) - -const ( - instanceId = "i-1234567890abcdef0" - region = "us-west-2" -) - -var ( - disks = make(map[string]*cloud.Disk) - snapshots = make(map[string]*cloud.Snapshot) - snapshotNameToID = make(map[string]string) - - fakeMetaData = &cloud.Metadata{ - InstanceID: instanceId, - Region: region, - } -) - -func TestSanity(t *testing.T) { - defer func() { - if r := recover(); r != nil { - t.Errorf("Test panicked: %v", r) - } - }() - - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - - tmpDir, err := os.MkdirTemp("", "csi-sanity-") - if err != nil { - t.Fatalf("Failed to create sanity temp working dir: %v", err) - } - defer os.RemoveAll(tmpDir) - - defer func() { - if err = os.RemoveAll(tmpDir); err != nil { - t.Fatalf("Failed to clean up sanity temp working dir %s: %v", tmpDir, err.Error()) - } - }() - - endpoint := fmt.Sprintf("unix:%s/csi.sock", tmpDir) - mountPath := path.Join(tmpDir, "mount") - stagePath := path.Join(tmpDir, "stage") - - fakeMounter, fakeIdentifier, fakeResizeFs, fakeCloud := createMockObjects(mockCtrl) - - mockNodeService(fakeMounter, fakeIdentifier, fakeResizeFs, mountPath) - mockControllerService(fakeCloud, mountPath) - - drv, err := d.NewFakeDriver(endpoint, fakeCloud, fakeMetaData, fakeMounter) - if err != nil { - t.Fatalf("Failed to create fake driver: %v", err.Error()) - } - go func() { - if err := drv.Run(); err != nil { - panic(fmt.Sprintf("%v", err)) - } - }() - - config := csisanity.TestConfig{ - TargetPath: mountPath, - StagingPath: stagePath, - Address: endpoint, - DialOptions: []grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())}, - IDGen: csisanity.DefaultIDGenerator{}, - TestVolumeSize: 10 * util.GiB, - TestVolumeAccessType: "mount", - } - csisanity.Test(t, config) -} - -func createMockObjects(mockCtrl *gomock.Controller) (*d.MockMounter, *d.MockDeviceIdentifier, *d.MockResizefs, *cloud.MockCloud) { - fakeMounter := d.NewMockMounter(mockCtrl) - fakeIdentifier := d.NewMockDeviceIdentifier(mockCtrl) - fakeResizeFs := d.NewMockResizefs(mockCtrl) - fakeCloud := cloud.NewMockCloud(mockCtrl) - - return fakeMounter, fakeIdentifier, fakeResizeFs, fakeCloud -} - -func mockNodeService(m *d.MockMounter, i *d.MockDeviceIdentifier, r *d.MockResizefs, mountPath string) { - m.EXPECT().Unpublish(gomock.Any()).Return(nil).AnyTimes() - m.EXPECT().GetDeviceNameFromMount(gomock.Any()).Return("", 0, nil).AnyTimes() - m.EXPECT().Unstage(gomock.Any()).Return(nil).AnyTimes() - m.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - m.EXPECT().NeedResize(gomock.Any(), gomock.Any()).Return(false, nil).AnyTimes() - m.EXPECT().MakeDir(gomock.Any()).Return(nil).AnyTimes() - m.EXPECT().IsLikelyNotMountPoint(gomock.Any()).Return(true, nil).AnyTimes() - m.EXPECT().Mount(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - - m.EXPECT().PathExists(gomock.Any()).DoAndReturn( - func(targetPath string) (bool, error) { - if targetPath == mountPath { - return true, nil - } - return false, nil - }, - ).AnyTimes() - - m.EXPECT().NewResizeFs().Return(r, nil).AnyTimes() - i.EXPECT().Lstat(gomock.Any()).Return(nil, nil).AnyTimes() - r.EXPECT().Resize(gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes() -} - -func mockControllerService(c *cloud.MockCloud, mountPath string) { - // CreateDisk - c.EXPECT().CreateDisk(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, volumeID string, diskOptions *cloud.DiskOptions) (*cloud.Disk, error) { - for _, existingDisk := range disks { - if existingDisk.VolumeID == volumeID && existingDisk.CapacityGiB != util.BytesToGiB(diskOptions.CapacityBytes) { - return nil, cloud.ErrAlreadyExists - } - } - - if diskOptions.SnapshotID != "" { - if _, exists := snapshots[diskOptions.SnapshotID]; !exists { - return nil, cloud.ErrNotFound - } - } - - newDisk := &cloud.Disk{ - VolumeID: volumeID, - AvailabilityZone: diskOptions.AvailabilityZone, - CapacityGiB: util.BytesToGiB(diskOptions.CapacityBytes), - } - disks[volumeID] = newDisk - return newDisk, nil - }, - ).AnyTimes() - - // DeleteDisk - c.EXPECT().DeleteDisk(gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, volumeID string) (bool, error) { - _, exists := disks[volumeID] - if !exists { - return false, cloud.ErrNotFound - } - delete(disks, volumeID) - return true, nil - }, - ).AnyTimes() - - // GetDiskByID - c.EXPECT().GetDiskByID(gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, volumeID string) (*cloud.Disk, error) { - disk, exists := disks[volumeID] - if !exists { - return nil, cloud.ErrNotFound - } - return disk, nil - }, - ).AnyTimes() - - // CreateSnapshot - c.EXPECT(). - CreateSnapshot(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, volumeID string, opts *cloud.SnapshotOptions) (*cloud.Snapshot, error) { - snapshotID := fmt.Sprintf("snapshot-%d", rand.New(rand.NewSource(time.Now().UnixNano())).Uint64()) - - _, exists := snapshots[snapshotID] - if exists { - return nil, cloud.ErrAlreadyExists - } - newSnapshot := &cloud.Snapshot{ - SnapshotID: snapshotID, - SourceVolumeID: volumeID, - CreationTime: time.Now(), - } - snapshots[snapshotID] = newSnapshot - snapshotNameToID[opts.Tags["CSIVolumeSnapshotName"]] = snapshotID - return newSnapshot, nil - }).AnyTimes() - - // DeleteSnapshot - c.EXPECT().DeleteSnapshot(gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, snapshotID string) (bool, error) { - if _, exists := snapshots[snapshotID]; !exists { - return false, cloud.ErrNotFound - } - for name, id := range snapshotNameToID { - if id == snapshotID { - delete(snapshotNameToID, name) - break - } - } - delete(snapshots, snapshotID) - return true, nil - }, - ).AnyTimes() - - // GetSnapshotByID - c.EXPECT().GetSnapshotByID(gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, snapshotID string) (*cloud.Snapshot, error) { - snapshot, exists := snapshots[snapshotID] - if !exists { - return nil, cloud.ErrNotFound - } - return snapshot, nil - }, - ).AnyTimes() - - // GetSnapshotByName - c.EXPECT().GetSnapshotByName(gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, name string) (*cloud.Snapshot, error) { - if snapshotID, exists := snapshotNameToID[name]; exists { - return snapshots[snapshotID], nil - } - return nil, cloud.ErrNotFound - }, - ).AnyTimes() - - // ListSnapshots - c.EXPECT(). - ListSnapshots(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). - DoAndReturn(func(ctx context.Context, sourceVolumeID string, maxResults int32, nextToken string) (*cloud.ListSnapshotsResponse, error) { - var s []*cloud.Snapshot - startIndex := 0 - var err error - - if nextToken != "" { - startIndex, err = strconv.Atoi(nextToken) - if err != nil { - return nil, fmt.Errorf("invalid next token %s", nextToken) - } - } - - var nextTokenStr string - count := 0 - for _, snap := range snapshots { - if snap.SourceVolumeID == sourceVolumeID || sourceVolumeID == "" { - if startIndex <= count { - s = append(s, snap) - if maxResults > 0 && int32(len(s)) >= maxResults { - nextTokenStr = strconv.Itoa(startIndex + int(maxResults)) - break - } - } - count++ - } - } - - return &cloud.ListSnapshotsResponse{ - Snapshots: s, - NextToken: nextTokenStr, - }, nil - }). - AnyTimes() - - // AttachDisk - c.EXPECT().AttachDisk(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, volumeID string, instanceID string) (string, error) { - _, diskExists := disks[volumeID] - if !diskExists || instanceID != fakeMetaData.InstanceID { - return "", cloud.ErrNotFound - } - return mountPath, nil - }, - ).AnyTimes() - - // DetachDisk - c.EXPECT().DetachDisk(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, volumeID string, instanceID string) (bool, error) { - _, diskExists := disks[volumeID] - if !diskExists || instanceID != fakeMetaData.InstanceID { - return false, cloud.ErrNotFound - } - return true, nil - }, - ).AnyTimes() - - // ResizeOrModifyDisk - c.EXPECT().ResizeOrModifyDisk(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( - func(ctx context.Context, volumeID string, newSizeBytes int64, modifyOptions *cloud.ModifyDiskOptions) (int32, error) { - disk, exists := disks[volumeID] - if !exists { - return 0, cloud.ErrNotFound - } - newSizeGiB := util.BytesToGiB(newSizeBytes) - disk.CapacityGiB = newSizeGiB - disks[volumeID] = disk - return newSizeGiB, nil - }, - ).AnyTimes() -}