diff --git a/pkg/operator/options/options.go b/pkg/operator/options/options.go index 69f19208e09a..960fa080e4d6 100644 --- a/pkg/operator/options/options.go +++ b/pkg/operator/options/options.go @@ -47,7 +47,8 @@ type Options struct { InterruptionQueueName string ReservedENIs int - setFlags map[string]bool + isolatedVPC string + setFlags map[string]bool } func (o *Options) AddFlags(fs *flag.FlagSet) { @@ -56,7 +57,7 @@ func (o *Options) AddFlags(fs *flag.FlagSet) { fs.StringVar(&o.ClusterCABundle, "cluster-ca-bundle", env.WithDefaultString("CLUSTER_CA_BUNDLE", ""), "Cluster CA bundle for nodes to use for TLS connections with the API server. If not set, this is taken from the controller's TLS configuration.") fs.StringVar(&o.ClusterName, "cluster-name", env.WithDefaultString("CLUSTER_NAME", ""), "[REQUIRED] The kubernetes cluster name for resource discovery.") fs.StringVar(&o.ClusterEndpoint, "cluster-endpoint", env.WithDefaultString("CLUSTER_ENDPOINT", ""), "The external kubernetes cluster endpoint for new nodes to connect with. If not specified, will discover the cluster endpoint using DescribeCluster API.") - fs.BoolVar(&o.IsolatedVPC, "isolated-vpc", env.WithDefaultBool("ISOLATED_VPC", false), "If true, then assume we can't reach AWS services which don't have a VPC endpoint. This also has the effect of disabling look-ups to the AWS pricing endpoint.") + fs.StringVar(&o.isolatedVPC, "isolated-vpc", env.WithDefaultString("ISOLATED_VPC", "false"), "If true, then assume we can't reach AWS services which don't have a VPC endpoint. This also has the effect of disabling look-ups to the AWS pricing endpoint.") fs.Float64Var(&o.VMMemoryOverheadPercent, "vm-memory-overhead-percent", env.WithDefaultFloat64("VM_MEMORY_OVERHEAD_PERCENT", 0.075), "The VM memory overhead as a percent that will be subtracted from the total memory for all instance types.") fs.StringVar(&o.InterruptionQueueName, "interruption-queue-name", env.WithDefaultString("INTERRUPTION_QUEUE_NAME", ""), "Interruption queue is disabled if not specified. Enabling interruption handling may require additional permissions on the controller service account. Additional permissions are outlined in the docs.") fs.IntVar(&o.ReservedENIs, "reserved-enis", env.WithDefaultInt("RESERVED_ENIS", 0), "Reserved ENIs are not included in the calculations for max-pods or kube-reserved. This is most often used in the VPC CNI custom networking setup https://docs.aws.amazon.com/eks/latest/userguide/cni-custom-network.html.") @@ -87,6 +88,9 @@ func (o *Options) Parse(fs *flag.FlagSet, args ...string) error { return fmt.Errorf("validating options, %w", err) } + // Parse options which can't be directly parsed by flag + o.IsolatedVPC = (o.isolatedVPC == "true") + return nil } diff --git a/pkg/operator/options/options_validation.go b/pkg/operator/options/options_validation.go index 7b70e7606493..d947aa752444 100644 --- a/pkg/operator/options/options_validation.go +++ b/pkg/operator/options/options_validation.go @@ -27,6 +27,8 @@ func (o Options) Validate() error { o.validateEndpoint(), o.validateVMMemoryOverheadPercent(), o.validateAssumeRoleDuration(), + o.validateReservedENIs(), + o.validateIsolateVPC(), ) } @@ -37,7 +39,17 @@ func (o Options) validateAssumeRoleDuration() error { return nil } +func (o Options) validateIsolateVPC() error { + if o.isolatedVPC != "true" && o.isolatedVPC != "false" { + return fmt.Errorf("%q is not a valid value for isolate-vpc, options are true or false", o.isolatedVPC) + } + return nil +} + func (o Options) validateEndpoint() error { + if o.ClusterEndpoint == "" { + return nil + } endpoint, err := url.Parse(o.ClusterEndpoint) // url.Parse() will accept a lot of input without error; make // sure it's a real URL @@ -54,6 +66,13 @@ func (o Options) validateVMMemoryOverheadPercent() error { return nil } +func (o Options) validateReservedENIs() error { + if o.ReservedENIs < 0 { + return fmt.Errorf("reserved-enis may not be negative") + } + return nil +} + // Note: add back to Validate when karpenter-global-settings (and merge logic) are completely removed func (o Options) validateRequiredFields() error { if o.ClusterName == "" { diff --git a/pkg/operator/options/suite_test.go b/pkg/operator/options/suite_test.go new file mode 100644 index 000000000000..b69131348cc0 --- /dev/null +++ b/pkg/operator/options/suite_test.go @@ -0,0 +1,118 @@ +/* +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_test + +import ( + "context" + "flag" + "fmt" + "testing" + "time" + + "github.com/aws/karpenter/pkg/apis/settings" + "github.com/aws/karpenter/pkg/operator/options" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "knative.dev/pkg/logging/testing" +) + +var ctx context.Context + +func TestAPIs(t *testing.T) { + ctx = TestContextWithLogger(t) + RegisterFailHandler(Fail) + RunSpecs(t, "Options") +} + +var _ = Describe("Options", func() { + var fs *flag.FlagSet + var opts *options.Options + BeforeEach(func() { + fs = flag.NewFlagSet("karpenter", flag.ContinueOnError) + opts = &options.Options{} + opts.AddFlags(fs) + + // Inject default settings + var err error + ctx, err = (&settings.Settings{}).Inject(ctx, nil) + Expect(err).To(BeNil()) + }) + + Context("Merging", func() { + It("shouldn't overwrite cli flags / env vars", func() { + err := opts.Parse( + fs, + "--assume-role-arn", "options-cluster-role", + "--cluster-ca-bundle", "options-bundle", + "--cluster-name", "options-cluster", + "--cluster-endpoint", "https://options-cluster", + "--isolated-vpc", "false", + "--interruption-queue-name", "options-cluster", + ) + Expect(err).ToNot(HaveOccurred()) + ctx = settings.ToContext(ctx, &settings.Settings{ + AssumeRoleARN: "settings-cluster-role", + AssumeRoleDuration: time.Minute * 20, + ClusterCABundle: "settings-bundle", + ClusterName: "settings-cluster", + ClusterEndpoint: "https://settings-cluster", + IsolatedVPC: true, + VMMemoryOverheadPercent: 0.1, + InterruptionQueueName: "settings-cluster", + ReservedENIs: 10, + }) + opts.MergeSettings(ctx) + Expect(opts.AssumeRoleARN).To(Equal("options-cluster-role")) + Expect(opts.AssumeRoleDuration).To(Equal(time.Minute * 20)) + Expect(opts.ClusterCABundle).To(Equal("options-bundle")) + Expect(opts.ClusterName).To(Equal("options-cluster")) + Expect(opts.ClusterEndpoint).To(Equal("https://options-cluster")) + Expect(opts.IsolatedVPC).To(BeFalse()) + Expect(opts.VMMemoryOverheadPercent).To(Equal(0.1)) + Expect(opts.InterruptionQueueName).To(Equal("options-cluster")) + Expect(opts.ReservedENIs).To(Equal(10)) + }) + + }) + + Context("Validation", func() { + It("should fail when cluster name is not set", func() { + err := opts.Parse(fs) + // Overwrite ClusterName since it is commonly set by environment variables in dev environments + opts.ClusterName = "" + Expect(err).ToNot(HaveOccurred()) + Expect(func() { + opts.MergeSettings(ctx) + fmt.Printf("%#v", opts) + }).To(Panic()) + }) + It("should fail when assume role duration is less than 15 minutes", func() { + err := opts.Parse(fs, "--assume-role-duration", "1s") + Expect(err).To(HaveOccurred()) + }) + It("should fail when clusterEndpoint is invalid (not absolute)", func() { + err := opts.Parse(fs, "--cluster-endpoint", "00000000000000000000000.gr7.us-west-2.eks.amazonaws.com") + Expect(err).To(HaveOccurred()) + }) + It("should fail when vmMemoryOverheadPercent is negative", func() { + err := opts.Parse(fs, "--vm-memory-overhead-percent", "-0.01") + Expect(err).To(HaveOccurred()) + }) + It("should fail when reservedENIs is negative", func() { + err := opts.Parse(fs, "--reserved-enis", "-1") + Expect(err).To(HaveOccurred()) + }) + }) +})