Skip to content

Commit

Permalink
added version controller and other feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
edibble21 committed Nov 25, 2024
1 parent 6b79ce7 commit 15e55ed
Show file tree
Hide file tree
Showing 16 changed files with 228 additions and 21 deletions.
1 change: 1 addition & 0 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func main() {
op.PricingProvider,
op.AMIProvider,
op.LaunchTemplateProvider,
op.VersionProvider,
op.InstanceTypesProvider,
)...).
Start(ctx)
Expand Down
7 changes: 7 additions & 0 deletions pkg/cloudprovider/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
"github.com/aws/karpenter-provider-aws/pkg/cloudprovider"
"github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status"
controllersversion "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/version"
"github.com/aws/karpenter-provider-aws/pkg/fake"
"github.com/aws/karpenter-provider-aws/pkg/operator/options"
"github.com/aws/karpenter-provider-aws/pkg/test"
Expand Down Expand Up @@ -1149,7 +1150,9 @@ var _ = Describe("CloudProvider", func() {
Tags: []ec2types.Tag{{Key: aws.String("Name"), Value: aws.String("test-subnet-2")}}},
}})
controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider)
versionController := controllersversion.NewController(awsEnv.VersionProvider)
ExpectApplied(ctx, env.Client, nodePool, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, controller, nodeClass)
pod := coretest.UnschedulablePod(coretest.PodOptions{NodeSelector: map[string]string{corev1.LabelTopologyZone: "test-zone-1a"}})
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, pod)
Expand All @@ -1166,11 +1169,13 @@ var _ = Describe("CloudProvider", func() {
Tags: []ec2types.Tag{{Key: aws.String("Name"), Value: aws.String("test-subnet-2")}}},
}})
controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider)
versionController := controllersversion.NewController(awsEnv.VersionProvider)
nodeClass.Spec.Kubelet = &v1.KubeletConfiguration{
MaxPods: aws.Int32(1),
}
ExpectApplied(ctx, env.Client, nodePool, nodeClass)
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, controller, nodeClass)
pod1 := coretest.UnschedulablePod(coretest.PodOptions{NodeSelector: map[string]string{corev1.LabelTopologyZone: "test-zone-1a"}})
pod2 := coretest.UnschedulablePod(coretest.PodOptions{NodeSelector: map[string]string{corev1.LabelTopologyZone: "test-zone-1a"}})
Expand Down Expand Up @@ -1207,6 +1212,8 @@ var _ = Describe("CloudProvider", func() {
nodeClass.Spec.SubnetSelectorTerms = []v1.SubnetSelectorTerm{{Tags: map[string]string{"Name": "test-subnet-1"}}}
ExpectApplied(ctx, env.Client, nodePool, nodeClass)
controller := status.NewController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider)
versionController := controllersversion.NewController(awsEnv.VersionProvider)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, controller, nodeClass)
podSubnet1 := coretest.UnschedulablePod()
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, prov, podSubnet1)
Expand Down
4 changes: 4 additions & 0 deletions pkg/controllers/controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ import (
controllersinstancetypecapacity "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/instancetype/capacity"
controllerspricing "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/pricing"
ssminvalidation "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/ssm/invalidation"
controllersversion "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/version"
"github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate"
"github.com/aws/karpenter-provider-aws/pkg/providers/version"

servicesqs "github.com/aws/aws-sdk-go-v2/service/sqs"
"github.com/samber/lo"
Expand Down Expand Up @@ -76,6 +78,7 @@ func NewControllers(
pricingProvider pricing.Provider,
amiProvider amifamily.Provider,
launchTemplateProvider launchtemplate.Provider,
versionProvider *version.DefaultProvider,
instanceTypeProvider *instancetype.DefaultProvider) []controller.Controller {
controllers := []controller.Controller{
nodeclasshash.NewController(kubeClient),
Expand All @@ -89,6 +92,7 @@ func NewControllers(
ssminvalidation.NewController(ssmCache, amiProvider),
status.NewController[*v1.EC2NodeClass](kubeClient, mgr.GetEventRecorderFor("karpenter"), status.EmitDeprecatedMetrics),
opevents.NewController[*corev1.Node](kubeClient, clk),
controllersversion.NewController(versionProvider),
}
if options.FromContext(ctx).InterruptionQueue != "" {
sqsapi := servicesqs.NewFromConfig(cfg)
Expand Down
4 changes: 4 additions & 0 deletions pkg/controllers/nodeclass/status/ami_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
var _ = Describe("NodeClass AMI Status Controller", func() {
var k8sVersion string
BeforeEach(func() {
ExpectSingletonReconciled(ctx, versionController)
k8sVersion = lo.Must(awsEnv.VersionProvider.Get(ctx))
nodeClass = test.EC2NodeClass(v1.EC2NodeClass{
Spec: v1.EC2NodeClassSpec{
Expand Down Expand Up @@ -132,6 +133,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
}
nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}}
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
nodeClass = ExpectExists(ctx, env.Client, nodeClass)

Expand Down Expand Up @@ -216,6 +218,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
}
nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2@latest"}}
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
nodeClass = ExpectExists(ctx, env.Client, nodeClass)

Expand Down Expand Up @@ -419,6 +422,7 @@ var _ = Describe("NodeClass AMI Status Controller", func() {
}
nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "windows2022@latest"}}
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
nodeClass = ExpectExists(ctx, env.Client, nodeClass)

Expand Down
5 changes: 5 additions & 0 deletions pkg/controllers/nodeclass/status/instanceprofile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
It("should create the instance profile when it doesn't exist", func() {
nodeClass.Spec.Role = "test-role"
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)

Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1))
Expand All @@ -64,6 +65,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {

nodeClass.Spec.Role = "test-role"
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)

Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1))
Expand All @@ -89,6 +91,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {

nodeClass.Spec.Role = "test-role"
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)

Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1))
Expand Down Expand Up @@ -129,6 +132,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
nodeClass.Spec.Role = ""
nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile")
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)

nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expand All @@ -139,6 +143,7 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
nodeClass.Spec.Role = ""
nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile")
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)

Expect(awsEnv.IAMAPI.CreateInstanceProfileBehavior.Calls()).To(BeZero())
Expand Down
3 changes: 3 additions & 0 deletions pkg/controllers/nodeclass/status/launchtemplate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ var _ = Describe("NodeClass Launch Template CIDR Resolution Controller", func()
nodeClass.Spec.AMIFamily = lo.ToPtr(family)
nodeClass.Spec.AMISelectorTerms = terms
ExpectApplied(ctx, env.Client, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
Expect(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load()).To(BeNil())
},
Expand All @@ -73,6 +74,7 @@ var _ = Describe("NodeClass Launch Template CIDR Resolution Controller", func()
nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}}
ExpectApplied(ctx, env.Client, nodeClass)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("10.100.0.0/16"))
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.StatusConditions().IsTrue(status.ConditionReady)).To(BeTrue())
Expand All @@ -90,6 +92,7 @@ var _ = Describe("NodeClass Launch Template CIDR Resolution Controller", func()
nodeClass.Spec.AMISelectorTerms = []v1.AMISelectorTerm{{Alias: "al2023@latest"}}
ExpectApplied(ctx, env.Client, nodeClass)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)
ExpectSingletonReconciled(ctx, versionController)
Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("2001:db8::/64"))
nodeClass = ExpectExists(ctx, env.Client, nodeClass)
Expect(nodeClass.StatusConditions().IsTrue(status.ConditionReady)).To(BeTrue())
Expand Down
4 changes: 4 additions & 0 deletions pkg/controllers/nodeclass/status/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/aws/karpenter-provider-aws/pkg/apis"
v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
"github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status"
controllersversion "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/version"
"github.com/aws/karpenter-provider-aws/pkg/operator/options"
"github.com/aws/karpenter-provider-aws/pkg/test"

Expand All @@ -40,6 +41,7 @@ var env *coretest.Environment
var awsEnv *test.Environment
var nodeClass *v1.EC2NodeClass
var statusController *status.Controller
var versionController *controllersversion.Controller

func TestAPIs(t *testing.T) {
ctx = TestContextWithLogger(t)
Expand All @@ -61,6 +63,8 @@ var _ = BeforeSuite(func() {
awsEnv.InstanceProfileProvider,
awsEnv.LaunchTemplateProvider,
)

versionController = controllersversion.NewController(awsEnv.VersionProvider)
})

var _ = AfterSuite(func() {
Expand Down
6 changes: 6 additions & 0 deletions pkg/controllers/providers/ssm/invalidation/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/aws/karpenter-provider-aws/pkg/apis"
v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
"github.com/aws/karpenter-provider-aws/pkg/controllers/providers/ssm/invalidation"
controllersversion "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/version"
"github.com/aws/karpenter-provider-aws/pkg/operator/options"
"github.com/aws/karpenter-provider-aws/pkg/providers/ssm"
"github.com/aws/karpenter-provider-aws/pkg/test"
Expand All @@ -44,6 +45,7 @@ var stop context.CancelFunc
var env *coretest.Environment
var awsEnv *test.Environment
var invalidationController *invalidation.Controller
var versionController *controllersversion.Controller

func TestAWS(t *testing.T) {
ctx = TestContextWithLogger(t)
Expand All @@ -59,6 +61,7 @@ var _ = BeforeSuite(func() {
awsEnv = test.NewEnvironment(ctx, env)

invalidationController = invalidation.NewController(awsEnv.SSMCache, awsEnv.AMIProvider)
versionController = controllersversion.NewController(awsEnv.VersionProvider)
})

var _ = AfterSuite(func() {
Expand All @@ -82,6 +85,7 @@ var _ = Describe("SSM Invalidation Controller", func() {
}
})
It("shouldn't invalidate cache entries for non-deprecated AMIs", func() {
ExpectSingletonReconciled(ctx, versionController)
_, err := awsEnv.AMIProvider.List(ctx, nodeClass)
Expect(err).To(BeNil())
currentEntries := getSSMCacheEntries()
Expand All @@ -100,6 +104,7 @@ var _ = Describe("SSM Invalidation Controller", func() {
}
})
It("shouldn't invalidate cache entries for deprecated AMIs when the SSM parameter is immutable", func() {
ExpectSingletonReconciled(ctx, versionController)
nodeClass.Spec.AMISelectorTerms[0].Alias = "al2023@v20241024"
_, err := awsEnv.AMIProvider.List(ctx, nodeClass)
Expect(err).To(BeNil())
Expand All @@ -120,6 +125,7 @@ var _ = Describe("SSM Invalidation Controller", func() {
}
})
It("should invalidate cache entries for deprecated AMIs when the SSM parameter is mutable", func() {
ExpectSingletonReconciled(ctx, versionController)
_, err := awsEnv.AMIProvider.List(ctx, nodeClass)
Expect(err).To(BeNil())
currentEntries := getSSMCacheEntries()
Expand Down
67 changes: 67 additions & 0 deletions pkg/controllers/providers/version/controller.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
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 version

import (
"context"
"fmt"
"time"

"github.com/awslabs/operatorpkg/singleton"
lop "github.com/samber/lo/parallel"
"go.uber.org/multierr"
controllerruntime "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/karpenter/pkg/operator/injection"

"github.com/aws/karpenter-provider-aws/pkg/providers/version"
)

type Controller struct {
versionProvider *version.DefaultProvider
}

func NewController(versionProvider *version.DefaultProvider) *Controller {
return &Controller{
versionProvider: versionProvider,
}
}

func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) {
ctx = injection.WithControllerName(ctx, "providers.version")

work := []func(ctx context.Context) error{
c.versionProvider.UpdateVersion,
}
errs := make([]error, len(work))
lop.ForEach(work, func(f func(ctx context.Context) error, i int) {
if err := f(ctx); err != nil {
errs[i] = err
}
})
if err := multierr.Combine(errs...); err != nil {
return reconcile.Result{}, fmt.Errorf("updating version, %w", err)
}
return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil
}

func (c *Controller) Register(_ context.Context, m manager.Manager) error {
// Includes a default exponential failure rate limiter of base: time.Millisecond, and max: 1000*time.Second
return controllerruntime.NewControllerManagedBy(m).
Named("providers.version").
WatchesRawSource(singleton.Source()).
Complete(singleton.AsReconciler(c))
}
87 changes: 87 additions & 0 deletions pkg/controllers/providers/version/suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
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 version_test

import (
"context"
"testing"

"sigs.k8s.io/karpenter/pkg/test/v1alpha1"

coreoptions "sigs.k8s.io/karpenter/pkg/operator/options"
coretest "sigs.k8s.io/karpenter/pkg/test"

"github.com/aws/aws-sdk-go-v2/service/eks"
ekstypes "github.com/aws/aws-sdk-go-v2/service/eks/types"
"github.com/aws/karpenter-provider-aws/pkg/apis"
controllersversion "github.com/aws/karpenter-provider-aws/pkg/controllers/providers/version"
"github.com/aws/karpenter-provider-aws/pkg/operator/options"
"github.com/aws/karpenter-provider-aws/pkg/test"
"github.com/samber/lo"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
. "sigs.k8s.io/karpenter/pkg/test/expectations"
. "sigs.k8s.io/karpenter/pkg/utils/testing"
)

var ctx context.Context
var stop context.CancelFunc
var env *coretest.Environment
var awsEnv *test.Environment
var controller *controllersversion.Controller

func TestAWS(t *testing.T) {
ctx = TestContextWithLogger(t)
RegisterFailHandler(Fail)
RunSpecs(t, "InstanceType")
}

var _ = BeforeSuite(func() {
env = coretest.NewEnvironment(coretest.WithCRDs(apis.CRDs...), coretest.WithCRDs(v1alpha1.CRDs...))
ctx = coreoptions.ToContext(ctx, coretest.Options())
ctx = options.ToContext(ctx, test.Options())
ctx, stop = context.WithCancel(ctx)
awsEnv = test.NewEnvironment(ctx, env)
controller = controllersversion.NewController(awsEnv.VersionProvider)
})

var _ = AfterSuite(func() {
stop()
Expect(env.Stop()).To(Succeed(), "Failed to stop environment")
})

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

awsEnv.Reset()
})

var _ = AfterEach(func() {
ExpectCleanedUp(ctx, env.Client)
})

var _ = Describe("Version", func() {
It("should update Version with response from the DescribeCluster API", func() {
awsEnv.EKSAPI.DescribeClusterBehavior.Output.Set(&eks.DescribeClusterOutput{
Cluster: &ekstypes.Cluster{
Version: lo.ToPtr("1.29"),
},
})
ExpectSingletonReconciled(ctx, controller)
Expect(awsEnv.VersionProvider.Get(ctx)).To(Equal("1.29"))
})
})
Loading

0 comments on commit 15e55ed

Please sign in to comment.