[CRD change] Add LocalDNSProfile to AKSNodeClassSpec + NAP Integration Tests #1233

saewoni · 2025-10-24T22:28:01Z

Fixes #

Description
We need to expose the localdns configuration in the CRD for Karpenter. This will allow NAP to leverage the same localdns currently used for node pools in AKS-RP.

How was this change tested?

Unit tests below

pkg/apis/v1alpha2/aksnodeclass_hash_test.go
pkg/apis/v1beta1/aksnodeclass_hash_test.go
pkg/apis/v1alpha2/crd_validation_cel_test.go
pkg/apis/v1beta1/crd_validation_cel_test.go

Need to learn how to do test like this PR: #688
Does this change impact docs?

Yes, PR includes docs updates
Yes, issue opened: #
No

Release Note

New AKSNodeClass.Spec field (LocalDNSProfile)

Copilot

Pull Request Overview

This PR adds LocalDNSProfile support to the AKSNodeClass CRD, enabling Karpenter to configure local DNS settings for AKS nodes. The implementation follows the AKS-RP proto definitions for consistency with the broader Azure ecosystem.

Key changes:

New LocalDNSProfile field in AKSNodeClassSpec with comprehensive DNS configuration options
Validation tests for all enum types and boundaries
Hash tests to ensure configuration changes trigger node updates

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pkg/apis/v1beta1/aksnodeclass.go	Added LocalDNSProfile types and enum constants with kubebuilder validation tags
pkg/apis/v1alpha2/aksnodeclass.go	Added LocalDNSProfile types and enum constants with kubebuilder validation tags
pkg/apis/v1beta1/zz_generated.deepcopy.go	Auto-generated DeepCopy methods for LocalDNSProfile and LocalDNSOverrides
pkg/apis/v1alpha2/zz_generated.deepcopy.go	Auto-generated DeepCopy methods for LocalDNSProfile and LocalDNSOverrides
pkg/apis/v1beta1/crd_validation_cel_test.go	Added validation tests for all LocalDNS enum types
pkg/apis/v1alpha2/crd_validation_cel_test.go	Added validation tests for all LocalDNS enum types
pkg/apis/v1beta1/aksnodeclass_hash_test.go	Added hash tests for LocalDNSProfile fields
pkg/apis/v1alpha2/aksnodeclass_hash_test.go	Added hash tests for LocalDNSProfile fields
pkg/apis/crds/karpenter.azure.com_aksnodeclasses.yaml	Updated CRD with LocalDNSProfile schema definitions
charts/karpenter-crd/templates/karpenter.azure.com_aksnodeclasses.yaml	Updated CRD template with LocalDNSProfile schema definitions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/apis/v1alpha2/aksnodeclass.go

Copilot

Pull Request Overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

matthchr

I don't think zz_generated.pricing.go needs to be regenerated for this PR?

pkg/apis/v1alpha2/aksnodeclass.go

pkg/providers/imagefamily/azlinux.go

pkg/apis/v1beta1/aksnodeclass.go

Copilot

Pull request overview

Copilot reviewed 36 out of 36 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/suites/integration/localdns_test.go

pkg/providers/instancetype/instancetypes.go

pkg/providers/labels/labels.go

pkg/apis/v1alpha2/crd_validation_cel_test.go

pkg/apis/v1beta1/aksnodeclass.go

matthchr · 2025-12-03T22:07:30Z

pkg/providers/imagefamily/customscriptsbootstrap/utils_test.go

+	}
+}
+
+func TestConvertLocalDNSOverrideToModel(t *testing.T) {


minor, style: Consider just putting these tests into the above test and passing everything through convertLocalDNSToModel (test the public interface or at least the most public part)

pkg/providers/labels/labels_test.go

typos.toml

test/suites/integration/localdns_test.go

saewoni · 2025-12-04T00:44:32Z

pkg/apis/v1beta1/aksnodeclass.go

+// LocalDNSOverrides specifies DNS override configuration
+type LocalDNSOverrides struct {
+	// Log level for DNS queries in localDNS.
+	// +optional


yup - i will make localdnsoverrides required only if localdns config is provided. i need to add the validation rule for it still. last time i attempted, my validation rule was too expensive so karpenter repo rejected it

…nMilliCores (stylecheck)

This reverts commit 21c7ab0.

This reverts commit d8e6a50.

… on the zone names

…tatus/localdns_test.go

…in pkg/test/nodeclass.go ApplyDefaultStatus function

… inflate pod

tallaxes

The logic looks good to me; left some comments around API, runtime validation (and status condition), and caching.

For the API best practices, had some feedback, started to question myself, and ended up just running kube-api-linter. Left what it found as comments - look for "(kubeapilinter)" - pretty much everything looks legitimate / should be addressed, I think. (For more insight on each see API Conventions doc.) We really should integrate kube-api-linter as part of our dev cycle/CI, TODO. There are more issues it flags elsewhere in our API, can be addressed later. But for the new API we introduce, let's get it right from the get go. (It might be possible to prioritize externally visible changes, and follow-up with internal-only later ...)

pkg/providers/instancetype/instancetypes.go

tallaxes · 2025-12-05T22:09:33Z

pkg/providers/instancetype/instancetypes.go

 		if !p.isInstanceTypeSupportedByEncryptionAtHost(sku, nodeClass) {
 			continue
 		}
+		if !p.isInstanceTypeSupportedByLocalDNS(sku, nodeClass) {


I think you have to add local DNS setting (maybe IsLocalDNSEnabled) to the cache key above, since it now affects the set of instance types

pkg/apis/v1beta1/aksnodeclass.go

tallaxes · 2025-12-05T23:42:38Z

pkg/apis/v1beta1/aksnodeclass_status.go

 	ConditionTypeImagesReady            = "ImagesReady"
 	ConditionTypeKubernetesVersionReady = "KubernetesVersionReady"
 	ConditionTypeSubnetsReady           = "SubnetsReady"
+	ConditionTypeLocalDNSReady          = "LocalDNSReady"


Unlike other existing conditions (which fetch certain things and wait for them to be available/ready), this one is just about static API validation, and we are likely to have more of this kind of validation going forward (for other fields as well). So it probably needs to be a single ValidationSucceeded condition, with multiple possible reasons for it to be false - like LocalDNSValiationFailed in this case. Could use AWS provider as an example (e.g. https://github.com/aws/karpenter-provider-aws/blob/main/pkg/controllers/nodeclass/validation.go)

…NodeCount

…alidation

saewoni marked this pull request as ready for review October 27, 2025 18:14

saewoni changed the title ~~[Karpenter CRD change] Add LocalDNSProfile to AKSNodeClassSpec~~ [CRD change] Add LocalDNSProfile to AKSNodeClassSpec Oct 27, 2025

saewoni requested a review from Copilot October 27, 2025 23:01

Copilot AI reviewed Oct 27, 2025

View reviewed changes

pkg/apis/v1alpha2/aksnodeclass.go Outdated Show resolved Hide resolved

saewoni requested a review from Copilot October 27, 2025 23:03

Copilot AI reviewed Oct 27, 2025

View reviewed changes

matthchr reviewed Oct 28, 2025

View reviewed changes

pkg/apis/v1alpha2/aksnodeclass.go Outdated Show resolved Hide resolved

matthchr reviewed Oct 30, 2025

View reviewed changes

pkg/apis/v1alpha2/aksnodeclass.go Outdated Show resolved Hide resolved

matthchr reviewed Oct 30, 2025

View reviewed changes

pkg/apis/v1alpha2/aksnodeclass.go Outdated Show resolved Hide resolved

pkg/providers/imagefamily/azlinux.go Show resolved Hide resolved

saewoni changed the title ~~[CRD change] Add LocalDNSProfile to AKSNodeClassSpec~~ [CRD change] Add LocalDNSProfile to AKSNodeClassSpec + Integration Tests Oct 31, 2025

saewoni changed the title ~~[CRD change] Add LocalDNSProfile to AKSNodeClassSpec + Integration Tests~~ [CRD change] Add LocalDNSProfile to AKSNodeClassSpec + NAP Integration Tests Oct 31, 2025

saewoni commented Nov 14, 2025

View reviewed changes

pkg/apis/v1beta1/aksnodeclass.go Outdated Show resolved Hide resolved

saewoni force-pushed the sakwa/localdns_for_NAP branch 2 times, most recently from 29c6b95 to 4a16ccc Compare November 26, 2025 03:56

saewoni requested a review from Copilot December 3, 2025 05:02

Copilot started reviewing on behalf of saewoni December 3, 2025 05:02 View session

Copilot finished reviewing on behalf of saewoni December 3, 2025 05:03

Copilot AI reviewed Dec 3, 2025

View reviewed changes

test/suites/integration/localdns_test.go Outdated Show resolved Hide resolved

yewmsft reviewed Dec 3, 2025

View reviewed changes

pkg/providers/instancetype/instancetypes.go Outdated Show resolved Hide resolved

yewmsft reviewed Dec 3, 2025

View reviewed changes

pkg/providers/labels/labels.go Outdated Show resolved Hide resolved

matthchr reviewed Dec 3, 2025

View reviewed changes

saewoni force-pushed the sakwa/localdns_for_NAP branch from 618d2ab to 6a42d21 Compare December 3, 2025 23:32

saewoni commented Dec 4, 2025

View reviewed changes

yewmsft previously approved these changes Dec 4, 2025

View reviewed changes

saewoni dismissed yewmsft’s stale review via 3a0d8a1 December 4, 2025 22:16

Azure deleted a comment from matthchr Dec 5, 2025

saewoni added 3 commits December 5, 2025 18:35

address ST1003: struct field CpuLimitInMilliCores should be CPULimitI…

84c8588

…nMilliCores (stylecheck)

move back fips where it was

07b404e

after make presubmit

b6d03f5

saewoni added 3 commits December 5, 2025 18:35

localdns does not support ubuntu 2004

f926faf

cap the overrides map to be at 10

d8e6a50

update cel test with maxproperties validation

21c7ab0

saewoni force-pushed the sakwa/localdns_for_NAP branch from f4c29d2 to 21c7ab0 Compare December 5, 2025 18:35

saewoni added 9 commits December 5, 2025 18:45

Revert "update cel test with maxproperties validation"

b3fb129

This reverts commit 21c7ab0.

Revert "cap the overrides map to be at 10"

0d8e546

This reverts commit d8e6a50.

remove regex validation for localdns

c49fd17

removed cel validation tests following removing regex validation test…

2f91d48

… on the zone names

moved regex validation to runtime validation, controllers/nodeclass/s…

f6501db

…tatus/localdns_test.go

add localdns to the controller's reconciler logic, set localdnsready …

2c7ef5e

…in pkg/test/nodeclass.go ApplyDefaultStatus function

remove sleep from the integration test, update the dns resolution for…

2e6d929

… inflate pod

comment out standard dns ips for now

a97cb85

add back sleep

de843f1

tallaxes reviewed Dec 6, 2025

View reviewed changes

saewoni added 16 commits December 6, 2025 00:07

change from env.ExpectCreatedNodeCount -> env.EventuallyExpectCreated…

3b2f1d4

…NodeCount

happy path works!

dc26633

update servestate from pointer to nonpointer

11177e5

add omitempty for MaxConcurrent

d3ff22d

update ForwardPolicy from pointer to non-pointer

89a6b01

update forwarddestination

4dce2e5

update mode from pointer to nonpointer

afe0a0e

update localdnsprotocol from pointer -> nonpointer

479615e

update querylogging

6bfafc2

convert overrides from map to list type, tests still need work

a7a609b

update tests after dnsoverrides change

3f7db6e

make changes for reconcile logic and remove cel validation for overrides

8ab2480

reduce CEL validation and increase localdns controller x reconciler v…

7eb99d4

…alidation

update the crd and update the hash test

3587919

include ISLocalDNSEnabled in p.instanceTypesCache.Get's key

1e35980

localdns -> validation controller

4cb5167

[CRD change] Add LocalDNSProfile to AKSNodeClassSpec + NAP Integration Tests #1233

Are you sure you want to change the base?

[CRD change] Add LocalDNSProfile to AKSNodeClassSpec + NAP Integration Tests #1233

Conversation

saewoni commented Oct 24, 2025 • edited by tallaxes Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

matthchr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthchr Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saewoni Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

tallaxes left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tallaxes Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tallaxes Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

saewoni commented Oct 24, 2025 •

edited by tallaxes

Loading

tallaxes left a comment •

edited

Loading

tallaxes Dec 5, 2025 •

edited

Loading