Adding `resyncperiod` to Corefile (#205)

* Removing old unused inline k8s API code and tests.
* Adding parsing implementation for `resyncperiod` keyword from Corefile.
* Adding tests for parsing `resyncperiod` keyword from Corefile.
8 Updating README.md and conf/k8sCorefile.

conf/k8sCorefile

@@ -2,6 +2,9 @@
 .:53 {
     # use kubernetes middleware for domain "coredns.local"
     kubernetes coredns.local {
+		# Kubernetes data API resync period
+		# Example values: 60s, 5m, 1h
+		resyncperiod 5m
         # Use url for k8s API endpoint
         endpoint http://localhost:8080
         # Assemble k8s record names with the template
@@ -11,7 +14,5 @@
     }
     # Perform DNS response caching for the coredns.local zone
     # Cache timeout is provided by the integer in seconds
-    # This works for the kubernetes middleware.)
-    #cache 20 coredns.local
-    #cache 160 coredns.local
+    #cache 180 coredns.local
 }

core/setup/kubernetes.go

@@ -2,6 +2,7 @@ package setup
 
 import (
 	"errors"
+	"fmt"
 	"log"
 	"strings"
 	"time"
@@ -27,7 +28,6 @@ func Kubernetes(c *Controller) (middleware.Middleware, error) {
 	if err != nil {
 		return nil, err
 	}
-	log.Printf("[debug] after parse and start KubeCache, APIconn is: %v", kubernetes.APIConn)
 
 	return func(next middleware.Handler) middleware.Handler {
 		kubernetes.Next = next
@@ -51,7 +51,6 @@ func kubernetesParse(c *Controller) (kubernetes.Kubernetes, error) {
 		if c.Val() == "kubernetes" {
 			zones := c.RemainingArgs()
 
-			log.Printf("[debug] Zones: %v", zones)
 			if len(zones) == 0 {
 				k8s.Zones = c.ServerBlockHosts
 				log.Printf("[debug] Zones(from ServerBlockHosts): %v", zones)
@@ -97,6 +96,19 @@ func kubernetesParse(c *Controller) (kubernetes.Kubernetes, error) {
 						log.Printf("[debug] 'endpoint' keyword provided without any endpoint url value.")
 						return kubernetes.Kubernetes{}, c.ArgErr()
 					}
+				case "resyncperiod":
+					args := c.RemainingArgs()
+					if len(args) != 0 {
+						k8s.ResyncPeriod, err = time.ParseDuration(args[0])
+						if err != nil {
+							err = errors.New(fmt.Sprintf("Unable to parse resync duration value. Value provided was '%v'. Example valid values: '15s', '5m', '1h'. Error was: %v", args[0], err))
+							log.Printf("[ERROR] %v", err)
+							return kubernetes.Kubernetes{}, err
+						}
+					} else {
+						log.Printf("[debug] 'resyncperiod' keyword provided without any duration value.")
+						return kubernetes.Kubernetes{}, c.ArgErr()
+					}
 				}
 			}
 			return k8s, nil

core/setup/kubernetes_test.go

@@ -3,17 +3,19 @@ package setup
 import (
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestKubernetesParse(t *testing.T) {
 	tests := []struct {
-		description        string
-		input              string
-		shouldErr          bool
-		expectedErrContent string // substring from the expected error. Empty for positive cases.
-		expectedZoneCount  int    // expected count of defined zones.
-		expectedNTValid    bool   // NameTemplate to be initialized and valid
-		expectedNSCount    int    // expected count of namespaces.
+		description          string        // Human-facing description of test case
+		input                string        // Corefile data as string
+		shouldErr            bool          // true if test case is exected to produce an error.
+		expectedErrContent   string        // substring from the expected error. Empty for positive cases.
+		expectedZoneCount    int           // expected count of defined zones.
+		expectedNTValid      bool          // NameTemplate to be initialized and valid
+		expectedNSCount      int           // expected count of namespaces.
+		expectedResyncPeriod time.Duration // expected resync period value
 	}{
 		// positive
 		{
@@ -24,6 +26,7 @@ func TestKubernetesParse(t *testing.T) {
 			1,
 			true,
 			0,
+			defaultResyncPeriod,
 		},
 		{
 			"kubernetes keyword with multiple zones",
@@ -33,6 +36,7 @@ func TestKubernetesParse(t *testing.T) {
 			2,
 			true,
 			0,
+			defaultResyncPeriod,
 		},
 		{
 			"kubernetes keyword with zone and empty braces",
@@ -43,6 +47,7 @@ func TestKubernetesParse(t *testing.T) {
 			1,
 			true,
 			0,
+			defaultResyncPeriod,
 		},
 		{
 			"endpoint keyword with url",
@@ -54,6 +59,7 @@ func TestKubernetesParse(t *testing.T) {
 			1,
 			true,
 			0,
+			defaultResyncPeriod,
 		},
 		{
 			"template keyword with valid template",
@@ -65,6 +71,7 @@ func TestKubernetesParse(t *testing.T) {
 			1,
 			true,
 			0,
+			defaultResyncPeriod,
 		},
 		{
 			"namespaces keyword with one namespace",
@@ -76,6 +83,7 @@ func TestKubernetesParse(t *testing.T) {
 			1,
 			true,
 			1,
+			defaultResyncPeriod,
 		},
 		{
 			"namespaces keyword with multiple namespaces",
@@ -87,10 +95,36 @@ func TestKubernetesParse(t *testing.T) {
 			1,
 			true,
 			2,
+			defaultResyncPeriod,
+		},
+		{
+			"resync period in seconds",
+			`kubernetes coredns.local {
+    resyncperiod 30s
+}`,
+			false,
+			"",
+			1,
+			true,
+			0,
+			30 * time.Second,
+		},
+		{
+			"resync period in minutes",
+			`kubernetes coredns.local {
+    resyncperiod 15m
+}`,
+			false,
+			"",
+			1,
+			true,
+			0,
+			15 * time.Minute,
 		},
 		{
 			"fully specified valid config",
 			`kubernetes coredns.local test.local {
+    resyncperiod 15m
 	endpoint http://localhost:8080
 	template {service}.{namespace}.{zone}
 	namespaces demo test
@@ -100,6 +134,7 @@ func TestKubernetesParse(t *testing.T) {
 			2,
 			true,
 			2,
+			15 * time.Minute,
 		},
 		// negative
 		{
@@ -110,6 +145,7 @@ func TestKubernetesParse(t *testing.T) {
 			-1,
 			false,
 			-1,
+			defaultResyncPeriod,
 		},
 		{
 			"kubernetes keyword without a zone",
@@ -119,6 +155,7 @@ func TestKubernetesParse(t *testing.T) {
 			-1,
 			true,
 			0,
+			defaultResyncPeriod,
 		},
 		{
 			"endpoint keyword without an endpoint value",
@@ -130,6 +167,7 @@ func TestKubernetesParse(t *testing.T) {
 			-1,
 			true,
 			-1,
+			defaultResyncPeriod,
 		},
 		{
 			"template keyword without a template value",
@@ -141,6 +179,7 @@ func TestKubernetesParse(t *testing.T) {
 			-1,
 			false,
 			0,
+			defaultResyncPeriod,
 		},
 		{
 			"template keyword with an invalid template value",
@@ -152,6 +191,7 @@ func TestKubernetesParse(t *testing.T) {
 			-1,
 			false,
 			0,
+			defaultResyncPeriod,
 		},
 		{
 			"namespace keyword without a namespace value",
@@ -163,6 +203,43 @@ func TestKubernetesParse(t *testing.T) {
 			-1,
 			true,
 			-1,
+			defaultResyncPeriod,
+		},
+		{
+			"resyncperiod keyword without a duration value",
+			`kubernetes coredns.local {
+    resyncperiod
+}`,
+			true,
+			"Wrong argument count or unexpected line ending after 'resyncperiod'",
+			-1,
+			true,
+			0,
+			0 * time.Minute,
+		},
+		{
+			"resync period no units",
+			`kubernetes coredns.local {
+    resyncperiod 15
+}`,
+			true,
+			"Unable to parse resync duration value. Value provided was ",
+			-1,
+			true,
+			0,
+			0 * time.Second,
+		},
+		{
+			"resync period invalid",
+			`kubernetes coredns.local {
+    resyncperiod abc
+}`,
+			true,
+			"Unable to parse resync duration value. Value provided was ",
+			-1,
+			true,
+			0,
+			0 * time.Second,
 		},
 	}
 
@@ -218,8 +295,12 @@ func TestKubernetesParse(t *testing.T) {
 		foundNSCount := len(k8sController.Namespaces)
 		if foundNSCount != test.expectedNSCount {
 			t.Errorf("Test %d: Expected kubernetes controller to be initialized with %d namespaces. Instead found %d namespaces: '%v' for input '%s'", i, test.expectedNSCount, foundNSCount, k8sController.Namespaces, test.input)
-			t.Logf("k8sController is: %v", k8sController)
-			t.Logf("k8sController.Namespaces is: %v", k8sController.Namespaces)
+		}
+
+		//    ResyncPeriod
+		foundResyncPeriod := k8sController.ResyncPeriod
+		if foundResyncPeriod != test.expectedResyncPeriod {
+			t.Errorf("Test %d: Expected kubernetes controller to be initialized with resync period '%s'. Instead found period '%s' for input '%s'", test.expectedResyncPeriod, foundResyncPeriod, test.input)
 		}
 	}
 }

middleware/kubernetes/README.md

@@ -35,6 +35,9 @@ This is the default kubernetes setup, with everything specified in full:
 .:53 {
     # use kubernetes middleware for domain "coredns.local"
     kubernetes coredns.local {
+        # Kubernetes data API resync period
+        # Example values: 60s, 5m, 1h
+        resyncperiod 5m
         # Use url for k8s API endpoint
         endpoint http://localhost:8080
         # Assemble k8s record names with the template
@@ -42,10 +45,17 @@ This is the default kubernetes setup, with everything specified in full:
         # Only expose the k8s namespace "demo"
         namespaces demo
     }
-#    cache 160 coredns.local
+    # Perform DNS response caching for the coredns.local zone
+    # Cache timeout is provided by the integer in seconds
+    #cache 180 coredns.local
 }
 ~~~
 
+Notes:
+* If the `namespaces` keyword is omitted, all kubernetes namespaces are exposed.
+* If the `template` keyword is omitted, the default template of "{service}.{namespace}.{zone}" is used.
+* If the `resyncperiod` keyword is omitted, the default resync period is 5 minutes.
+
 ### Basic Setup
 
 #### Launch Kubernetes
@@ -305,14 +315,9 @@ TBD:
 	* Performance
 		* Improve lookup to reduce size of query result obtained from k8s API.
 		  (namespace-based?, other ideas?)
-		* Caching/notification of k8s API dataset. (See aledbf fork for
-		  implementation ideas.)
-		* DNS response caching is good, but we should also cache at the http query 
-		  level as well. (Take a look at https://github.com/patrickmn/go-cache as 
-		  a potential expiring cache implementation for the http API queries.)
 * Additional features:
 	* Reverse IN-ADDR entries for services. (Is there any value in supporting 
-	  reverse lookup records?)
+	  reverse lookup records?) (need tests, functionality should work based on @aledbf's code.)
 	* How to support label specification in Corefile to allow use of labels to 
 	  indicate zone? (Is this even useful?) For example, the following
 	  configuration exposes all services labeled for the "staging" environment
@@ -333,14 +338,6 @@ TBD:
 	  flattening to lower case and mapping of non-DNS characters to DNS characters
 	  in a standard way.)
 	* Expose arbitrary kubernetes repository data as TXT records?
-	* (done) ~~Support custom user-provided templates for k8s names. A string provided
-	  in the middleware configuration like `{service}.{namespace}.{type}` defines
-	  the template of how to construct record names for the zone. This example
-	  would produce `myservice.mynamespace.svc.cluster.local`. (Basic template
-	  implemented. Need to slice zone out of current template implementation.)~~
-	* (done) ~~Implement namespace filtering to different zones. That is, zone "a.b"
-	  publishes services from namespace "foo", and zone "x.y" publishes services
-	  from namespaces "bar" and "baz". (Basic version implemented -- need test cases.)~~
 * DNS Correctness
 	* Do we need to generate synthetic zone records for namespaces?
 	* Do we need to generate synthetic zone records for the skydns synthetic zones?
@@ -352,7 +349,3 @@ TBD:
 	  pre-loaded k8s API cache. With and without CoreDNS response caching.
     * Try to get rid of kubernetes launch scripts by moving operations into
       .travis.yml file.
-	* ~~Implement test cases for http data parsing using dependency injection
-	  for http get operations.~~
-    * ~~Automate integration testing with kubernetes. (k8s launch and service
-      start-up automation is in middleware/kubernetes/tests)~~

middleware/kubernetes/controller.go

@@ -129,7 +129,7 @@ func (dns *dnsController) Stop() error {
 
 // Run starts the controller.
 func (dns *dnsController) Run() {
-	log.Println("[debug] starting coredns controller")
+	log.Println("[debug] Starting k8s notification controllers")
 
 	go dns.endpController.Run(dns.stopCh)
 	go dns.svcController.Run(dns.stopCh)

middleware/kubernetes/k8sclient/dataobjects.go

-package k8sclient
-
-import (
-	"encoding/json"
-	"net/http"
-)
-
-// getK8sAPIResponse wraps the http.Get(url) function to provide dependency
-// injection for unit testing.
-var getK8sAPIResponse = func(url string) (resp *http.Response, err error) {
-	resp, err = http.Get(url)
-	return resp, err
-}
-
-func parseJson(url string, target interface{}) error {
-	r, err := getK8sAPIResponse(url)
-	if err != nil {
-		return err
-	}
-	defer r.Body.Close()
-
-	return json.NewDecoder(r.Body).Decode(target)
-}
-
-// Kubernetes Resource List
-type ResourceList struct {
-	Kind         string     `json:"kind"`
-	GroupVersion string     `json:"groupVersion"`
-	Resources    []resource `json:"resources"`
-}
-
-type resource struct {
-	Name       string `json:"name"`
-	Namespaced bool   `json:"namespaced"`
-	Kind       string `json:"kind"`
-}
-
-// Kubernetes NamespaceList
-type NamespaceList struct {
-	Kind       string          `json:"kind"`
-	APIVersion string          `json:"apiVersion"`
-	Metadata   apiListMetadata `json:"metadata"`
-	Items      []nsItems       `json:"items"`
-}
-
-type apiListMetadata struct {
-	SelfLink        string `json:"selfLink"`
-	ResourceVersion string `json:"resourceVersion"`
-}
-
-type nsItems struct {
-	Metadata nsMetadata `json:"metadata"`
-	Spec     nsSpec     `json:"spec"`
-	Status   nsStatus   `json:"status"`
-}
-
-type nsMetadata struct {
-	Name              string `json:"name"`
-	SelfLink          string `json:"selfLink"`
-	Uid               string `json:"uid"`
-	ResourceVersion   string `json:"resourceVersion"`
-	CreationTimestamp string `json:"creationTimestamp"`
-}
-
-type nsSpec struct {
-	Finalizers []string `json:"finalizers"`
-}
-
-type nsStatus struct {
-	Phase string `json:"phase"`
-}
-
-// Kubernetes ServiceList
-type ServiceList struct {
-	Kind       string          `json:"kind"`
-	APIVersion string          `json:"apiVersion"`
-	Metadata   apiListMetadata `json:"metadata"`
-	Items      []ServiceItem   `json:"items"`
-}
-
-type ServiceItem struct {
-	Metadata serviceMetadata `json:"metadata"`
-	Spec     serviceSpec     `json:"spec"`
-	//    Status     serviceStatus    `json:"status"`
-}
-
-type serviceMetadata struct {
-	Name              string `json:"name"`
-	Namespace         string `json:"namespace"`
-	SelfLink          string `json:"selfLink"`
-	Uid               string `json:"uid"`
-	ResourceVersion   string `json:"resourceVersion"`
-	CreationTimestamp string `json:"creationTimestamp"`
-	// labels
-}
-
-type serviceSpec struct {
-	Ports           []servicePort `json:"ports"`
-	ClusterIP       string        `json:"clusterIP"`
-	Type            string        `json:"type"`
-	SessionAffinity string        `json:"sessionAffinity"`
-}
-
-type servicePort struct {
-	Name       string `json:"name"`
-	Protocol   string `json:"protocol"`
-	Port       int    `json:"port"`
-	TargetPort int    `json:"targetPort"`
-}
-
-type serviceStatus struct {
-	LoadBalancer string `json:"loadBalancer"`
-}

middleware/kubernetes/k8sclient/k8sclient.go

-package k8sclient
-
-import (
-	"errors"
-	"log"
-	"net/url"
-	"strings"
-)
-
-// API strings
-const (
-	apiBase       = "/api/v1"
-	apiNamespaces = "/namespaces"
-	apiServices   = "/services"
-)
-
-// Defaults
-const (
-	defaultBaseURL = "http://localhost:8080"
-)
-
-type K8sConnector struct {
-	baseURL string
-}
-
-func (c *K8sConnector) SetBaseURL(u string) error {
-	url, error := url.Parse(u)
-
-	if error != nil {
-		return error
-	}
-
-	if !url.IsAbs() {
-		return errors.New("k8sclient: Kubernetes endpoint url must be an absolute URL")
-	}
-
-	c.baseURL = url.String()
-	return nil
-}
-
-func (c *K8sConnector) GetBaseURL() string {
-	return c.baseURL
-}
-
-// URL constructor separated from code to support dependency injection
-// for unit tests.
-var makeURL = func(parts []string) string {
-	return strings.Join(parts, "")
-}
-
-func (c *K8sConnector) GetResourceList() (*ResourceList, error) {
-	resources := new(ResourceList)
-
-	url := makeURL([]string{c.baseURL, apiBase})
-	err := parseJson(url, resources)
-	// TODO: handle no response from k8s
-	if err != nil {
-		log.Printf("[ERROR] Response from kubernetes API for GetResourceList() is: %v\n", err)
-		return nil, err
-	}
-
-	return resources, nil
-}
-
-func (c *K8sConnector) GetNamespaceList() (*NamespaceList, error) {
-	namespaces := new(NamespaceList)
-
-	url := makeURL([]string{c.baseURL, apiBase, apiNamespaces})
-	err := parseJson(url, namespaces)
-	if err != nil {
-		log.Printf("[ERROR] Response from kubernetes API for GetNamespaceList() is: %v\n", err)
-		return nil, err
-	}
-
-	return namespaces, nil
-}
-
-func (c *K8sConnector) GetServiceList() (*ServiceList, error) {
-	services := new(ServiceList)
-
-	url := makeURL([]string{c.baseURL, apiBase, apiServices})
-	err := parseJson(url, services)
-	// TODO: handle no response from k8s
-	if err != nil {
-		log.Printf("[ERROR] Response from kubernetes API for GetServiceList() is: %v\n", err)
-		return nil, err
-	}
-
-	return services, nil
-}
-
-// GetServicesByNamespace returns a map of
-// namespacename :: [ kubernetesServiceItem ]
-func (c *K8sConnector) GetServicesByNamespace() (map[string][]ServiceItem, error) {
-
-	items := make(map[string][]ServiceItem)
-
-	k8sServiceList, err := c.GetServiceList()
-
-	if err != nil {
-		log.Printf("[ERROR] Getting service list produced error: %v", err)
-		return nil, err
-	}
-
-	// TODO: handle no response from k8s
-	if k8sServiceList == nil {
-		return nil, nil
-	}
-
-	k8sItemList := k8sServiceList.Items
-
-	for _, i := range k8sItemList {
-		namespace := i.Metadata.Namespace
-		items[namespace] = append(items[namespace], i)
-	}
-
-	return items, nil
-}
-
-func NewK8sConnector(baseURL string) *K8sConnector {
-	k := new(K8sConnector)
-
-	if baseURL == "" {
-		baseURL = defaultBaseURL
-	}
-
-	err := k.SetBaseURL(baseURL)
-	if err != nil {
-		return nil
-	}
-
-	return k
-}

middleware/kubernetes/kubernetes.go

@@ -20,10 +20,6 @@ import (
 	clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
 )
 
-const (
-	defaultResyncPeriod = 5 * time.Minute
-)
-
 type Kubernetes struct {
 	Next         middleware.Handler
 	Zones        []string
@@ -37,7 +33,7 @@ type Kubernetes struct {
 
 func (g *Kubernetes) StartKubeCache() error {
 	// For a custom api server or running outside a k8s cluster
-	// set URL in env.KUBERNETES_MASTER
+	// set URL in env.KUBERNETES_MASTER or set endpoint in Corefile
 	loadingRules := clientcmd.NewDefaultClientConfigLoadingRules()
 	overrides := &clientcmd.ConfigOverrides{}
 	if len(g.APIEndpoint) > 0 {
@@ -55,6 +51,7 @@ func (g *Kubernetes) StartKubeCache() error {
 		log.Printf("[ERROR] Failed to create kubernetes notification controller: %v", err)
 		return err
 	}
+	log.Printf("[debug] Starting kubernetes middleware with k8s API resync period: %s", g.ResyncPeriod)
 	g.APIConn = newdnsController(kubeClient, g.ResyncPeriod)
 
 	go g.APIConn.Run()