Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: karpenter-global-settings migration #4789

Merged
merged 4 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
4 changes: 2 additions & 2 deletions hack/code/prices_gen.go → hack/code/prices_gen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/aws/karpenter/pkg/apis/settings"
"github.com/aws/karpenter/pkg/operator/options"
"github.com/aws/karpenter/pkg/providers/pricing"
"github.com/aws/karpenter/pkg/test"
)
Expand Down Expand Up @@ -93,7 +93,7 @@ func main() {
os.Setenv("AWS_SDK_LOAD_CONFIG", "true")
os.Setenv("AWS_REGION", region)
ctx := context.Background()
ctx = settings.ToContext(ctx, test.Settings())
ctx = options.ToContext(ctx, test.Options())
sess := session.Must(session.NewSession())
ec2 := ec22.New(sess)
src := &bytes.Buffer{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
8 changes: 4 additions & 4 deletions hack/codegen.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ bandwidth() {
NO_UPDATE=''
SUBJECT="Bandwidth"

go run hack/code/bandwidth_gen.go -- "${GENERATED_FILE}"
go run hack/code/bandwidth_gen/main.go -- "${GENERATED_FILE}"

GIT_DIFF=$(git diff --stat "${GENERATED_FILE}")
checkForUpdates "${GIT_DIFF}" "${NO_UPDATE}" "${SUBJECT}" "${GENERATED_FILE}"
Expand All @@ -30,7 +30,7 @@ pricing() {
NO_UPDATE=" ${GENERATED_FILE} "$'| 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)'
SUBJECT="Pricing"

go run hack/code/prices_gen.go --partition "$partition" --output "$GENERATED_FILE"
go run hack/code/prices_gen/main.go --partition "$partition" --output "$GENERATED_FILE"

GIT_DIFF=$(git diff --stat "${GENERATED_FILE}")
checkForUpdates "${GIT_DIFF}" "${NO_UPDATE}" "${SUBJECT} beside timestamps since last update" "${GENERATED_FILE}"
Expand All @@ -42,7 +42,7 @@ vpcLimits() {
NO_UPDATE=''
SUBJECT="VPC Limits"

go run hack/code/vpc_limits_gen.go -- \
go run hack/code/vpc_limits_gen/main.go -- \
--url=https://raw.githubusercontent.com/aws/amazon-vpc-resource-controller-k8s/master/pkg/aws/vpc/limits.go \
--output="${GENERATED_FILE}"

Expand All @@ -55,7 +55,7 @@ instanceTypeTestData() {
NO_UPDATE=''
SUBJECT="Instance Type Test Data"

go run hack/code/instancetype_testdata_gen.go --out-file ${GENERATED_FILE} \
go run hack/code/instancetype_testdata_gen/main.go --out-file ${GENERATED_FILE} \
--instance-types t3.large,m5.large,m5.xlarge,p3.8xlarge,g4dn.8xlarge,c6g.large,inf1.2xlarge,inf1.6xlarge,trn1.2xlarge,m5.metal,dl1.24xlarge,m6idn.32xlarge,t4g.small,t4g.xlarge,t4g.medium

GIT_DIFF=$(git diff --stat "${GENERATED_FILE}")
Expand Down
8 changes: 6 additions & 2 deletions hack/docs/configuration_gen_docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import (
"os"
"strings"

"github.com/aws/karpenter-core/pkg/operator/options"
coreoptions "github.com/aws/karpenter-core/pkg/operator/options"
"github.com/aws/karpenter/pkg/operator/options"
)

func main() {
Expand All @@ -48,7 +49,10 @@ func main() {
topDoc := fmt.Sprintf("%s%s\n\n", startDocSections[0], genStart)
bottomDoc := fmt.Sprintf("\n%s%s", genEnd, endDocSections[1])

fs := &options.FlagSet{FlagSet: flag.NewFlagSet("karpenter", flag.ContinueOnError)}
fs := &coreoptions.FlagSet {
FlagSet: flag.NewFlagSet("karpenter", flag.ContinueOnError),
}
(&coreoptions.Options{}).AddFlags(fs)
(&options.Options{}).AddFlags(fs)

envVarsBlock := "| Environment Variable | CLI Flag | Description |\n"
Expand Down
8 changes: 4 additions & 4 deletions hack/docs/instancetypes_gen_docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/manager"

coresettings "github.com/aws/karpenter-core/pkg/apis/settings"
"github.com/aws/karpenter-core/pkg/apis/v1alpha5"
coreoperator "github.com/aws/karpenter-core/pkg/operator"
coreoptions "github.com/aws/karpenter-core/pkg/operator/options"
coretest "github.com/aws/karpenter-core/pkg/test"
nodepoolutil "github.com/aws/karpenter-core/pkg/utils/nodepool"
"github.com/aws/karpenter/pkg/apis/settings"
awscloudprovider "github.com/aws/karpenter/pkg/cloudprovider"
"github.com/aws/karpenter/pkg/operator"
"github.com/aws/karpenter/pkg/operator/options"
"github.com/aws/karpenter/pkg/test"

"github.com/aws/karpenter-core/pkg/cloudprovider"
Expand Down Expand Up @@ -87,8 +87,8 @@ func main() {
lo.Must0(os.Setenv("AWS_SDK_LOAD_CONFIG", "true"))
lo.Must0(os.Setenv("AWS_REGION", "us-east-1"))

ctx := coresettings.ToContext(context.Background(), coretest.Settings())
ctx = settings.ToContext(ctx, test.Settings(test.SettingOptions{
ctx := coreoptions.ToContext(context.Background(), coretest.Options())
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
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/settings/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (

v1 "k8s.io/api/core/v1"
"knative.dev/pkg/configmap"

coresettings "github.com/aws/karpenter-core/pkg/apis/settings"
)

type settingsKeyType struct{}
Expand Down Expand Up @@ -68,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),
Expand All @@ -92,6 +97,10 @@ func (*Settings) Inject(ctx context.Context, cm *v1.ConfigMap) (context.Context,
return ToContext(ctx, s), nil
}

func (*Settings) FromContext(ctx context.Context) coresettings.Injectable {
return FromContext(ctx)
}

func ToContext(ctx context.Context, s *Settings) context.Context {
return context.WithValue(ctx, ContextKey, s)
}
Expand Down
8 changes: 0 additions & 8 deletions pkg/apis/settings/settings_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ func (s Settings) Validate() (errs *apis.FieldError) {
return errs.Also(
s.validateEndpoint(),
s.validateTags(),
s.validateClusterName(),
s.validateVMMemoryOverheadPercent(),
s.validateReservedENIs(),
s.validateAssumeRoleDuration(),
Expand All @@ -42,13 +41,6 @@ func (s Settings) validateAssumeRoleDuration() (errs *apis.FieldError) {
return nil
}

func (s Settings) validateClusterName() (errs *apis.FieldError) {
if s.ClusterName == "" {
return errs.Also(apis.ErrMissingField("clusterName is required", "clusterName"))
}
return nil
}

func (s Settings) validateEndpoint() (errs *apis.FieldError) {
if s.ClusterEndpoint == "" {
return nil
Expand Down
9 changes: 0 additions & 9 deletions pkg/apis/settings/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,6 @@ var _ = Describe("Validation", func() {
Expect(s.Tags).To(HaveKeyWithValue("kubernetes.io/role/key", "value2"))
Expect(s.Tags).To(HaveKeyWithValue("kubernetes.io/cluster/other-tag/hello", "value3"))
})
It("should fail validation with panic when clusterName not included", func() {
cm := &v1.ConfigMap{
Data: map[string]string{
"aws.clusterEndpoint": "https://00000000000000000000000.gr7.us-west-2.eks.amazonaws.com",
},
}
_, err := (&settings.Settings{}).Inject(ctx, cm)
Expect(err).To(HaveOccurred())
})
It("should fail validation when clusterEndpoint is invalid (not absolute)", func() {
cm := &v1.ConfigMap{
Data: map[string]string{
Expand Down
12 changes: 6 additions & 6 deletions pkg/cloudprovider/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,23 +30,22 @@ import (

"github.com/aws/karpenter/pkg/apis"
"github.com/aws/karpenter/pkg/apis/settings"
"github.com/aws/karpenter/pkg/operator/options"
"github.com/aws/karpenter/pkg/test"

"github.com/aws/karpenter/pkg/cloudprovider"

coresettings "github.com/aws/karpenter-core/pkg/apis/settings"
"github.com/aws/karpenter-core/pkg/controllers/provisioning"
"github.com/aws/karpenter-core/pkg/controllers/state"
"github.com/aws/karpenter-core/pkg/events"
"github.com/aws/karpenter-core/pkg/operator/options"
coreoptions "github.com/aws/karpenter-core/pkg/operator/options"
"github.com/aws/karpenter-core/pkg/operator/scheme"
coretest "github.com/aws/karpenter-core/pkg/test"
. "github.com/aws/karpenter-core/pkg/test/expectations"
)

var ctx context.Context
var stop context.CancelFunc
var opts *options.Options
var env *coretest.Environment
var awsEnv *test.Environment
var prov *provisioning.Provisioner
Expand All @@ -63,7 +62,8 @@ func TestAWS(t *testing.T) {

var _ = BeforeSuite(func() {
env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...))
ctx = coresettings.ToContext(ctx, coretest.Settings())
ctx = coreoptions.ToContext(ctx, coretest.Options())
ctx = options.ToContext(ctx, test.Options())
ctx = settings.ToContext(ctx, test.Settings())
ctx, stop = context.WithCancel(ctx)
awsEnv = test.NewEnvironment(ctx, env)
Expand All @@ -81,8 +81,8 @@ var _ = AfterSuite(func() {
})

var _ = BeforeEach(func() {
ctx = options.ToContext(ctx, opts)
ctx = coresettings.ToContext(ctx, coretest.Settings())
ctx = coreoptions.ToContext(ctx, coretest.Options())
ctx = options.ToContext(ctx, test.Options())
ctx = settings.ToContext(ctx, test.Settings())

cluster.Reset()
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ import (

"github.com/aws/karpenter-core/pkg/events"
"github.com/aws/karpenter-core/pkg/operator/controller"
"github.com/aws/karpenter/pkg/apis/settings"
"github.com/aws/karpenter/pkg/cache"
"github.com/aws/karpenter/pkg/cloudprovider"
"github.com/aws/karpenter/pkg/controllers/interruption"
nodeclaimgarbagecollection "github.com/aws/karpenter/pkg/controllers/nodeclaim/garbagecollection"
nodeclaimlink "github.com/aws/karpenter/pkg/controllers/nodeclaim/link"
nodeclaimtagging "github.com/aws/karpenter/pkg/controllers/nodeclaim/tagging"
"github.com/aws/karpenter/pkg/controllers/nodeclass"
"github.com/aws/karpenter/pkg/operator/options"
"github.com/aws/karpenter/pkg/providers/amifamily"
"github.com/aws/karpenter/pkg/providers/instance"
"github.com/aws/karpenter/pkg/providers/instanceprofile"
Expand All @@ -54,10 +54,10 @@ func NewControllers(ctx context.Context, sess *session.Session, clk clock.Clock,
nodeclaimgarbagecollection.NewController(kubeClient, cloudProvider, linkController),
nodeclaimtagging.NewController(kubeClient, instanceProvider),
}
if settings.FromContext(ctx).InterruptionQueueName != "" {
if options.FromContext(ctx).InterruptionQueue != "" {
controllers = append(controllers, interruption.NewController(kubeClient, clk, recorder, interruption.NewSQSProvider(sqs.New(sess)), unavailableOfferings))
}
if settings.FromContext(ctx).IsolatedVPC {
if options.FromContext(ctx).IsolatedVPC {
logging.FromContext(ctx).Infof("assuming isolated VPC, pricing information will not be updated")
} else {
controllers = append(controllers, pricing.NewController(pricingProvider))
Expand Down
6 changes: 3 additions & 3 deletions pkg/controllers/interruption/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ import (

"github.com/aws/karpenter-core/pkg/apis/v1beta1"
"github.com/aws/karpenter-core/pkg/utils/pretty"
"github.com/aws/karpenter/pkg/apis/settings"
"github.com/aws/karpenter/pkg/apis/v1alpha1"
"github.com/aws/karpenter/pkg/cache"
interruptionevents "github.com/aws/karpenter/pkg/controllers/interruption/events"
"github.com/aws/karpenter/pkg/controllers/interruption/messages"
"github.com/aws/karpenter/pkg/controllers/interruption/messages/statechange"
"github.com/aws/karpenter/pkg/operator/options"
"github.com/aws/karpenter/pkg/utils"

"github.com/aws/karpenter-core/pkg/events"
Expand Down Expand Up @@ -80,8 +80,8 @@ func NewController(kubeClient client.Client, clk clock.Clock, recorder events.Re
}

func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconcile.Result, error) {
ctx = logging.WithLogger(ctx, logging.FromContext(ctx).With("queue", settings.FromContext(ctx).InterruptionQueueName))
if c.cm.HasChanged(settings.FromContext(ctx).InterruptionQueueName, nil) {
ctx = logging.WithLogger(ctx, logging.FromContext(ctx).With("queue", options.FromContext(ctx).InterruptionQueue))
if c.cm.HasChanged(options.FromContext(ctx).InterruptionQueue, nil) {
logging.FromContext(ctx).Debugf("watching interruption queue")
}
sqsMessages, err := c.sqsProvider.GetSQSMessages(ctx)
Expand Down
10 changes: 6 additions & 4 deletions pkg/controllers/interruption/interruption_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/aws/karpenter-core/pkg/operator/scheme"
"github.com/aws/karpenter/pkg/operator/options"
"github.com/aws/karpenter/pkg/apis/settings"
awscache "github.com/aws/karpenter/pkg/cache"
"github.com/aws/karpenter/pkg/controllers/interruption"
"github.com/aws/karpenter/pkg/controllers/interruption/events"
"github.com/aws/karpenter/pkg/fake"
"github.com/aws/karpenter/pkg/test"

coresettings "github.com/aws/karpenter-core/pkg/apis/settings"
coreoptions "github.com/aws/karpenter-core/pkg/operator/options"
"github.com/aws/karpenter-core/pkg/apis/v1alpha5"
coretest "github.com/aws/karpenter-core/pkg/test"
)
Expand All @@ -77,12 +78,13 @@ func BenchmarkNotification100(b *testing.B) {
func benchmarkNotificationController(b *testing.B, messageCount int) {
ctx = logging.WithLogger(ctx, logging.FromContext(ctx).With("message-count", messageCount))
fakeClock = &clock.FakeClock{}
ctx = coresettings.ToContext(ctx, coretest.Settings())
ctx = settings.ToContext(ctx, test.Settings(test.SettingOptions{
ctx = coreoptions.ToContext(ctx, coretest.Options())
ctx = options.ToContext(ctx, test.Options(test.OptionsFields{
ClusterName: lo.ToPtr("karpenter-notification-benchmarking"),
IsolatedVPC: lo.ToPtr(true),
InterruptionQueueName: lo.ToPtr("test-cluster"),
}))
ctx = settings.ToContext(ctx, test.Settings())
env = coretest.NewEnvironment(scheme.Scheme)
// Stop the coretest environment after the coretest completes
defer func() {
Expand Down Expand Up @@ -181,7 +183,7 @@ func newProviders(kubeClient client.Client) providerSet {

func (p *providerSet) makeInfrastructure(ctx context.Context) error {
if _, err := p.sqsAPI.CreateQueueWithContext(ctx, &sqs.CreateQueueInput{
QueueName: lo.ToPtr(settings.FromContext(ctx).InterruptionQueueName),
QueueName: lo.ToPtr(options.FromContext(ctx).InterruptionQueueName),
Attributes: map[string]*string{
sqs.QueueAttributeNameMessageRetentionPeriod: aws.String("1200"), // 20 minutes for this test
},
Expand Down
8 changes: 4 additions & 4 deletions pkg/controllers/interruption/sqs.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
"github.com/samber/lo"

"github.com/aws/karpenter-core/pkg/utils/atomic"
"github.com/aws/karpenter/pkg/apis/settings"
awserrors "github.com/aws/karpenter/pkg/errors"
"github.com/aws/karpenter/pkg/operator/options"
)

type SQSProvider struct {
Expand All @@ -43,7 +43,7 @@ func NewSQSProvider(client sqsiface.SQSAPI) *SQSProvider {
}
provider.queueURL.Resolve = func(ctx context.Context) (string, error) {
input := &sqs.GetQueueUrlInput{
QueueName: aws.String(settings.FromContext(ctx).InterruptionQueueName),
QueueName: aws.String(options.FromContext(ctx).InterruptionQueue),
}
ret, err := provider.client.GetQueueUrlWithContext(ctx, input)
if err != nil {
Expand All @@ -66,12 +66,12 @@ func (s *SQSProvider) QueueExists(ctx context.Context) (bool, error) {
}

func (s *SQSProvider) DiscoverQueueURL(ctx context.Context) (string, error) {
if settings.FromContext(ctx).InterruptionQueueName != lo.FromPtr(s.queueName.Load()) {
if options.FromContext(ctx).InterruptionQueue != lo.FromPtr(s.queueName.Load()) {
res, err := s.queueURL.TryGet(ctx, atomic.IgnoreCacheOption)
if err != nil {
return res, err
}
s.queueName.Store(lo.ToPtr(settings.FromContext(ctx).InterruptionQueueName))
s.queueName.Store(lo.ToPtr(options.FromContext(ctx).InterruptionQueue))
return res, nil
}
return s.queueURL.TryGet(ctx)
Expand Down
10 changes: 6 additions & 4 deletions pkg/controllers/interruption/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ import (
_ "knative.dev/pkg/system/testing"
"sigs.k8s.io/controller-runtime/pkg/client"

coresettings "github.com/aws/karpenter-core/pkg/apis/settings"
"github.com/aws/karpenter-core/pkg/apis/v1alpha5"
corev1beta1 "github.com/aws/karpenter-core/pkg/apis/v1beta1"
"github.com/aws/karpenter-core/pkg/events"
coreoptions "github.com/aws/karpenter-core/pkg/operator/options"
"github.com/aws/karpenter-core/pkg/operator/scheme"
coretest "github.com/aws/karpenter-core/pkg/test"
. "github.com/aws/karpenter-core/pkg/test/expectations"
Expand All @@ -53,6 +53,7 @@ import (
"github.com/aws/karpenter/pkg/controllers/interruption/messages/spotinterruption"
"github.com/aws/karpenter/pkg/controllers/interruption/messages/statechange"
"github.com/aws/karpenter/pkg/fake"
"github.com/aws/karpenter/pkg/operator/options"
"github.com/aws/karpenter/pkg/test"
"github.com/aws/karpenter/pkg/utils"
)
Expand Down Expand Up @@ -91,10 +92,11 @@ var _ = AfterSuite(func() {
})

var _ = BeforeEach(func() {
ctx = coresettings.ToContext(ctx, coretest.Settings())
ctx = settings.ToContext(ctx, test.Settings(test.SettingOptions{
InterruptionQueueName: lo.ToPtr("test-cluster"),
ctx = coreoptions.ToContext(ctx, coretest.Options())
ctx = options.ToContext(ctx, test.Options(test.OptionsFields{
InterruptionQueue: lo.ToPtr("test-cluster"),
}))
ctx = settings.ToContext(ctx, test.Settings())
unavailableOfferingsCache.Flush()
sqsapi.Reset()
sqsProvider.Reset()
Expand Down
Loading