From 9d5b8cd13d541cc3d2b7598ab255643d6298ba9d Mon Sep 17 00:00:00 2001 From: Mat Lowery Date: Thu, 29 Jul 2021 21:27:25 -0600 Subject: [PATCH] kubernetes: Improve namespace usage (#4767) * Use GetByKey instead of List in GetNamespaceByName. * Add ToNamespace to reduce memory for namespace cache. Signed-off-by: Mat Lowery --- plugin/k8s_external/external_test.go | 9 ++-- plugin/kubernetes/controller.go | 33 ++++++++------- plugin/kubernetes/external_test.go | 9 ++-- plugin/kubernetes/handler_test.go | 10 ++--- plugin/kubernetes/kubernetes_test.go | 8 ++-- plugin/kubernetes/namespace.go | 4 +- plugin/kubernetes/ns_test.go | 5 ++- plugin/kubernetes/object/namespace.go | 61 +++++++++++++++++++++++++++ plugin/kubernetes/reverse_test.go | 8 ++-- 9 files changed, 100 insertions(+), 47 deletions(-) create mode 100644 plugin/kubernetes/object/namespace.go diff --git a/plugin/k8s_external/external_test.go b/plugin/k8s_external/external_test.go index e51fc6895..02266a71a 100644 --- a/plugin/k8s_external/external_test.go +++ b/plugin/k8s_external/external_test.go @@ -12,7 +12,6 @@ import ( "github.com/miekg/dns" api "k8s.io/api/core/v1" - meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestExternal(t *testing.T) { @@ -176,11 +175,9 @@ func (external) GetNodeByName(ctx context.Context, name string) (*api.Node, erro func (external) SvcIndex(s string) []*object.Service { return svcIndexExternal[s] } func (external) PodIndex(string) []*object.Pod { return nil } -func (external) GetNamespaceByName(name string) (*api.Namespace, error) { - return &api.Namespace{ - ObjectMeta: meta.ObjectMeta{ - Name: name, - }, +func (external) GetNamespaceByName(name string) (*object.Namespace, error) { + return &object.Namespace{ + Name: name, }, nil } diff --git a/plugin/kubernetes/controller.go b/plugin/kubernetes/controller.go index 7155c320e..f3dbff454 100644 --- a/plugin/kubernetes/controller.go +++ b/plugin/kubernetes/controller.go @@ -39,7 +39,7 @@ type dnsController interface { EpIndexReverse(string) []*object.Endpoints GetNodeByName(context.Context, string) (*api.Node, error) - GetNamespaceByName(string) (*api.Namespace, error) + GetNamespaceByName(string) (*object.Namespace, error) Run() HasSynced() bool @@ -150,14 +150,16 @@ func newdnsController(ctx context.Context, kubeClient kubernetes.Interface, opts dns.epLock.Unlock() } - dns.nsLister, dns.nsController = cache.NewInformer( + dns.nsLister, dns.nsController = object.NewIndexerInformer( &cache.ListWatch{ ListFunc: namespaceListFunc(ctx, dns.client, dns.namespaceSelector), WatchFunc: namespaceWatchFunc(ctx, dns.client, dns.namespaceSelector), }, &api.Namespace{}, - defaultResyncPeriod, - cache.ResourceEventHandlerFuncs{}) + cache.ResourceEventHandlerFuncs{}, + cache.Indexers{}, + object.DefaultProcessor(object.ToNamespace, nil), + ) return &dns } @@ -539,18 +541,19 @@ func (dns *dnsControl) GetNodeByName(ctx context.Context, name string) (*api.Nod } // GetNamespaceByName returns the namespace by name. If nothing is found an error is returned. -func (dns *dnsControl) GetNamespaceByName(name string) (*api.Namespace, error) { - os := dns.nsLister.List() - for _, o := range os { - ns, ok := o.(*api.Namespace) - if !ok { - continue - } - if name == ns.ObjectMeta.Name { - return ns, nil - } +func (dns *dnsControl) GetNamespaceByName(name string) (*object.Namespace, error) { + o, exists, err := dns.nsLister.GetByKey(name) + if err != nil { + return nil, err } - return nil, fmt.Errorf("namespace not found") + if !exists { + return nil, fmt.Errorf("namespace not found") + } + ns, ok := o.(*object.Namespace) + if !ok { + return nil, fmt.Errorf("found key but not namespace") + } + return ns, nil } func (dns *dnsControl) Add(obj interface{}) { dns.updateModified() } diff --git a/plugin/kubernetes/external_test.go b/plugin/kubernetes/external_test.go index 560983e4c..9444a999e 100644 --- a/plugin/kubernetes/external_test.go +++ b/plugin/kubernetes/external_test.go @@ -11,7 +11,6 @@ import ( "github.com/miekg/dns" api "k8s.io/api/core/v1" - meta "k8s.io/apimachinery/pkg/apis/meta/v1" ) var extCases = []struct { @@ -91,11 +90,9 @@ func (external) GetNodeByName(ctx context.Context, name string) (*api.Node, erro func (external) SvcIndex(s string) []*object.Service { return svcIndexExternal[s] } func (external) PodIndex(string) []*object.Pod { return nil } -func (external) GetNamespaceByName(name string) (*api.Namespace, error) { - return &api.Namespace{ - ObjectMeta: meta.ObjectMeta{ - Name: name, - }, +func (external) GetNamespaceByName(name string) (*object.Namespace, error) { + return &object.Namespace{ + Name: name, }, nil } diff --git a/plugin/kubernetes/handler_test.go b/plugin/kubernetes/handler_test.go index 19eed2d50..709d831b3 100644 --- a/plugin/kubernetes/handler_test.go +++ b/plugin/kubernetes/handler_test.go @@ -782,16 +782,14 @@ func (APIConnServeTest) GetNodeByName(ctx context.Context, name string) (*api.No }, nil } -func (APIConnServeTest) GetNamespaceByName(name string) (*api.Namespace, error) { +func (APIConnServeTest) GetNamespaceByName(name string) (*object.Namespace, error) { if name == "pod-nons" { // handler_pod_verified_test.go uses this for non-existent namespace. - return &api.Namespace{}, nil + return nil, fmt.Errorf("namespace not found") } if name == "nsnoexist" { return nil, fmt.Errorf("namespace not found") } - return &api.Namespace{ - ObjectMeta: meta.ObjectMeta{ - Name: name, - }, + return &object.Namespace{ + Name: name, }, nil } diff --git a/plugin/kubernetes/kubernetes_test.go b/plugin/kubernetes/kubernetes_test.go index a342d9a23..0d5e6b637 100644 --- a/plugin/kubernetes/kubernetes_test.go +++ b/plugin/kubernetes/kubernetes_test.go @@ -253,11 +253,9 @@ func (APIConnServiceTest) GetNodeByName(ctx context.Context, name string) (*api. }, nil } -func (APIConnServiceTest) GetNamespaceByName(name string) (*api.Namespace, error) { - return &api.Namespace{ - ObjectMeta: meta.ObjectMeta{ - Name: name, - }, +func (APIConnServiceTest) GetNamespaceByName(name string) (*object.Namespace, error) { + return &object.Namespace{ + Name: name, }, nil } diff --git a/plugin/kubernetes/namespace.go b/plugin/kubernetes/namespace.go index 6eab13867..4dcc3afa4 100644 --- a/plugin/kubernetes/namespace.go +++ b/plugin/kubernetes/namespace.go @@ -5,11 +5,11 @@ package kubernetes // Returns true even for namespaces not exposed by plugin configuration, // see namespaceExposed. func (k *Kubernetes) filteredNamespaceExists(namespace string) bool { - ns, err := k.APIConn.GetNamespaceByName(namespace) + _, err := k.APIConn.GetNamespaceByName(namespace) if err != nil { return false } - return ns.ObjectMeta.Name == namespace + return true } // configuredNamespace returns true when the namespace is exposed through the plugin diff --git a/plugin/kubernetes/ns_test.go b/plugin/kubernetes/ns_test.go index 6370675cd..c952d66f0 100644 --- a/plugin/kubernetes/ns_test.go +++ b/plugin/kubernetes/ns_test.go @@ -2,6 +2,7 @@ package kubernetes import ( "context" + "fmt" "net" "testing" @@ -91,8 +92,8 @@ func (APIConnTest) EpIndexReverse(ip string) []*object.Endpoints { func (APIConnTest) GetNodeByName(ctx context.Context, name string) (*api.Node, error) { return &api.Node{}, nil } -func (APIConnTest) GetNamespaceByName(name string) (*api.Namespace, error) { - return &api.Namespace{}, nil +func (APIConnTest) GetNamespaceByName(name string) (*object.Namespace, error) { + return nil, fmt.Errorf("namespace not found") } func TestNsAddrs(t *testing.T) { diff --git a/plugin/kubernetes/object/namespace.go b/plugin/kubernetes/object/namespace.go new file mode 100644 index 000000000..b800cbb77 --- /dev/null +++ b/plugin/kubernetes/object/namespace.go @@ -0,0 +1,61 @@ +package object + +import ( + "fmt" + + api "k8s.io/api/core/v1" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +// Namespace is a stripped down api.Namespace with only the items we need for CoreDNS. +type Namespace struct { + // Don't add new fields to this struct without talking to the CoreDNS maintainers. + Version string + Name string + + *Empty +} + +// ToNamespace returns a function that converts an api.Namespace to a *Namespace. +func ToNamespace(obj meta.Object) (meta.Object, error) { + ns, ok := obj.(*api.Namespace) + if !ok { + return nil, fmt.Errorf("unexpected object %v", obj) + } + n := &Namespace{ + Version: ns.GetResourceVersion(), + Name: ns.GetName(), + } + *ns = api.Namespace{} + return n, nil +} + +var _ runtime.Object = &Namespace{} + +// DeepCopyObject implements the ObjectKind interface. +func (n *Namespace) DeepCopyObject() runtime.Object { + n1 := &Namespace{ + Version: n.Version, + Name: n.Name, + } + return n1 +} + +// GetNamespace implements the metav1.Object interface. +func (n *Namespace) GetNamespace() string { return "" } + +// SetNamespace implements the metav1.Object interface. +func (n *Namespace) SetNamespace(namespace string) {} + +// GetName implements the metav1.Object interface. +func (n *Namespace) GetName() string { return n.Name } + +// SetName implements the metav1.Object interface. +func (n *Namespace) SetName(name string) {} + +// GetResourceVersion implements the metav1.Object interface. +func (n *Namespace) GetResourceVersion() string { return n.Version } + +// SetResourceVersion implements the metav1.Object interface. +func (n *Namespace) SetResourceVersion(version string) {} diff --git a/plugin/kubernetes/reverse_test.go b/plugin/kubernetes/reverse_test.go index 2e888f064..7ceee6590 100644 --- a/plugin/kubernetes/reverse_test.go +++ b/plugin/kubernetes/reverse_test.go @@ -142,11 +142,9 @@ func (APIConnReverseTest) GetNodeByName(ctx context.Context, name string) (*api. }, nil } -func (APIConnReverseTest) GetNamespaceByName(name string) (*api.Namespace, error) { - return &api.Namespace{ - ObjectMeta: meta.ObjectMeta{ - Name: name, - }, +func (APIConnReverseTest) GetNamespaceByName(name string) (*object.Namespace, error) { + return &object.Namespace{ + Name: name, }, nil }