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: initialization touchups #2676

Closed
wants to merge 1 commit into from
Closed
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
23 changes: 9 additions & 14 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,15 @@ import (
)

func main() {
options, manager := operator.NewOptionsWithManagerOrDie()
cloudProvider := cloudprovider.CloudProvider(awscloudprovider.NewCloudProvider(options.Ctx, cloudprovider.Options{
ClientSet: options.Clientset,
KubeClient: options.KubeClient,
StartAsync: options.StartAsync,
}))
if hp, ok := cloudProvider.(operator.HealthCheck); ok {
utilruntime.Must(manager.AddHealthzCheck("cloud-provider", hp.LivenessProbe))
}
cloudProvider = cloudprovidermetrics.Decorate(cloudProvider)
if err := operator.RegisterControllers(options.Ctx,
manager,
controllers.GetControllers(options, cloudProvider)...,
).Start(options.Ctx); err != nil {
ctx, manager := operator.NewOrDie()
cloudProvider := awscloudprovider.NewCloudProvider(ctx, cloudprovider.Options{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some small concerns about exposing the manager through the context in this way. If someone does a typed conversion to the operator.Context object within the cloudprovider, they could then have access to the manager which seems like something we want to avoid

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Will play with this.

ClientSet: ctx.Clientset,
KubeClient: ctx.KubeClient,
StartAsync: ctx.StartAsync,
})
utilruntime.Must(manager.AddHealthzCheck("cloud-provider", cloudProvider.LivenessProbe))
operator.RegisterControllers(ctx, manager, controllers.GetControllers(ctx, cloudprovidermetrics.Decorate(cloudProvider))...)
if err := manager.Start(ctx); err != nil {
panic(fmt.Sprintf("Unable to start manager, %s", err))
}
}
4 changes: 2 additions & 2 deletions hack/docs/configuration_gen_docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"os"
"strings"

"github.com/aws/karpenter/pkg/utils/options"
"github.com/aws/karpenter/pkg/operator"
)

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

opts := options.New()
opts := operator.NewOptions()

envVarsBlock := "| Environment Variable | CLI Flag | Description |\n"
envVarsBlock += "|--|--|--|\n"
Expand Down
7 changes: 3 additions & 4 deletions hack/docs/instancetypes_gen_docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ import (
"github.com/aws/karpenter/pkg/cloudproviders/aws/apis/v1alpha1"
awscloudprovider "github.com/aws/karpenter/pkg/cloudproviders/aws/cloudprovider"
"github.com/aws/karpenter/pkg/cloudproviders/common/cloudprovider"
"github.com/aws/karpenter/pkg/utils/injection"
"github.com/aws/karpenter/pkg/utils/options"
"github.com/aws/karpenter/pkg/operator"
"github.com/aws/karpenter/pkg/utils/resources"
)

Expand All @@ -52,9 +51,9 @@ func main() {
os.Setenv("CLUSTER_ENDPOINT", "https://docs-gen.aws")
os.Setenv("AWS_ISOLATED_VPC", "true") // disable pricing lookup

opts := options.New()
opts := operator.NewOptions()
opts = opts.MustParse()
ctx := injection.WithOptions(context.Background(), *opts)
ctx := operator.WithOptions(context.Background(), *opts)

cp := awscloudprovider.NewCloudProvider(ctx, cloudprovider.Options{})
provider := v1alpha1.AWS{SubnetSelector: map[string]string{
Expand Down
3 changes: 2 additions & 1 deletion pkg/cloudproviders/aws/apis/v1alpha1/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/samber/lo"

"github.com/aws/karpenter-core/pkg/apis/provisioning/v1alpha5"
"github.com/aws/karpenter/pkg/utils/injection"

"github.com/aws/karpenter-core/pkg/apis/provisioning/v1alpha5"
)

func MergeTags(ctx context.Context, custom ...map[string]string) (result []*ec2.Tag) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/cloudproviders/aws/cloudprovider/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/transport"
"knative.dev/pkg/apis"
"knative.dev/pkg/injection"
"knative.dev/pkg/logging"
"knative.dev/pkg/ptr"
k8sClient "sigs.k8s.io/controller-runtime/pkg/client"
Expand All @@ -52,7 +53,6 @@ import (
"github.com/aws/karpenter/pkg/cloudproviders/aws/cloudprovider/amifamily"
"github.com/aws/karpenter/pkg/cloudproviders/common/cloudprovider"
"github.com/aws/karpenter/pkg/utils/functional"
"github.com/aws/karpenter/pkg/utils/injection"
"github.com/aws/karpenter/pkg/utils/project"
)

Expand Down
9 changes: 4 additions & 5 deletions pkg/cloudproviders/aws/cloudprovider/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,12 @@ import (

"github.com/aws/karpenter-core/pkg/apis/provisioning/v1alpha5"
awserrors "github.com/aws/karpenter/pkg/cloudproviders/aws/errors"
"github.com/aws/karpenter/pkg/operator"

"github.com/aws/karpenter-core/pkg/scheduling"
"github.com/aws/karpenter/pkg/cloudproviders/aws/apis/v1alpha1"
"github.com/aws/karpenter/pkg/cloudproviders/common/cloudprovider"
"github.com/aws/karpenter/pkg/utils/functional"
"github.com/aws/karpenter/pkg/utils/injection"
"github.com/aws/karpenter/pkg/utils/options"
"github.com/aws/karpenter/pkg/utils/resources"
)

Expand Down Expand Up @@ -145,7 +144,7 @@ func (p *InstanceProvider) launchInstance(ctx context.Context, provider *v1alpha
logging.FromContext(ctx).Warn(err.Error())
}
// Create fleet
tags := v1alpha1.MergeTags(ctx, provider.Tags, map[string]string{fmt.Sprintf("kubernetes.io/cluster/%s", injection.GetOptions(ctx).ClusterName): "owned"})
tags := v1alpha1.MergeTags(ctx, provider.Tags, map[string]string{fmt.Sprintf("kubernetes.io/cluster/%s", operator.GetOptions(ctx).ClusterName): "owned"})
createFleetInput := &ec2.CreateFleetInput{
Type: aws.String(ec2.FleetTypeInstant),
Context: provider.Context,
Expand Down Expand Up @@ -317,7 +316,7 @@ func (p *InstanceProvider) getInstance(ctx context.Context, id string) (*ec2.Ins
if *instance.State.Name == ec2.InstanceStateNameTerminated {
return nil, awserrors.InstanceTerminatedError{Err: fmt.Errorf("instance is in terminated state")}
}
if injection.GetOptions(ctx).GetAWSNodeNameConvention() == options.ResourceName {
if operator.GetOptions(ctx).GetAWSNodeNameConvention() == operator.ResourceName {
return instance, nil
}
if len(aws.StringValue(instance.PrivateDnsName)) == 0 {
Expand All @@ -330,7 +329,7 @@ func (p *InstanceProvider) instanceToNode(ctx context.Context, instance *ec2.Ins
for _, instanceType := range instanceTypes {
if instanceType.Name() == aws.StringValue(instance.InstanceType) {
nodeName := strings.ToLower(aws.StringValue(instance.PrivateDnsName))
if injection.GetOptions(ctx).GetAWSNodeNameConvention() == options.ResourceName {
if operator.GetOptions(ctx).GetAWSNodeNameConvention() == operator.ResourceName {
nodeName = aws.StringValue(instance.InstanceId)
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/cloudproviders/aws/cloudprovider/instancetype.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
"github.com/aws/karpenter/pkg/cloudproviders/aws/apis/v1alpha1"
"github.com/aws/karpenter/pkg/cloudproviders/aws/cloudprovider/amifamily"
"github.com/aws/karpenter/pkg/cloudproviders/common/cloudprovider"
"github.com/aws/karpenter/pkg/utils/injection"
"github.com/aws/karpenter/pkg/operator"
"github.com/aws/karpenter/pkg/utils/resources"
)

Expand Down Expand Up @@ -69,8 +69,8 @@ func NewInstanceType(ctx context.Context, info *ec2.InstanceTypeInfo, kc *v1alph
instanceType.maxPods = instanceType.computeMaxPods(ctx, kc)

// Precompute to minimize memory/compute overhead
instanceType.resources = instanceType.computeResources(injection.GetOptions(ctx).AWSEnablePodENI)
instanceType.overhead = instanceType.computeOverhead(injection.GetOptions(ctx).VMMemoryOverhead, kc)
instanceType.resources = instanceType.computeResources(operator.GetOptions(ctx).AWSEnablePodENI)
instanceType.overhead = instanceType.computeOverhead(operator.GetOptions(ctx).VMMemoryOverhead, kc)
instanceType.requirements = instanceType.computeRequirements()
return instanceType
}
Expand Down Expand Up @@ -370,7 +370,7 @@ func (i *InstanceType) computeMaxPods(ctx context.Context, kc *v1alpha5.KubeletC
switch {
case kc != nil && kc.MaxPods != nil:
mp = ptr.Int64(int64(ptr.Int32Value(kc.MaxPods)))
case !injection.GetOptions(ctx).AWSENILimitedPodDensity:
case !operator.GetOptions(ctx).AWSENILimitedPodDensity:
mp = ptr.Int64(110)
default:
mp = ptr.Int64(i.eniLimitedPods())
Expand Down
4 changes: 2 additions & 2 deletions pkg/cloudproviders/aws/cloudprovider/instancetypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/aws/karpenter-core/pkg/apis/provisioning/v1alpha5"
awscache "github.com/aws/karpenter/pkg/cloudproviders/aws/cache"
"github.com/aws/karpenter/pkg/cloudproviders/common/cloudprovider"
"github.com/aws/karpenter/pkg/operator"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/session"
Expand All @@ -37,7 +38,6 @@ import (

"github.com/aws/karpenter/pkg/cloudproviders/aws/apis/v1alpha1"
"github.com/aws/karpenter/pkg/utils/functional"
"github.com/aws/karpenter/pkg/utils/injection"
"github.com/aws/karpenter/pkg/utils/pretty"
)

Expand Down Expand Up @@ -71,7 +71,7 @@ func NewInstanceTypeProvider(ctx context.Context, sess *session.Session, options
NewPricingAPI(sess, *sess.Config.Region),
ec2api,
*sess.Config.Region,
injection.GetOptions(ctx).AWSIsolatedVPC, options.StartAsync),
operator.GetOptions(ctx).AWSIsolatedVPC, options.StartAsync),
cache: cache.New(InstanceTypesAndZonesCacheTTL, CacheCleanupInterval),
unavailableOfferings: unavailableOfferings,
cm: pretty.NewChangeMonitor(),
Expand Down
Loading