Merge pull request #77 from pravisankar/move-spec-fields

Move clusterIP and clusterDomain spec fields to status

assets/cluster-dns-cr.yaml

@@ -6,6 +6,3 @@ metadata:
   # Ensure that only the operator can delete the default cluster dns object.
   finalizers:
   - dns.openshift.io/default-cluster-dns
-spec:
-  # ClusterIP is applied at runtime using the installer config.
-  clusterDomain: cluster.local

manifests/0000_08_cluster-dns-operator_00-cluster-role.yaml

@@ -11,6 +11,13 @@ rules:
   verbs:
   - "*"
 
+- apiGroups:
+  - dns.openshift.io
+  resources:
+  - clusterdnses/status
+  verbs:
+  - update
+
 - apiGroups:
   - apps
   - extensions

manifests/0000_08_cluster-dns-operator_00-custom-resource-definition.yaml

@@ -11,3 +11,5 @@ spec:
     singular: clusterdns
   scope: Cluster
   version: v1alpha1
+  subresources:
+    status: {}

pkg/apis/dns/v1alpha1/types.go

@@ -6,6 +6,7 @@ import (
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
+// ClusterDNSList contains a list of ClusterDNS
 type ClusterDNSList struct {
 	metav1.TypeMeta `json:",inline"`
 	metav1.ListMeta `json:"metadata"`
@@ -14,6 +15,8 @@ type ClusterDNSList struct {
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 
+// ClusterDNS is the Schema for the clusterdnses API
+// +k8s:openapi-gen=true
 type ClusterDNS struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata"`
@@ -22,10 +25,12 @@ type ClusterDNS struct {
 }
 
 type ClusterDNSSpec struct {
-	ClusterIP *string `json:"clusterIP"`
-
-	ClusterDomain *string `json:"clusterDomain"`
 }
+
+// ClusterDNSStatus defines the observed state of ClusterDNS
 type ClusterDNSStatus struct {
-	// Fill me
+	// ClusterIP is the service IP reserved for cluster DNS service
+	ClusterIP string `json:"clusterIP"`
+	// ClusterDomain is the internal domain used in the cluster
+	ClusterDomain string `json:"clusterDomain"`
 }

pkg/manifests/manifests.go

@@ -4,11 +4,8 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"net"
 	"strings"
 
-	configv1 "github.com/openshift/api/config/v1"
-
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -19,8 +16,6 @@ import (
 
 	dnsv1alpha1 "github.com/openshift/cluster-dns-operator/pkg/apis/dns/v1alpha1"
 	"github.com/openshift/cluster-dns-operator/pkg/operator"
-
-	"github.com/apparentlymart/go-cidr/cidr"
 )
 
 const (
@@ -50,31 +45,12 @@ func NewFactory(config operator.Config) *Factory {
 	return &Factory{config: config}
 }
 
-// ClusterDNSDefaultCR builds a default cluster DNS with a cluster IP set to the
-// 10th IP from the service CIDR range defined in the install config within the
-// cluster config.
-func (f *Factory) ClusterDNSDefaultCR(networkConfig *configv1.Network) (*dnsv1alpha1.ClusterDNS, error) {
-	if networkConfig == nil {
-		return nil, fmt.Errorf("networkConfig is required")
-	}
-
+// ClusterDNSDefaultCR builds a default cluster DNS
+func (f *Factory) ClusterDNSDefaultCR() (*dnsv1alpha1.ClusterDNS, error) {
 	cr, err := NewClusterDNS(MustAssetReader(ClusterDNSDefaultCR))
 	if err != nil {
 		return nil, err
 	}
-
-	_, serviceCIDR, err := net.ParseCIDR(networkConfig.Status.ServiceNetwork[0])
-	if err != nil {
-		return nil, fmt.Errorf("invalid serviceCIDR %q: %v", networkConfig.Status.ServiceNetwork[0], err)
-	}
-
-	dnsClusterIP, err := cidr.Host(serviceCIDR, 10)
-	if err != nil {
-		return nil, fmt.Errorf("invalid serviceCIDR %v: %v", serviceCIDR, err)
-	}
-
-	ip := dnsClusterIP.String()
-	cr.Spec.ClusterIP = &ip
 	return cr, nil
 }
 
@@ -110,20 +86,20 @@ func (f *Factory) DNSClusterRoleBinding() (*rbacv1.ClusterRoleBinding, error) {
 	return crb, nil
 }
 
-func (f *Factory) DNSConfigMap(dns *dnsv1alpha1.ClusterDNS) (*corev1.ConfigMap, error) {
+func (f *Factory) DNSConfigMap(dns *dnsv1alpha1.ClusterDNS, clusterDomain string) (*corev1.ConfigMap, error) {
 	cm, err := NewConfigMap(MustAssetReader(DNSConfigMap))
 	if err != nil {
 		return nil, err
 	}
 	cm.Name = "dns-" + dns.Name
 
-	if dns.Spec.ClusterDomain != nil {
-		cm.Data["Corefile"] = strings.Replace(cm.Data["Corefile"], "cluster.local", *dns.Spec.ClusterDomain, -1)
+	if len(clusterDomain) > 0 {
+		cm.Data["Corefile"] = strings.Replace(cm.Data["Corefile"], "cluster.local", clusterDomain, -1)
 	}
 	return cm, nil
 }
 
-func (f *Factory) DNSDaemonSet(dns *dnsv1alpha1.ClusterDNS) (*appsv1.DaemonSet, error) {
+func (f *Factory) DNSDaemonSet(dns *dnsv1alpha1.ClusterDNS, clusterIP, clusterDomain string) (*appsv1.DaemonSet, error) {
 	ds, err := NewDaemonSet(MustAssetReader(DNSDaemonSet))
 	if err != nil {
 		return nil, err
@@ -158,28 +134,30 @@ func (f *Factory) DNSDaemonSet(dns *dnsv1alpha1.ClusterDNS) (*appsv1.DaemonSet,
 			ds.Spec.Template.Spec.Containers[i].Image = f.config.CoreDNSImage
 		case "dns-node-resolver":
 			ds.Spec.Template.Spec.Containers[i].Image = f.config.OpenshiftCLIImage
-			if dns.Spec.ClusterIP != nil && dns.Spec.ClusterDomain != nil {
-				if c.Env == nil {
-					c.Env = []corev1.EnvVar{}
-				}
-				envs := []corev1.EnvVar{
-					{
-						Name:  "NAMESERVER",
-						Value: *dns.Spec.ClusterIP,
-					},
-					{
-						Name:  "CLUSTER_DOMAIN",
-						Value: *dns.Spec.ClusterDomain,
-					},
-				}
-				ds.Spec.Template.Spec.Containers[i].Env = append(ds.Spec.Template.Spec.Containers[i].Env, envs...)
+			envs := []corev1.EnvVar{}
+			if len(clusterIP) > 0 {
+				envs = append(envs, corev1.EnvVar{
+					Name:  "NAMESERVER",
+					Value: clusterIP,
+				})
+			}
+			if len(clusterDomain) > 0 {
+				envs = append(envs, corev1.EnvVar{
+					Name:  "CLUSTER_DOMAIN",
+					Value: clusterDomain,
+				})
+			}
+
+			if ds.Spec.Template.Spec.Containers[i].Env == nil {
+				ds.Spec.Template.Spec.Containers[i].Env = []corev1.EnvVar{}
 			}
+			ds.Spec.Template.Spec.Containers[i].Env = append(ds.Spec.Template.Spec.Containers[i].Env, envs...)
 		}
 	}
 	return ds, nil
 }
 
-func (f *Factory) DNSService(dns *dnsv1alpha1.ClusterDNS) (*corev1.Service, error) {
+func (f *Factory) DNSService(dns *dnsv1alpha1.ClusterDNS, clusterIP string) (*corev1.Service, error) {
 	s, err := NewService(MustAssetReader(DNSService))
 	if err != nil {
 		return nil, err
@@ -196,8 +174,8 @@ func (f *Factory) DNSService(dns *dnsv1alpha1.ClusterDNS) (*corev1.Service, erro
 	}
 	s.Spec.Selector["dns"] = s.Name
 
-	if dns.Spec.ClusterIP != nil {
-		s.Spec.ClusterIP = *dns.Spec.ClusterIP
+	if len(clusterIP) > 0 {
+		s.Spec.ClusterIP = clusterIP
 	}
 	return s, nil
 }

pkg/manifests/manifests_test.go

@@ -7,8 +7,6 @@ import (
 	"github.com/openshift/cluster-dns-operator/pkg/operator"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	configv1 "github.com/openshift/api/config/v1"
 )
 
 func TestManifests(t *testing.T) {
@@ -18,17 +16,17 @@ func TestManifests(t *testing.T) {
 	}
 
 	f := NewFactory(config)
-
+	clusterDomain := "cluster.local"
+	clusterIP := "172.30.77.10"
 	dns := &dnsv1alpha1.ClusterDNS{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "default",
 		},
-		Spec: dnsv1alpha1.ClusterDNSSpec{
-			ClusterDomain: stringPtr("cluster.local"),
-			ClusterIP:     stringPtr("172.30.77.10"),
-		},
 	}
 
+	if _, err := f.ClusterDNSDefaultCR(); err != nil {
+		t.Errorf("invalid ClusterDNSDefaultCR: %v", err)
+	}
 	if _, err := f.DNSNamespace(); err != nil {
 		t.Errorf("invalid DNSNamespace: %v", err)
 	}
@@ -41,10 +39,10 @@ func TestManifests(t *testing.T) {
 	if _, err := f.DNSClusterRoleBinding(); err != nil {
 		t.Errorf("invalid DNSClusterRoleBinding: %v", err)
 	}
-	if _, err := f.DNSConfigMap(dns); err != nil {
+	if _, err := f.DNSConfigMap(dns, clusterDomain); err != nil {
 		t.Errorf("invalid DNSClusterRoleBinding: %v", err)
 	}
-	if ds, err := f.DNSDaemonSet(dns); err != nil {
+	if ds, err := f.DNSDaemonSet(dns, clusterIP, clusterDomain); err != nil {
 		t.Errorf("invalid DNSDaemonSet: %v", err)
 	} else {
 		// Validate the daemonset
@@ -69,44 +67,21 @@ func TestManifests(t *testing.T) {
 				nameserver, ok := envs["NAMESERVER"]
 				if !ok {
 					t.Errorf("NAMESERVER env for dns node resolver image not found")
-				} else if *dns.Spec.ClusterIP != nameserver {
-					t.Errorf("expected NAMESERVER env for dns node resolver image %q, got %q", *dns.Spec.ClusterIP, nameserver)
+				} else if clusterIP != nameserver {
+					t.Errorf("expected NAMESERVER env for dns node resolver image %q, got %q", clusterIP, nameserver)
 				}
-				clusterDomain, ok := envs["CLUSTER_DOMAIN"]
+				domain, ok := envs["CLUSTER_DOMAIN"]
 				if !ok {
 					t.Errorf("CLUSTER_DOMAIN env for dns node resolver image not found")
-				} else if *dns.Spec.ClusterDomain != clusterDomain {
-					t.Errorf("expected CLUSTER_DOMAIN env for dns node resolver image %q, got %q", *dns.Spec.ClusterDomain, clusterDomain)
+				} else if clusterDomain != domain {
+					t.Errorf("expected CLUSTER_DOMAIN env for dns node resolver image %q, got %q", clusterDomain, domain)
 				}
 			default:
 				t.Errorf("unexpected daemonset container %q", c.Name)
 			}
 		}
 	}
-	if _, err := f.DNSService(dns); err != nil {
+	if _, err := f.DNSService(dns, clusterIP); err != nil {
 		t.Errorf("invalid DNSService: %v", err)
 	}
 }
-
-func TestDefaultClusterDNS(t *testing.T) {
-	networkConfig := &configv1.Network{
-		Status: configv1.NetworkStatus{
-			ServiceNetwork: []string{"10.3.0.0/16"},
-		},
-	}
-
-	config := operator.Config{
-		CoreDNSImage:      "quay.io/openshift/coredns:test",
-		OpenshiftCLIImage: "openshift/origin-cli:test",
-	}
-
-	def, err := NewFactory(config).ClusterDNSDefaultCR(networkConfig)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if e, a := "10.3.0.10", *def.Spec.ClusterIP; e != a {
-		t.Errorf("expected default clusterdns clusterIP=%s, got %s", e, a)
-	}
-}
-
-func stringPtr(s string) *string { return &s }