Bug 1813062: Updates Status Reconciliation to Support DNS Service

Makefile

@@ -11,6 +11,8 @@ IMAGE_TAG=openshift/origin-cluster-dns-operator
 GO=GO111MODULE=on GOFLAGS=-mod=vendor go
 GO_BUILD_RECIPE=CGO_ENABLED=0 $(GO) build -o $(BIN) $(MAIN_PACKAGE)
 
+TEST ?= .*
+
 .PHONY: build
 build:
 	$(GO_BUILD_RECIPE)
@@ -44,7 +46,7 @@ release-local:
 
 .PHONY: test-e2e
 test-e2e:
-	KUBERNETES_CONFIG="$(KUBECONFIG)" $(GO) test -v -tags e2e ./...
+	$(GO) test -timeout 1h -count 1 -v -tags e2e -run "$(TEST)" ./test/e2e
 
 .PHONY: verify
 verify:
@@ -63,6 +65,10 @@ else
 	docker build -t $(IMAGE_TAG) .
 endif
 
+.PHONY: run-local
+run-local: build
+	hack/run-local.sh
+
 .PHONY: clean
 clean:
 	$(GO) clean

cmd/dns-operator/main.go

@@ -5,7 +5,7 @@ import (
 
 	"github.com/openshift/cluster-dns-operator/pkg/operator"
 	operatorconfig "github.com/openshift/cluster-dns-operator/pkg/operator/config"
-	"github.com/openshift/cluster-dns-operator/pkg/operator/controller"
+	statuscontroller "github.com/openshift/cluster-dns-operator/pkg/operator/controller/status"
 
 	"github.com/sirupsen/logrus"
 
@@ -14,14 +14,16 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/runtime/signals"
 )
 
+const operatorNamespace = "openshift-dns-operator"
+
 func main() {
 	metrics.DefaultBindAddress = ":60000"
 
 	// Collect operator configuration.
 	releaseVersion := os.Getenv("RELEASE_VERSION")
 	if len(releaseVersion) == 0 {
-		releaseVersion = controller.UnknownVersionValue
-		logrus.Infof("RELEASE_VERSION environment variable is missing, defaulting to %q", controller.UnknownVersionValue)
+		releaseVersion = statuscontroller.UnknownVersionValue
+		logrus.Infof("RELEASE_VERSION environment variable is missing, defaulting to %q", statuscontroller.UnknownVersionValue)
 	}
 	coreDNSImage := os.Getenv("IMAGE")
 	if len(coreDNSImage) == 0 {
@@ -38,6 +40,7 @@ func main() {
 	}
 
 	operatorConfig := operatorconfig.Config{
+		OperatorNamespace:      operatorNamespace,
 		OperatorReleaseVersion: releaseVersion,
 		CoreDNSImage:           coreDNSImage,
 		OpenshiftCLIImage:      cliImage,

hack/run-local.sh

@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+
+set -euo pipefail
+
+oc scale --replicas 0 -n openshift-cluster-version deployments/cluster-version-operator
+oc scale --replicas 0 -n openshift-dns-operator deployments dns-operator
+
+IMAGE=$(oc get -n openshift-dns-operator deployments/dns-operator -o json | jq -r '.spec.template.spec.containers[0].env[] | select(.name=="IMAGE").value')
+OPENSHIFT_CLI_IMAGE=$(oc get -n openshift-dns-operator deployments/dns-operator -o json | jq -r '.spec.template.spec.containers[0].env[] | select(.name=="OPENSHIFT_CLI_IMAGE").value')
+KUBE_RBAC_PROXY_IMAGE=$(oc get -n openshift-dns-operator deployments/dns-operator -o json | jq -r '.spec.template.spec.containers[0].env[] | select(.name=="KUBE_RBAC_PROXY_IMAGE").value')
+RELEASE_VERSION=$(oc get clusterversion/version -o json | jq -r '.status.desired.version')
+NAMESPACE="${NAMESPACE:-"openshift-dns-operator"}"
+
+echo "Image: ${IMAGE}"
+echo "OpenShift CLI Image: ${OPENSHIFT_CLI_IMAGE}"
+echo "Kube RBAC Proxy Image: ${KUBE_RBAC_PROXY_IMAGE}"
+echo "Release version: ${RELEASE_VERSION}"
+echo "Operator Namespace: ${NAMESPACE}"
+
+RELEASE_VERSION="${RELEASE_VERSION}" IMAGE="${IMAGE}" OPENSHIFT_CLI_IMAGE="${OPENSHIFT_CLI_IMAGE}" \
+KUBE_RBAC_PROXY_IMAGE="${KUBE_RBAC_PROXY_IMAGE}" ./dns-operator "$@"

pkg/operator/config/config.go

@@ -6,6 +6,9 @@ type Config struct {
 	// OperatorReleaseVersion is the current version of the operator.
 	OperatorReleaseVersion string
 
+	// OperatorNamespace is the namespace that the operator runs in.
+	OperatorNamespace string
+
 	// CoreDNSImage is the CoreDNS image to manage.
 	CoreDNSImage string
 

pkg/operator/controller/controller.go

@@ -9,6 +9,7 @@ import (
 	operatorv1 "github.com/openshift/api/operator/v1"
 
 	"github.com/openshift/cluster-dns-operator/pkg/manifests"
+	operatorconfig "github.com/openshift/cluster-dns-operator/pkg/operator/config"
 	"github.com/openshift/cluster-dns-operator/pkg/util/slice"
 
 	"github.com/sirupsen/logrus"
@@ -48,7 +49,7 @@ const (
 // DNS resources.
 //
 // The controller will be pre-configured to watch for DNS resources.
-func New(mgr manager.Manager, config Config) (controller.Controller, error) {
+func New(mgr manager.Manager, config operatorconfig.Config) (controller.Controller, error) {
 	reconciler := &reconciler{
 		Config: config,
 		client: mgr.GetClient(),
@@ -73,18 +74,10 @@ func New(mgr manager.Manager, config Config) (controller.Controller, error) {
 	return c, nil
 }
 
-// Config holds all the things necessary for the controller to run.
-type Config struct {
-	CoreDNSImage           string
-	OpenshiftCLIImage      string
-	OperatorReleaseVersion string
-	KubeRBACProxyImage     string
-}
-
 // reconciler handles the actual dns reconciliation logic in response to
 // events.
 type reconciler struct {
-	Config
+	operatorconfig.Config
 
 	client client.Client
 	cache  cache.Cache
@@ -154,11 +147,6 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 		}
 	}
 
-	// TODO: Should this be another controller?
-	if err := r.syncOperatorStatus(); err != nil {
-		errs = append(errs, fmt.Errorf("failed to sync operator status: %v", err))
-	}
-
 	// Log in case of errors as the controller's logs get eaten.
 	if len(errs) > 0 {
 		logrus.Errorf("failed to reconcile request %s: %v", request, utilerrors.NewAggregate(errs))

pkg/operator/controller/dns_status.go

@@ -49,7 +49,7 @@ func computeDNSStatusConditions(oldConditions []operatorv1.OperatorCondition, cl
 
 	conditions := []operatorv1.OperatorCondition{
 		computeDNSDegradedCondition(oldDegradedCondition, clusterIP, ds),
-		computeDNSProgressingCondition(oldProgressingCondition, ds),
+		computeDNSProgressingCondition(oldProgressingCondition, clusterIP, ds),
 		computeDNSAvailableCondition(oldAvailableCondition, clusterIP, ds),
 	}
 
@@ -67,12 +67,12 @@ func computeDNSDegradedCondition(oldCondition *operatorv1.OperatorCondition, clu
 	switch {
 	case len(clusterIP) == 0 && ds.Status.NumberAvailable == 0:
 		degradedCondition.Status = operatorv1.ConditionTrue
-		degradedCondition.Reason = "NoClusterIPAndDaemonSet"
-		degradedCondition.Message = "No ClusterIP assigned to DNS Service and no DaemonSet pods running"
+		degradedCondition.Reason = "NoServiceIPAndNoDaemonSetPods"
+		degradedCondition.Message = "No IP assigned to DNS service and no DaemonSet pods running"
 	case len(clusterIP) == 0:
 		degradedCondition.Status = operatorv1.ConditionTrue
-		degradedCondition.Reason = "NoClusterIP"
-		degradedCondition.Message = "No ClusterIP assigned to DNS Service"
+		degradedCondition.Reason = "NoServiceIP"
+		degradedCondition.Message = "No IP assigned to DNS service"
 	case ds.Status.DesiredNumberScheduled == 0:
 		degradedCondition.Status = operatorv1.ConditionTrue
 		degradedCondition.Reason = "NoPodsDesired"
@@ -88,27 +88,37 @@ func computeDNSDegradedCondition(oldCondition *operatorv1.OperatorCondition, clu
 	default:
 		degradedCondition.Status = operatorv1.ConditionFalse
 		degradedCondition.Reason = "AsExpected"
-		degradedCondition.Message = "ClusterIP assigned to DNS Service and minimum DaemonSet pods running"
+		degradedCondition.Message = "IP assigned to DNS service and minimum DaemonSet pods running"
 	}
 
 	return setDNSLastTransitionTime(degradedCondition, oldCondition)
 }
 
 // computeDNSProgressingCondition computes the dns Progressing status condition
 // based on the status of ds.
-func computeDNSProgressingCondition(oldCondition *operatorv1.OperatorCondition, ds *appsv1.DaemonSet) operatorv1.OperatorCondition {
+func computeDNSProgressingCondition(oldCondition *operatorv1.OperatorCondition, clusterIP string, ds *appsv1.DaemonSet) operatorv1.OperatorCondition {
 	progressingCondition := &operatorv1.OperatorCondition{
 		Type: operatorv1.OperatorStatusTypeProgressing,
 	}
-	if ds.Status.NumberAvailable == ds.Status.DesiredNumberScheduled {
-		progressingCondition.Status = operatorv1.ConditionFalse
-		progressingCondition.Reason = "AsExpected"
-		progressingCondition.Message = "All expected Nodes running DaemonSet pod"
-	} else {
+	switch {
+	case len(clusterIP) == 0 && ds.Status.NumberAvailable != ds.Status.DesiredNumberScheduled:
+		progressingCondition.Status = operatorv1.ConditionTrue
+		progressingCondition.Reason = "Reconciling"
+		progressingCondition.Message = fmt.Sprintf("No IP assigned to DNS service and %d Nodes running a DaemonSet pod, want %d",
+			ds.Status.NumberAvailable, ds.Status.DesiredNumberScheduled)
+	case len(clusterIP) == 0:
+		progressingCondition.Status = operatorv1.ConditionTrue
+		progressingCondition.Reason = "Reconciling"
+		progressingCondition.Message = "No IP assigned to DNS service"
+	case ds.Status.NumberAvailable != ds.Status.DesiredNumberScheduled:
 		progressingCondition.Status = operatorv1.ConditionTrue
 		progressingCondition.Reason = "Reconciling"
 		progressingCondition.Message = fmt.Sprintf("%d Nodes running a DaemonSet pod, want %d",
 			ds.Status.NumberAvailable, ds.Status.DesiredNumberScheduled)
+	default:
+		progressingCondition.Status = operatorv1.ConditionFalse
+		progressingCondition.Reason = "AsExpected"
+		progressingCondition.Message = "All expected Nodes running DaemonSet pod and IP assigned to DNS service"
 	}
 
 	return setDNSLastTransitionTime(progressingCondition, oldCondition)
@@ -123,20 +133,20 @@ func computeDNSAvailableCondition(oldCondition *operatorv1.OperatorCondition, cl
 	switch {
 	case len(clusterIP) == 0 && ds.Status.NumberAvailable == 0:
 		availableCondition.Status = operatorv1.ConditionFalse
-		availableCondition.Reason = "NoClusterIPAndDaemonSet"
-		availableCondition.Message = "No ClusterIP assigned to DNS Service and no running DaemonSet pods"
+		availableCondition.Reason = "NoServiceIPAndNoDaemonSetPods"
+		availableCondition.Message = "No IP assigned to DNS service and no running DaemonSet pods"
 	case len(clusterIP) == 0:
 		availableCondition.Status = operatorv1.ConditionFalse
-		availableCondition.Reason = "NoDNSService"
-		availableCondition.Message = "No ClusterIP assigned to DNS Service"
+		availableCondition.Reason = "NoServiceIP"
+		availableCondition.Message = "No IP assigned to DNS service"
 	case ds.Status.NumberAvailable == 0:
 		availableCondition.Status = operatorv1.ConditionFalse
 		availableCondition.Reason = "DaemonSetUnavailable"
 		availableCondition.Message = "DaemonSet pod not running on any Nodes"
 	default:
 		availableCondition.Status = operatorv1.ConditionTrue
 		availableCondition.Reason = "AsExpected"
-		availableCondition.Message = "Minimum number of Nodes running DaemonSet pod"
+		availableCondition.Message = "Minimum number of Nodes running DaemonSet pod and IP assigned to DNS service"
 	}
 
 	return setDNSLastTransitionTime(availableCondition, oldCondition)

pkg/operator/controller/dns_status_test.go

@@ -27,6 +27,11 @@ func TestDNSStatusConditions(t *testing.T) {
 		{
 			description: "no cluster ip, 0/0 pods available",
 			inputs:      testInputs{false, 0, 0},
+			outputs:     testOutputs{true, true, false},
+		},
+		{
+			description: "cluster ip, 0/0 pods available",
+			inputs:      testInputs{true, 0, 0},
 			outputs:     testOutputs{true, false, false},
 		},
 		{
@@ -42,7 +47,7 @@ func TestDNSStatusConditions(t *testing.T) {
 		{
 			description: "no cluster ip, 2/2 pods available",
 			inputs:      testInputs{false, 2, 2},
-			outputs:     testOutputs{true, false, false},
+			outputs:     testOutputs{true, true, false},
 		},
 		{
 			description: "daemonset pod available on 0/0 nodes",

pkg/operator/controller/names.go

@@ -15,12 +15,36 @@ const (
 	// MetricsServingCertAnnotation is the annotation needed to generate
 	// the certificates for secure DNS metrics.
 	MetricsServingCertAnnotation = "service.beta.openshift.io/serving-cert-secret-name"
+
+	// DefaultOperandNamespace is the default namespace name of operands.
+	DefaultOperandNamespace = "openshift-dns"
+
+	// DefaultOperatorName is the default name of dns cluster operator.
+	DefaultOperatorName = "dns"
+
+	// DefaultDNSName is the default name of dns resource.
+	DefaultDNSName = "default"
 )
 
+// DNSClusterOperatorName returns the namespaced name of the ClusterOperator
+// resource for the operator.
+func DNSClusterOperatorName() types.NamespacedName {
+	return types.NamespacedName{
+		Name: DefaultOperatorName,
+	}
+}
+
+// DefaultDNSNamespaceName returns the namespaced name of the default DNS resource.
+func DefaultDNSNamespaceName() types.NamespacedName {
+	return types.NamespacedName{
+		Name: DefaultDNSName,
+	}
+}
+
 // DNSDaemonSetName returns the namespaced name for the dns daemonset.
 func DNSDaemonSetName(dns *operatorv1.DNS) types.NamespacedName {
 	return types.NamespacedName{
-		Namespace: "openshift-dns",
+		Namespace: DefaultOperandNamespace,
 		Name:      "dns-" + dns.Name,
 	}
 }