gh comments

hack/code/prices_gen.go

@@ -34,7 +34,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
-	awsoptions "github.com/aws/karpenter/pkg/operator/options"
+	"github.com/aws/karpenter/pkg/operator/options"
 	"github.com/aws/karpenter/pkg/providers/pricing"
 	"github.com/aws/karpenter/pkg/test"
 )
@@ -65,13 +65,13 @@ func getPartitionSuffix(partition string) string {
 	}
 }
 
-type Options struct {
+type CLIOptions struct {
 	partition string
 	output    string
 }
 
-func NewOptions() *Options {
-	o := &Options{}
+func NewCLIOptions() *CLIOptions {
+	o := &CLIOptions{}
 	flag.StringVar(&o.partition, "partition", "aws", "The partition to generate prices for. Valid options are \"aws\", \"aws-us-gov\", and \"aws-cn\".")
 	flag.StringVar(&o.output, "output", "pkg/providers/pricing/zz_generated.pricing_aws.go", "The destination for the generated go file.")
 	flag.Parse()
@@ -82,7 +82,7 @@ func NewOptions() *Options {
 }
 
 func main() {
-	opts := NewOptions()
+	opts := NewCLIOptions()
 	f, err := os.Create("pricing.heapprofile")
 	if err != nil {
 		log.Fatal("could not create memory profile: ", err)
@@ -93,7 +93,7 @@ func main() {
 	os.Setenv("AWS_SDK_LOAD_CONFIG", "true")
 	os.Setenv("AWS_REGION", region)
 	ctx := context.Background()
-	ctx = awsoptions.ToContext(ctx, test.Options())
+	ctx = options.ToContext(ctx, test.Options())
 	sess := session.Must(session.NewSession())
 	ec2 := ec22.New(sess)
 	src := &bytes.Buffer{}

hack/code/vpc_limits_gen.go

@@ -26,13 +26,13 @@ import (
 	"time"
 )
 
-type options struct {
+type Options struct {
 	sourceOutput string
 	urlInput     string
 }
 
 func main() {
-	opts := options{}
+	opts := Options{}
 	flag.StringVar(&opts.urlInput, "url", "https://raw.githubusercontent.com/aws/amazon-vpc-resource-controller-k8s/master/pkg/aws/vpc/limits.go",
 		"url of the raw vpc/limits.go file in the github.com/aws/amazon-vpc-resource-controller-k8s repo")
 	flag.StringVar(&opts.sourceOutput, "output", "pkg/providers/instancetype/zz_generated.vpclimits.go", "output location for the generated go source file")

hack/docs/instancetypes_gen_docs.go

@@ -42,7 +42,7 @@ import (
 	nodepoolutil "github.com/aws/karpenter-core/pkg/utils/nodepool"
 	awscloudprovider "github.com/aws/karpenter/pkg/cloudprovider"
 	"github.com/aws/karpenter/pkg/operator"
-	awsoptions "github.com/aws/karpenter/pkg/operator/options"
+	"github.com/aws/karpenter/pkg/operator/options"
 	"github.com/aws/karpenter/pkg/test"
 
 	"github.com/aws/karpenter-core/pkg/cloudprovider"
@@ -78,7 +78,7 @@ func main() {
 	lo.Must0(os.Setenv("AWS_REGION", "us-east-1"))
 
 	ctx := coreoptions.ToContext(context.Background(), coretest.Options())
-	ctx = awsoptions.ToContext(ctx, test.Options(test.OptionsFields{
+	ctx = options.ToContext(ctx, test.Options(test.OptionsFields{
 		ClusterName:     lo.ToPtr("docs-gen"),
 		ClusterEndpoint: lo.ToPtr("https://docs-gen.aws"),
 		IsolatedVPC:     lo.ToPtr(true), // disable pricing lookup

pkg/apis/settings/settings.go

@@ -70,6 +70,9 @@ func (*Settings) ConfigMap() string {
 // Inject creates a Settings from the supplied ConfigMap
 func (*Settings) Inject(ctx context.Context, cm *v1.ConfigMap) (context.Context, error) {
 	s := defaultSettings.DeepCopy()
+	if cm == nil {
+		return ToContext(ctx, s), nil
+	}
 
 	if err := configmap.Parse(cm.Data,
 		configmap.AsString("aws.assumeRoleARN", &s.AssumeRoleARN),
@@ -105,7 +108,7 @@ func ToContext(ctx context.Context, s *Settings) context.Context {
 func FromContext(ctx context.Context) *Settings {
 	data := ctx.Value(ContextKey)
 	if data == nil {
-		return nil
+		panic("settings not in context")
 	}
 	return data.(*Settings)
 }

pkg/operator/options/options.go

@@ -99,32 +99,21 @@ func (o *Options) MergeSettings(ctx context.Context) {
 	if s == nil {
 		return
 	}
-	if !o.setFlags["assume-role-arn"] {
-		o.AssumeRoleARN = s.AssumeRoleARN
-	}
-	if !o.setFlags["assume-role-duration"] {
-		o.AssumeRoleDuration = s.AssumeRoleDuration
-	}
-	if !o.setFlags["cluster-ca-bundle"] {
-		o.ClusterCABundle = s.ClusterCABundle
-	}
-	if !o.setFlags["cluster-name"] {
-		o.ClusterName = s.ClusterName
-	}
-	if !o.setFlags["cluster-endpoint"] {
-		o.ClusterEndpoint = s.ClusterEndpoint
-	}
-	if !o.setFlags["isolated-vpc"] {
-		o.IsolatedVPC = s.IsolatedVPC
-	}
-	if !o.setFlags["vm-memory-overhead-percent"] {
-		o.VMMemoryOverheadPercent = s.VMMemoryOverheadPercent
-	}
-	if !o.setFlags["interruption-queue-name"] {
-		o.InterruptionQueueName = s.InterruptionQueueName
-	}
-	if !o.setFlags["reserved-enis"] {
-		o.ReservedENIs = s.ReservedENIs
+
+	mergeField(&o.AssumeRoleARN, s.AssumeRoleARN, o.setFlags["assume-role-arn"])
+	mergeField(&o.AssumeRoleDuration, s.AssumeRoleDuration, o.setFlags["assume-role-duration"])
+	mergeField(&o.ClusterCABundle, s.ClusterCABundle, o.setFlags["cluster-ca-bundle"])
+	mergeField(&o.ClusterName, s.ClusterName, o.setFlags["cluster-name"])
+	mergeField(&o.ClusterEndpoint, s.ClusterEndpoint, o.setFlags["cluster-endpoint"])
+	mergeField(&o.IsolatedVPC, s.IsolatedVPC, o.setFlags["isolated-vpc"])
+	mergeField(&o.VMMemoryOverheadPercent, s.VMMemoryOverheadPercent, o.setFlags["vm-memory-overhead-percent"])
+	mergeField(&o.InterruptionQueueName, s.InterruptionQueueName, o.setFlags["interruption-queue-name"])
+	mergeField(&o.ReservedENIs, s.ReservedENIs, o.setFlags["reserved-enis"])
+
+	// Temporarily revalidate ClusterName, since default value is invalid
+	o.setFlags["cluster-name"] = true
+	if err := o.validateClusterName(); err != nil {
+		panic(fmt.Errorf("validating cluster name, %w", err))
 	}
 }
 
@@ -139,3 +128,10 @@ func FromContext(ctx context.Context) *Options {
 	}
 	return retval.(*Options)
 }
+
+// Note: Separated out to help with cyclomatic complexity check
+func mergeField[T any](dest *T, src T, isDestSet bool) {
+	if !isDestSet {
+		*dest = src
+	}
+}

pkg/operator/options/options_validation.go

@@ -32,12 +32,8 @@ func (o Options) Validate() (errs *apis.FieldError) {
 }
 
 func (o Options) validateAssumeRoleDuration() (errs *apis.FieldError) {
-	// TODO: Remove with karpenter-global-settings
-	if !o.setFlags["aws-assume-role-arn"] {
-		return nil
-	}
 	if o.AssumeRoleDuration < time.Minute*15 {
-		return errs.Also(apis.ErrInvalidValue("assumeRoleDuration cannot be less than 15 Minutes", "assumeRoleDuration"))
+		return errs.Also(apis.ErrInvalidValue("assume-role-duration cannot be less than 15 Minutes", "assume-role-duration"))
 	}
 	return nil
 }
@@ -48,29 +44,22 @@ func (o Options) validateClusterName() (errs *apis.FieldError) {
 		return nil
 	}
 	if o.ClusterName == "" {
-		return errs.Also(apis.ErrMissingField("clusterName is required", "clusterName"))
+		return errs.Also(apis.ErrMissingField("cluster-name is required", "cluster-name"))
 	}
 	return nil
 }
 
 func (o Options) validateEndpoint() (errs *apis.FieldError) {
-	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
 	if err != nil || !endpoint.IsAbs() || endpoint.Hostname() == "" {
-		return errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%q not a valid clusterEndpoint URL", o.ClusterEndpoint), "clusterEndpoint"))
+		return errs.Also(apis.ErrInvalidValue(fmt.Sprintf("%q not a valid cluster-endpoint URL", o.ClusterEndpoint), "cluster-endpoint"))
 	}
 	return nil
 }
 
 func (o Options) validateVMMemoryOverheadPercent() (errs *apis.FieldError) {
-	// TODO: Remove with karpenter-global-settings
-	if !o.setFlags["aws-vm-memory-overhead-percent"] {
-		return nil
-	}
 	if o.VMMemoryOverheadPercent < 0 {
 		return errs.Also(apis.ErrInvalidValue("cannot be negative", "vmMemoryOverheadPercent"))
 	}

pkg/providers/instancetype/nodeclass_test.go

@@ -647,7 +647,7 @@ var _ = Describe("NodeClass/InstanceTypes", func() {
 		var info *ec2.InstanceTypeInfo
 		BeforeEach(func() {
 			ctx = options.ToContext(ctx, test.Options(test.OptionsFields{
-				ClusterName: lo.ToPtr("CLUSTER_NAME"),
+				ClusterName: lo.ToPtr("karpenter-cluster"),
 			}))
 
 			var ok bool

test/pkg/environment/aws/environment.go

@@ -15,6 +15,7 @@ limitations under the License.
 package aws
 
 import (
+	"os"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -90,3 +91,15 @@ func GetTimeStreamAPI(session *session.Session) timestreamwriteiface.TimestreamW
 	}
 	return &NoOpTimeStreamAPI{}
 }
+
+func GetClusterName() string {
+	return lo.Must(os.LookupEnv("CLUSTER_NAME"))
+}
+
+func GetClusterEndpoint() string {
+	return lo.Must(os.LookupEnv("CLUSTER_ENDPOINT"))
+}
+
+func GetInterruptionQueueName() string {
+	return lo.Must(os.LookupEnv("INTERRUPTION_QUEUE_NAME"))
+}

test/suites/chaos/suite_test.go

@@ -17,7 +17,6 @@ package chaos_test
 import (
 	"context"
 	"fmt"
-	"os"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -42,6 +41,7 @@ import (
 	"github.com/aws/karpenter/pkg/apis/v1alpha1"
 	awstest "github.com/aws/karpenter/pkg/test"
 	"github.com/aws/karpenter/test/pkg/debug"
+	"github.com/aws/karpenter/test/pkg/environment/aws"
 	"github.com/aws/karpenter/test/pkg/environment/common"
 )
 
@@ -69,8 +69,8 @@ var _ = Describe("Chaos", func() {
 			defer cancel()
 
 			provider := awstest.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{
-				SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
-				SubnetSelector:        map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
+				SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": aws.GetClusterName()},
+				SubnetSelector:        map[string]string{"karpenter.sh/discovery": aws.GetClusterName()},
 			}})
 			provisioner := test.Provisioner(test.ProvisionerOptions{
 				Requirements: []v1.NodeSelectorRequirement{
@@ -114,8 +114,8 @@ var _ = Describe("Chaos", func() {
 			defer cancel()
 
 			provider := awstest.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{
-				SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
-				SubnetSelector:        map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
+				SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": aws.GetClusterName()},
+				SubnetSelector:        map[string]string{"karpenter.sh/discovery": aws.GetClusterName()},
 			}})
 			provisioner := test.Provisioner(test.ProvisionerOptions{
 				Requirements: []v1.NodeSelectorRequirement{

test/suites/consolidation/suite_test.go

@@ -16,13 +16,11 @@ package consolidation_test
 
 import (
 	"fmt"
-	"os"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/samber/lo"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -61,8 +59,8 @@ var _ = AfterEach(func() { env.AfterEach() })
 var _ = Describe("Consolidation", func() {
 	It("should consolidate nodes (delete)", Label(debug.NoWatch), Label(debug.NoEvents), func() {
 		provider := awstest.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{
-			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
-			SubnetSelector:        map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
+			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": environmentaws.GetClusterName()},
+			SubnetSelector:        map[string]string{"karpenter.sh/discovery": environmentaws.GetClusterName()},
 		}})
 		provisioner := test.Provisioner(test.ProvisionerOptions{
 			Requirements: []v1.NodeSelectorRequirement{
@@ -127,8 +125,8 @@ var _ = Describe("Consolidation", func() {
 	})
 	It("should consolidate on-demand nodes (replace)", func() {
 		provider := awstest.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{
-			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
-			SubnetSelector:        map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
+			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": environmentaws.GetClusterName()},
+			SubnetSelector:        map[string]string{"karpenter.sh/discovery": environmentaws.GetClusterName()},
 		}})
 		provisioner := test.Provisioner(test.ProvisionerOptions{
 			Requirements: []v1.NodeSelectorRequirement{
@@ -243,8 +241,8 @@ var _ = Describe("Consolidation", func() {
 	})
 	It("should consolidate on-demand nodes to spot (replace)", func() {
 		provider := awstest.AWSNodeTemplate(v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{
-			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
-			SubnetSelector:        map[string]string{"karpenter.sh/discovery": lo.Must(os.LookupEnv("CLUSTER_NAME"))},
+			SecurityGroupSelector: map[string]string{"karpenter.sh/discovery": environmentaws.GetClusterName()},
+			SubnetSelector:        map[string]string{"karpenter.sh/discovery": environmentaws.GetClusterName()},
 		}})
 		provisioner := test.Provisioner(test.ProvisionerOptions{
 			Requirements: []v1.NodeSelectorRequirement{