diff --git a/karpenter-operator/controllers/karpenter/karpenter_controller.go b/karpenter-operator/controllers/karpenter/karpenter_controller.go index 812784cba2..fb230b7172 100644 --- a/karpenter-operator/controllers/karpenter/karpenter_controller.go +++ b/karpenter-operator/controllers/karpenter/karpenter_controller.go @@ -14,10 +14,9 @@ import ( corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -27,6 +26,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/source" + + awskarpenterv1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" ) const ( @@ -81,10 +82,8 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, man } // Watch EC2NodeClass guest side. - if err := c.Watch(source.Kind[client.Object](mgr.GetCache(), &unstructured.Unstructured{Object: map[string]interface{}{ - "apiVersion": "karpenter.k8s.aws/v1", - "kind": "EC2NodeClass", - }}, &handler.EnqueueRequestForObject{})); err != nil { + if err := c.Watch(source.Kind(mgr.GetCache(), &awskarpenterv1.EC2NodeClass{}, + &handler.TypedEnqueueRequestForObject[*awskarpenterv1.EC2NodeClass]{})); err != nil { return fmt.Errorf("failed to watch EC2NodeClass: %w", err) } @@ -211,12 +210,7 @@ func (r *Reconciler) reconcileCRDs(ctx context.Context, onlyCreate bool) error { func (r *Reconciler) reconcileEC2NodeClassOwnedFields(ctx context.Context, userDataSecret *corev1.Secret) error { log := ctrl.LoggerFrom(ctx) - ec2NodeClassList := &unstructured.UnstructuredList{} - ec2NodeClassList.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "karpenter.k8s.aws", - Version: "v1", - Kind: "EC2NodeClassList", - }) + ec2NodeClassList := &awskarpenterv1.EC2NodeClassList{} err := r.GuestClient.List(ctx, ec2NodeClassList) if err != nil { return fmt.Errorf("failed to get EC2NodeClassList: %w", err) @@ -224,17 +218,12 @@ func (r *Reconciler) reconcileEC2NodeClassOwnedFields(ctx context.Context, userD errs := []error{} for _, ec2NodeClass := range ec2NodeClassList.Items { - ec2NodeClass.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "karpenter.k8s.aws", - Version: "v1", - Kind: "EC2NodeClass", - }) op, err := r.CreateOrUpdate(ctx, r.GuestClient, &ec2NodeClass, func() error { - ec2NodeClass.Object["spec"].(map[string]interface{})["userData"] = string(userDataSecret.Data["value"]) - ec2NodeClass.Object["spec"].(map[string]interface{})["amiFamily"] = "Custom" - ec2NodeClass.Object["spec"].(map[string]interface{})["amiSelectorTerms"] = []map[string]interface{}{ + ec2NodeClass.Spec.UserData = ptr.To(string(userDataSecret.Data["value"])) + ec2NodeClass.Spec.AMIFamily = ptr.To("Custom") + ec2NodeClass.Spec.AMISelectorTerms = []awskarpenterv1.AMISelectorTerm{ { - "id": string(userDataSecret.Labels[userDataAMILabel]), + ID: string(userDataSecret.Labels[userDataAMILabel]), }, } return nil @@ -256,40 +245,37 @@ func (r *Reconciler) reconcileEC2NodeClassDefault(ctx context.Context, userDataS log := ctrl.LoggerFrom(ctx) // Create an unstructured object for the EC2NodeClass - ec2NodeClass := &unstructured.Unstructured{} - ec2NodeClass.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "karpenter.k8s.aws", - Version: "v1", - Kind: "EC2NodeClass", - }) + ec2NodeClass := &awskarpenterv1.EC2NodeClass{} ec2NodeClass.SetName("default") op, err := r.CreateOrUpdate(ctx, r.GuestClient, ec2NodeClass, func() error { - ec2NodeClass.Object["spec"] = map[string]interface{}{} - ec2NodeClass.Object["spec"].(map[string]interface{})["role"] = "KarpenterNodeRole-agl" // TODO(alberto): set a convention for this e.g. openshift-karpenter-infraID - ec2NodeClass.Object["spec"].(map[string]interface{})["userData"] = string(userDataSecret.Data["value"]) - ec2NodeClass.Object["spec"].(map[string]interface{})["amiFamily"] = "Custom" - ec2NodeClass.Object["spec"].(map[string]interface{})["amiSelectorTerms"] = []map[string]interface{}{ - { - "id": string(userDataSecret.Labels[userDataAMILabel]), + ec2NodeClass.Spec = awskarpenterv1.EC2NodeClassSpec{ + Role: "KarpenterNodeRole-agl", // TODO(alberto): set a convention for this e.g. openshift-karpenter-infraID + UserData: ptr.To(string(userDataSecret.Data["value"])), + AMIFamily: ptr.To("Custom"), + AMISelectorTerms: []awskarpenterv1.AMISelectorTerm{ + { + ID: string(userDataSecret.Labels[userDataAMILabel]), + }, }, - } - ec2NodeClass.Object["spec"].(map[string]interface{})["subnetSelectorTerms"] = []map[string]interface{}{ - { - "tags": map[string]interface{}{ - "karpenter.sh/discovery": hcp.Spec.InfraID, + SubnetSelectorTerms: []awskarpenterv1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "karpenter.sh/discovery": hcp.Spec.InfraID, + }, }, }, - } - ec2NodeClass.Object["spec"].(map[string]interface{})["securityGroupSelectorTerms"] = []map[string]interface{}{ - { - "tags": map[string]interface{}{ - "karpenter.sh/discovery": hcp.Spec.InfraID, + SecurityGroupSelectorTerms: []awskarpenterv1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "karpenter.sh/discovery": hcp.Spec.InfraID, + }, }, }, } + if hcp.Annotations[hyperv1.AWSMachinePublicIPs] == "true" { - ec2NodeClass.Object["spec"].(map[string]interface{})["associatePublicIPAddress"] = true + ec2NodeClass.Spec.AssociatePublicIPAddress = ptr.To(true) } return nil }) diff --git a/karpenter-operator/controllers/karpenter/karpenter_controller_test.go b/karpenter-operator/controllers/karpenter/karpenter_controller_test.go index 5417e8b679..ba1a181cc4 100644 --- a/karpenter-operator/controllers/karpenter/karpenter_controller_test.go +++ b/karpenter-operator/controllers/karpenter/karpenter_controller_test.go @@ -12,13 +12,14 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + + awskarpenterv1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1" ) func TestReconcileEC2NodeClassDefault(t *testing.T) { @@ -27,20 +28,12 @@ func TestReconcileEC2NodeClassDefault(t *testing.T) { _ = hyperv1.AddToScheme(scheme) // Register the EC2NodeClass GVK in the scheme - ec2NodeClassGVK := schema.GroupVersionKind{ - Group: "karpenter.k8s.aws", - Version: "v1", - Kind: "EC2NodeClass", - } - scheme.AddKnownTypeWithName(ec2NodeClassGVK, &unstructured.Unstructured{}) scheme.AddKnownTypeWithName( schema.GroupVersionKind{ Group: "karpenter.k8s.aws", Version: "v1", - Kind: "EC2NodeClassList", - }, - &unstructured.UnstructuredList{}, - ) + Kind: "EC2NodeClass", + }, &awskarpenterv1.EC2NodeClass{}) testCases := []struct { name string @@ -88,55 +81,32 @@ func TestReconcileEC2NodeClassDefault(t *testing.T) { } // Verify the EC2NodeClass was created. - got := &unstructured.Unstructured{} - got.SetGroupVersionKind(ec2NodeClassGVK) - + got := &awskarpenterv1.EC2NodeClass{} err = fakeClient.Get(context.Background(), types.NamespacedName{Name: "default"}, got) if err != nil { t.Errorf("failed to get EC2NodeClass: %v", err) return } - spec, ok := got.Object["spec"].(map[string]interface{}) - if !ok { - t.Fatal("spec is not a map") - } - // Verify basic fields - g.Expect(spec["role"]).To(Equal("KarpenterNodeRole-agl"), "role = %v, want KarpenterNodeRole-agl", spec["role"]) - g.Expect(spec["userData"]).To(Equal("test-userdata"), "userData = %v, want test-userdata", spec["userData"]) - g.Expect(spec["amiFamily"]).To(Equal("Custom"), "amiFamily = %v, want Custom", spec["amiFamily"]) + g.Expect(got.Spec.Role).To(Equal("KarpenterNodeRole-agl"), "role = %v, want KarpenterNodeRole-agl", got.Spec.Role) + g.Expect(got.Spec.UserData).To(HaveValue(Equal("test-userdata")), "userData = %v, want test-userdata", got.Spec.UserData) + g.Expect(got.Spec.AMIFamily).To(HaveValue(Equal("Custom")), "amiFamily = %v, want Custom", got.Spec.AMIFamily) // Verify amiSelectorTerms - amiTerms, ok := spec["amiSelectorTerms"].([]interface{}) - g.Expect(ok).To(BeTrue(), "amiSelectorTerms should be a slice") - g.Expect(len(amiTerms)).To(Equal(1), "amiSelectorTerms should have exactly one element") - - amiTerm, ok := amiTerms[0].(map[string]interface{}) - g.Expect(ok).To(BeTrue(), "amiTerm should be a map") - g.Expect(amiTerm["id"]).To(Equal("ami-123"), "unexpected amiSelectorTerms: %v", amiTerms) + g.Expect(len(got.Spec.AMISelectorTerms)).To(Equal(1), "amiSelectorTerms should have exactly one element") + g.Expect(got.Spec.AMISelectorTerms[0].ID).To(Equal("ami-123"), "unexpected amiSelectorTerms: %v", got.Spec.AMISelectorTerms) // Verify selector terms have correct tags - expectedTag := map[string]interface{}{ + expectedTags := map[string]string{ "karpenter.sh/discovery": "test-infra", } - // Helper function to verify selector terms - verifySelectorTerms := func(field string, expectedTags map[string]interface{}) { - terms, ok := spec[field].([]interface{}) - g.Expect(ok).To(BeTrue(), "terms should be a slice for field %s", field) - g.Expect(len(terms)).To(Equal(1), "terms should have exactly one element for field %s", field) - - term, ok := terms[0].(map[string]interface{}) - g.Expect(ok).To(BeTrue(), "term should be a map for field %s", field) - - tags, ok := term["tags"].(map[string]interface{}) - g.Expect(ok).To(BeTrue(), "tags should be a map for field %s", field) - g.Expect(tags).To(Equal(expectedTags), "%s tags = %v, want %v", field, tags, expectedTags) - } + g.Expect(len(got.Spec.SubnetSelectorTerms)).To(Equal(1), "SubnetSelectorTerms should have exactly one element for field") + g.Expect(got.Spec.SubnetSelectorTerms[0].Tags).To(Equal(expectedTags), "SubnetSelectorTerms tags = %v, want %v", got.Spec.SubnetSelectorTerms[0].Tags, expectedTags) - verifySelectorTerms("subnetSelectorTerms", expectedTag) - verifySelectorTerms("securityGroupSelectorTerms", expectedTag) + g.Expect(len(got.Spec.SecurityGroupSelectorTerms)).To(Equal(1), "SecurityGroupSelectorTerms should have exactly one element for field") + g.Expect(got.Spec.SecurityGroupSelectorTerms[0].Tags).To(Equal(expectedTags), "SecurityGroupSelectorTerms tags = %v, want %v", got.Spec.SecurityGroupSelectorTerms[0].Tags, expectedTags) }) } } diff --git a/karpenter-operator/controllers/karpenter/machine_approver.go b/karpenter-operator/controllers/karpenter/machine_approver.go index 1cffe327e4..7e1725e2ba 100644 --- a/karpenter-operator/controllers/karpenter/machine_approver.go +++ b/karpenter-operator/controllers/karpenter/machine_approver.go @@ -17,8 +17,6 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime/schema" certificatesv1client "k8s.io/client-go/kubernetes/typed/certificates/v1" ctrl "sigs.k8s.io/controller-runtime" @@ -27,6 +25,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/source" + karpenterv1 "sigs.k8s.io/karpenter/pkg/apis/v1" ) const ( @@ -155,15 +154,14 @@ func (r *MachineApproverController) authorize(ctx context.Context, csr *certific return false, nil } -func getEC2InstancesDNSNames(ctx context.Context, nodeClaims *unstructured.UnstructuredList, ec2Client ec2iface.EC2API) ([]string, error) { +func getEC2InstancesDNSNames(ctx context.Context, nodeClaims []karpenterv1.NodeClaim, ec2Client ec2iface.EC2API) ([]string, error) { ec2InstanceIDs := []string{} - for _, claim := range nodeClaims.Items { - nodeName := claim.UnstructuredContent()["status"].(map[string]interface{})["nodeName"] - if nodeName != nil { + for _, claim := range nodeClaims { + if claim.Status.NodeName != "" { // skip if a node is already created for this nodeClaim. continue } - providerID := claim.UnstructuredContent()["status"].(map[string]interface{})["providerID"].(string) + providerID := claim.Status.ProviderID instanceID := providerID[strings.LastIndex(providerID, "/")+1:] ec2InstanceIDs = append(ec2InstanceIDs, instanceID) @@ -217,17 +215,12 @@ func getEC2Client() (ec2iface.EC2API, error) { return ec2Client, nil } -func listNodeClaims(ctx context.Context, client client.Client) (*unstructured.UnstructuredList, error) { - nodeClaimList := &unstructured.UnstructuredList{} - nodeClaimList.SetGroupVersionKind(schema.GroupVersionKind{ - Group: "karpenter.sh", - Version: "v1", - Kind: "NodeClaim", - }) +func listNodeClaims(ctx context.Context, client client.Client) ([]karpenterv1.NodeClaim, error) { + nodeClaimList := &karpenterv1.NodeClaimList{} err := client.List(ctx, nodeClaimList) if err != nil { return nil, fmt.Errorf("failed to list NodeClaims: %w", err) } - return nodeClaimList, nil + return nodeClaimList.Items, nil } diff --git a/karpenter-operator/controllers/karpenter/machine_approver_test.go b/karpenter-operator/controllers/karpenter/machine_approver_test.go index 7377f02275..2707840bf4 100644 --- a/karpenter-operator/controllers/karpenter/machine_approver_test.go +++ b/karpenter-operator/controllers/karpenter/machine_approver_test.go @@ -21,12 +21,12 @@ import ( "github.com/aws/aws-sdk-go/service/ec2/ec2iface" certificatesv1 "k8s.io/api/certificates/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + karpenterv1 "sigs.k8s.io/karpenter/pkg/apis/v1" ) type fakeEC2Client struct { @@ -50,29 +50,22 @@ func TestAuthorize(t *testing.T) { _ = hyperv1.AddToScheme(scheme) // Register the NodeClaim GVK in the scheme - nodeClaimGVK := schema.GroupVersionKind{ + scheme.AddKnownTypeWithName(schema.GroupVersionKind{ Group: "karpenter.sh", Version: "v1", Kind: "NodeClaim", - } - scheme.AddKnownTypeWithName(nodeClaimGVK, &unstructured.Unstructured{}) - scheme.AddKnownTypeWithName( - schema.GroupVersionKind{ - Group: "karpenter.sh", - Version: "v1", - Kind: "NodeClaimList", - }, - &unstructured.UnstructuredList{}, - ) + }, &karpenterv1.NodeClaim{}) + scheme.AddKnownTypeWithName(schema.GroupVersionKind{ + Group: "karpenter.sh", + Version: "v1", + Kind: "NodeClaimList", + }, &karpenterv1.NodeClaimList{}) - fakeNodeClaim := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "status": map[string]interface{}{ - "providerID": "aws:///fakeproviderID", - }, + fakeNodeClaim := &karpenterv1.NodeClaim{ + Status: karpenterv1.NodeClaimStatus{ + ProviderID: "aws:///fakeproviderID", }, } - fakeNodeClaim.SetGroupVersionKind(nodeClaimGVK) testCases := []struct { name string