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.

Sign up for GitHub

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

Jump to bottom

NodeLocal DNSCache #8780

Merged
merged 5 commits into from
Apr 10, 2020
Merged

NodeLocal DNSCache #8780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f754cbd
NodeLocal DNSCache
mazzy89 Mar 30, 2020
cf57350
Improved validation logic
mazzy89 Mar 30, 2020
d18c88a
Add unit tests
mazzy89 Mar 30, 2020
d5019a6
Simplify the spec and templates a bit
Mar 31, 2020
f5a5db4
Verify bindata
Apr 10, 2020
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
20 changes: 20 additions & 0 deletions docs/cluster_spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,26 @@ spec:

**Note:** If you are upgrading to CoreDNS, kube-dns will be left in place and must be removed manually (you can scale the kube-dns and kube-dns-autoscaler deployments in the `kube-system` namespace to 0 as a starting point). The `kube-dns` Service itself should be left in place, as this retains the ClusterIP and eliminates the possibility of DNS outages in your cluster. If you would like to continue autoscaling, update the `kube-dns-autoscaler` Deployment container command for `--target=Deployment/kube-dns` to be `--target=Deployment/coredns`.

If you are using CoreDNS, you can enable NodeLocal DNSCache. It is used to improve improve the Cluster DNS performance by running a dns caching agent on cluster nodes as a DaemonSet.

```yaml
spec:
kubeDNS:
mazzy89 marked this conversation as resolved.
Show resolved Hide resolved
provider: CoreDNS
nodeLocalDNS:
enabled: true
```

If you are using kube-proxy in ipvs mode or Cilium as CNI, you have to set the nodeLocalDNS as ClusterDNS.

```yaml
spec:
kubelet:
clusterDNS: 169.254.20.10
masterKubelet:
clusterDNS: 169.254.20.10
```

### kubeControllerManager
This block contains configurations for the `controller-manager`.

Expand Down
14 changes: 14 additions & 0 deletions k8s/crds/kops.k8s.io_clusters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1611,6 +1611,20 @@ spec:
dns container in the cluster. Default 70m.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
nodeLocalDNS:
description: NodeLocalDNS specifies the configuration for the
node-local-dns addon
properties:
enabled:
description: Disable indicates we do not wish to run the node-local-dns
addon
type: boolean
localIP:
description: Local listen IP address. It can be any IP in
the 169.254.20.0/16 space or any other IP address that can
be guaranteed to not collide with any existing IP.
type: string
type: object
provider:
description: Provider indicates whether CoreDNS or kube-dns will
be the default service discovery.
Expand Down
10 changes: 10 additions & 0 deletions pkg/apis/kops/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,16 @@ type KubeDNSConfig struct {
CPURequest *resource.Quantity `json:"cpuRequest,omitempty"`
// MemoryLimit specifies the memory limit of each dns container in the cluster. Default 170m.
MemoryLimit *resource.Quantity `json:"memoryLimit,omitempty"`
// NodeLocalDNS specifies the configuration for the node-local-dns addon
NodeLocalDNS *NodeLocalDNSConfig `json:"nodeLocalDNS,omitempty"`
}

// NodeLocalDNSConfig are options of the node-local-dns
type NodeLocalDNSConfig struct {
// Disable indicates we do not wish to run the node-local-dns addon
Enabled bool `json:"enabled,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

I think we actually should make this Enabled *bool, because we want to differentiate between:

nodeLocalDNSConfig:
  enabled: false
  localIP: 10.0.0.1

and

nodeLocalDNSConfig:
  localIP: 10.0.0.1

Copy link
Member

Choose a reason for hiding this comment

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

I suppose this could be a change we do if we consider enabling this by default. If not, we don't really need to distinguish between the two now.

// Local listen IP address. It can be any IP in the 169.254.20.0/16 space or any other IP address that can be guaranteed to not collide with any existing IP.
LocalIP string `json:"localIP,omitempty"`
}

// ExternalDNSConfig are options of the dns-controller
Expand Down
10 changes: 10 additions & 0 deletions pkg/apis/kops/v1alpha2/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,16 @@ type KubeDNSConfig struct {
CPURequest *resource.Quantity `json:"cpuRequest,omitempty"`
// MemoryLimit specifies the memory limit of each dns container in the cluster. Default 170m.
MemoryLimit *resource.Quantity `json:"memoryLimit,omitempty"`
// NodeLocalDNS specifies the configuration for the node-local-dns addon
NodeLocalDNS *NodeLocalDNSConfig `json:"nodeLocalDNS,omitempty"`
}

// NodeLocalDNSConfig are options of the node-local-dns
type NodeLocalDNSConfig struct {
// Disable indicates we do not wish to run the node-local-dns addon
Enabled bool `json:"enabled,omitempty"`
// Local listen IP address. It can be any IP in the 169.254.20.0/16 space or any other IP address that can be guaranteed to not collide with any existing IP.
LocalIP string `json:"localIP,omitempty"`
}

// ExternalDNSConfig are options of the dns-controller
Expand Down
50 changes: 50 additions & 0 deletions pkg/apis/kops/v1alpha2/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 19 additions & 4 deletions pkg/apis/kops/validation/legacy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,14 +277,23 @@ func ValidateCluster(c *kops.Cluster, strict bool) field.ErrorList {
allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("kubeDNS", "serverIP"), fmt.Sprintf("ServiceClusterIPRange %q must contain the DNS Server IP %q", c.Spec.ServiceClusterIPRange, address)))
}
if !featureflag.ExperimentalClusterDNS.Enabled() {
if c.Spec.Kubelet != nil && c.Spec.Kubelet.ClusterDNS != c.Spec.KubeDNS.ServerIP {
allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("kubeDNS", "serverIP"), "Kubelet ClusterDNS did not match cluster kubeDNS.serverIP"))
if isExperimentalClusterDNS(c.Spec.Kubelet, c.Spec.KubeDNS) {
allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("kubelet", "clusterDNS"), "Kubelet ClusterDNS did not match cluster kubeDNS.serverIP or nodeLocalDNS.localIP"))
}
if c.Spec.MasterKubelet != nil && c.Spec.MasterKubelet.ClusterDNS != c.Spec.KubeDNS.ServerIP {
allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("kubeDNS", "serverIP"), "MasterKubelet ClusterDNS did not match cluster kubeDNS.serverIP"))
if isExperimentalClusterDNS(c.Spec.MasterKubelet, c.Spec.KubeDNS) {
allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("masterKubelet", "clusterDNS"), "MasterKubelet ClusterDNS did not match cluster kubeDNS.serverIP or nodeLocalDNS.localIP"))
}
}
}

// @ check that NodeLocalDNS addon is configured correctly
if c.Spec.KubeDNS.NodeLocalDNS != nil && c.Spec.KubeDNS.NodeLocalDNS.Enabled {
if c.Spec.KubeDNS.Provider != "CoreDNS" {
mazzy89 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually need the "raw" DNS provider to be CoreDNS? I didn't think it was required?

allErrs = append(allErrs, field.Forbidden(fieldSpec.Child("kubeDNS", "provider"), "KubeDNS provider must be set to CoreDNS if NodeLocalDNS addon is enabled"))
}

allErrs = append(allErrs, validateNodeLocalDNS(&c.Spec, fieldSpec.Child("spec"))...)
}
}

// @check the nameservers are valid
Expand Down Expand Up @@ -705,3 +714,9 @@ func validateKubelet(k *kops.KubeletConfigSpec, c *kops.Cluster, kubeletPath *fi
}
return allErrs
}

func isExperimentalClusterDNS(k *kops.KubeletConfigSpec, dns *kops.KubeDNSConfig) bool {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I'm not clear why this function is called "isExperimentalClusterDNS". Maybe isValidClusterDNS ?

Copy link
Member

Choose a reason for hiding this comment

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

It is just because when this one returns true, one needs the ExperimentalClusterDNS featureflag on. I am not really sure this feature flag makes sense. As in it doesn't really guard a feature.

One option would certainly be to remove the feature flag and only consider "valid" configurations.


return k != nil && k.ClusterDNS != dns.ServerIP && dns.NodeLocalDNS != nil && k.ClusterDNS != dns.NodeLocalDNS.LocalIP

}
24 changes: 24 additions & 0 deletions pkg/apis/kops/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,3 +646,27 @@ func validateRollingUpdate(rollingUpdate *kops.RollingUpdate, fldpath *field.Pat

return allErrs
}

func validateNodeLocalDNS(spec *kops.ClusterSpec, fldpath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if spec.KubeDNS.NodeLocalDNS.LocalIP != "" {
address := spec.KubeDNS.NodeLocalDNS.LocalIP
ip := net.ParseIP(address)
if ip == nil {
allErrs = append(allErrs, field.Invalid(fldpath.Child("kubeDNS", "nodeLocalDNS", "localIP"), address, "Cluster had an invalid kubeDNS.nodeLocalDNS.localIP"))
}
}

if (spec.KubeProxy != nil && spec.KubeProxy.ProxyMode == "ipvs") || (spec.Networking != nil && spec.Networking.Cilium != nil) {
if spec.Kubelet != nil && spec.Kubelet.ClusterDNS != spec.KubeDNS.NodeLocalDNS.LocalIP {
allErrs = append(allErrs, field.Forbidden(fldpath.Child("kubelet", "clusterDNS"), "Kubelet ClusterDNS must be set to the default IP address for LocalIP"))
}

if spec.MasterKubelet != nil && spec.MasterKubelet.ClusterDNS != spec.KubeDNS.NodeLocalDNS.LocalIP {
allErrs = append(allErrs, field.Forbidden(fldpath.Child("kubelet", "clusterDNS"), "MasterKubelet ClusterDNS must be set to the default IP address for LocalIP"))
}
}

return allErrs
}
85 changes: 85 additions & 0 deletions pkg/apis/kops/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,3 +534,88 @@ func Test_Validate_RollingUpdate(t *testing.T) {
func intStr(i intstr.IntOrString) *intstr.IntOrString {
return &i
}

func Test_Validate_NodeLocalDNS(t *testing.T) {
grid := []struct {
Input kops.ClusterSpec
ExpectedErrors []string
}{
{
Input: kops.ClusterSpec{
KubeProxy: &kops.KubeProxyConfig{
ProxyMode: "iptables",
},
KubeDNS: &kops.KubeDNSConfig{
Provider: "CoreDNS",
NodeLocalDNS: &kops.NodeLocalDNSConfig{
Enabled: true,
},
},
},
ExpectedErrors: []string{},
},
{
Input: kops.ClusterSpec{
Kubelet: &kops.KubeletConfigSpec{
ClusterDNS: "100.64.0.10",
},
KubeProxy: &kops.KubeProxyConfig{
ProxyMode: "ipvs",
},
KubeDNS: &kops.KubeDNSConfig{
Provider: "CoreDNS",
NodeLocalDNS: &kops.NodeLocalDNSConfig{
Enabled: true,
},
},
},
ExpectedErrors: []string{"Forbidden::spec.kubelet.clusterDNS"},
},
{
Input: kops.ClusterSpec{
Kubelet: &kops.KubeletConfigSpec{
ClusterDNS: "100.64.0.10",
},
KubeProxy: &kops.KubeProxyConfig{
ProxyMode: "ipvs",
},
KubeDNS: &kops.KubeDNSConfig{
Provider: "CoreDNS",
NodeLocalDNS: &kops.NodeLocalDNSConfig{
Enabled: true,
},
},
Networking: &kops.NetworkingSpec{
Cilium: &kops.CiliumNetworkingSpec{},
},
},
ExpectedErrors: []string{"Forbidden::spec.kubelet.clusterDNS"},
},
{
Input: kops.ClusterSpec{
Kubelet: &kops.KubeletConfigSpec{
ClusterDNS: "169.254.20.10",
},
KubeProxy: &kops.KubeProxyConfig{
ProxyMode: "iptables",
},
KubeDNS: &kops.KubeDNSConfig{
Provider: "CoreDNS",
NodeLocalDNS: &kops.NodeLocalDNSConfig{
Enabled: true,
LocalIP: "169.254.20.10",
},
},
Networking: &kops.NetworkingSpec{
Cilium: &kops.CiliumNetworkingSpec{},
},
},
ExpectedErrors: []string{},
},
}

for _, g := range grid {
errs := validateNodeLocalDNS(&g.Input, field.NewPath("spec"))
testErrors(t, g.Input, errs, g.ExpectedErrors)
}
}
21 changes: 21 additions & 0 deletions pkg/apis/kops/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions pkg/model/components/kubedns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,13 @@ func (b *KubeDnsOptionsBuilder) BuildOptions(o interface{}) error {
clusterSpec.KubeDNS.MemoryLimit = &defaultMemoryLimit
}

NodeLocalDNS := clusterSpec.KubeDNS.NodeLocalDNS
if NodeLocalDNS == nil {
mazzy89 marked this conversation as resolved.
Show resolved Hide resolved
NodeLocalDNS = &kops.NodeLocalDNSConfig{}
NodeLocalDNS.Enabled = false
} else if NodeLocalDNS.Enabled && NodeLocalDNS.LocalIP == "" {
NodeLocalDNS.LocalIP = "169.254.20.10"
mazzy89 marked this conversation as resolved.
Show resolved Hide resolved
}

return nil
}
Loading