Create a default HNCConfiguration singleton when starting HNC.

This PR creates a default HNCConfiguration singleton when starting HNC if the singleton does not exist. The feature is achieved by adding a channel in reconciler watch to trigger the reconciliation at startup. The reconciliation will ensure the default singleton will be created if it does not exist.

Tested: GKE cluster; unit tests.

Design doc: http://bit.ly/hnc-type-configuration
Issue: kubernetes-retired#411

incubator/hnc/go.sum

@@ -175,14 +175,17 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_golang v0.9.0 h1:tXuTFVHC03mW0D+Ua1Q2d1EAVqLTuggX50V0VLICCzY=
 github.com/prometheus/client_golang v0.9.0/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXPKyh/dDVn+NZz0KFw=
+github.com/prometheus/client_golang v0.9.2 h1:awm861/B8OKDd2I/6o1dy3ra4BamzKhYOiGItCeZ740=
 github.com/prometheus/client_golang v0.9.2/go.mod h1:OsXs2jCmiKlQ1lTBmv21f2mNfw4xf/QclQDMrYNZzcM=
 github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910 h1:idejC8f05m9MGOsuEi1ATq9shN03HrxNkD/luQvxCv8=
 github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
 github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e h1:n/3MEhJQjQxrOUCzh1Y3Re6aJUUWRp2M9+Oc3eVn/54=
 github.com/prometheus/common v0.0.0-20180801064454-c7de2306084e/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro=
+github.com/prometheus/common v0.0.0-20181126121408-4724e9255275 h1:PnBWHBf+6L0jOqq0gIVUe6Yk0/QMZ640k6NvkxcBf+8=
 github.com/prometheus/common v0.0.0-20181126121408-4724e9255275/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro=
 github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273 h1:agujYaXJSxSo18YNX3jzl+4G6Bstwt+kqv47GS12uL0=
 github.com/prometheus/procfs v0.0.0-20180725123919-05ee40e3a273/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
+github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a h1:9a8MnZMP0X2nLJdBg+pBmGgkJlSaKC2KaQmTCk1XDtE=
 github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
 github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=

incubator/hnc/pkg/reconcilers/hnc_config.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/api/errors"
@@ -29,6 +31,11 @@ type ConfigReconciler struct {
 	// Forest is the in-memory data structure that is shared with all other reconcilers.
 	Forest *forest.Forest
 
+	// Igniter is a channel of event.GenericEvent (see "Watching Channels" in
+	// https://book-v1.book.kubebuilder.io/beyond_basics/controller_watches.html)
+	// that is used to enqueue the singleton for initial reconciliation.
+	Igniter chan event.GenericEvent
+
 	// HierarchyConfigUpdates is a channel of events used to update hierarchy configuration changes performed by
 	// ObjectReconcilers. It is passed on to ObjectReconcilers for the updates. The ConfigReconciler itself does
 	// not use it.
@@ -56,7 +63,7 @@ func (r *ConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	// achieve the reconciliation.
 	r.Log.Info("Reconciling cluster-wide HNC configuration.")
 
-	// Create cooresponding ObjectReconcilers for newly added types, if needed.
+	// Create corresponding ObjectReconcilers for newly added types, if needed.
 	// TODO: Returning an error here will make controller-runtime to requeue the object to be reconciled again.
 	// A better way to handle this is to display the error as part of the HNCConfig.status field to suggest the error is a
 	// permanent problem and to stop controller-runtime from retrying.
@@ -175,5 +182,21 @@ func (r *ConfigReconciler) createObjectReconciler(gvk schema.GroupVersionKind) e
 func (r *ConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&api.HNCConfiguration{}).
+		Watches(&source.Channel{Source: r.Igniter}, &handler.EnqueueRequestForObject{}).
 		Complete(r)
 }
+
+// forceInitialReconcile forces reconciliation to start after setting up the
+// controller with the manager. This is used to create a default singleton if
+// there is no singleton in the cluster. This occurs in a goroutine so the
+// caller doesn't block; since the reconciler is never garbage-collected,
+// this is safe.
+func (r *ConfigReconciler) forceInitialReconcile(log logr.Logger, reason string) {
+	go func() {
+		log.Info("Enqueuing for reconciliation", "reason", reason)
+		// The watch handler doesn't care about anything except the metadata.
+		inst := &api.HNCConfiguration{}
+		inst.ObjectMeta.Name = api.HNCConfigSingleton
+		r.Igniter <- event.GenericEvent{Meta: inst}
+	}()
+}

incubator/hnc/pkg/reconcilers/hnc_config_test.go

@@ -2,12 +2,13 @@ package reconcilers_test
 
 import (
 	"context"
-	"time"
 
 	api "github.com/kubernetes-sigs/multi-tenancy/incubator/hnc/api/v1alpha1"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/rbac/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
 
@@ -22,6 +23,11 @@ var _ = Describe("HNCConfiguration", func() {
 	BeforeEach(func() {
 		fooName = createNS(ctx, "foo")
 		barName = createNS(ctx, "bar")
+
+		// Give foo a role.
+		makeRole(ctx, fooName, "foo-role")
+		// Give foo a role binding.
+		makeRoleBinding(ctx, fooName, "foo-role", "foo-admin", "foo-role-binding")
 	})
 
 	AfterEach(func() {
@@ -31,32 +37,56 @@ var _ = Describe("HNCConfiguration", func() {
 		}).Should(Succeed())
 	})
 
-	It("should propagate objects whose types have been added to HNCConfiguration", func() {
+	It("should set mode of Roles and RoleBindings as propagate by default", func() {
+		config := getHNCConfig(ctx)
+
+		Eventually(hasTypeWithMode("rbac.authorization.k8s.io/v1", "Role", api.Propagate, config)).Should(BeTrue())
+		Eventually(hasTypeWithMode("rbac.authorization.k8s.io/v1", "RoleBinding", api.Propagate, config)).Should(BeTrue())
+	})
+
+	It("should propagate Roles by default", func() {
 		setParent(ctx, barName, fooName)
-		makeSecret(ctx, fooName, "foo-sec")
 
-		// Wait 1 second to give "foo-sec" a chance to be propagated to bar, if it can be propagated.
-		time.Sleep(1 * time.Second)
-		// Foo should have "foo-sec" since we created it there.
-		Eventually(hasSecret(ctx, fooName, "foo-sec")).Should(BeTrue())
-		// "foo-sec" is not propagated to bar because Secret hasn't been configured in HNCConfiguration.
-		Eventually(hasSecret(ctx, barName, "foo-sec")).Should(BeFalse())
+		Eventually(hasRole(ctx, barName, "foo-role")).Should(BeTrue())
+		Expect(roleInheritedFrom(ctx, barName, "foo-role")).Should(Equal(fooName))
+	})
 
-		Eventually(func() error {
-			c := getHNCConfig(ctx)
-			return addSecretToHNCConfig(ctx, c)
-		}).Should(Succeed())
+	It("should propagate RoleBindings by default", func() {
+		setParent(ctx, barName, fooName)
 
+		Eventually(hasRoleBinding(ctx, barName, "foo-role-binding")).Should(BeTrue())
+		Expect(roleBindingInheritedFrom(ctx, barName, "foo-role-binding")).Should(Equal(fooName))
+	})
+
+	It("should only propagate objects whose types are in HNCConfiguration", func() {
+		setParent(ctx, barName, fooName)
+		makeSecret(ctx, fooName, "foo-sec")
+		makeResourceQuota(ctx, fooName, "foo-resource-quota")
+
+		addToHNCConfig(ctx, "v1", "Secret", api.Propagate)
+
+		// Foo should have both "foo-sec" and "foo-resource-quota" since we created there.
+		Eventually(hasSecret(ctx, fooName, "foo-sec")).Should(BeTrue())
+		Eventually(hasResourceQuota(ctx, fooName, "foo-resource-quota")).Should(BeTrue())
 		// "foo-sec" should now be propagated from foo to bar.
 		Eventually(hasSecret(ctx, barName, "foo-sec")).Should(BeTrue())
 		Expect(secretInheritedFrom(ctx, barName, "foo-sec")).Should(Equal(fooName))
+		// "foo-resource-quota" should not be propagated from foo to bar because ResourceQuota
+		// is not added to HNCConfiguration.
+		Expect(hasResourceQuota(ctx, barName, "foo-resource-quota")).Should(BeFalse())
 	})
 })
 
-func addSecretToHNCConfig(ctx context.Context, c *api.HNCConfiguration) error {
-	secSpec := api.TypeSynchronizationSpec{APIVersion: "v1", Kind: "Secret", Mode: api.Propagate}
-	c.Spec.Types = append(c.Spec.Types, secSpec)
-	return updateHNCConfig(ctx, c)
+func hasTypeWithMode(apiVersion, kind string, mode api.SynchronizationMode, config *api.HNCConfiguration) func() bool {
+	// `Eventually` only works with a fn that doesn't take any args
+	return func() bool {
+		for _, t := range config.Spec.Types {
+			if t.APIVersion == apiVersion && t.Kind == kind && t.Mode == mode {
+				return true
+			}
+		}
+		return false
+	}
 }
 
 func makeSecret(ctx context.Context, nsName, secretName string) {
@@ -89,3 +119,63 @@ func secretInheritedFrom(ctx context.Context, nsName, secretName string) string
 	lif, _ := sec.ObjectMeta.Labels["hnc.x-k8s.io/inheritedFrom"]
 	return lif
 }
+
+func makeRoleBinding(ctx context.Context, nsName, roleName, userName, roleBindingName string) {
+	roleBinding := &v1.RoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      roleBindingName,
+			Namespace: nsName,
+		},
+		Subjects: []v1.Subject{
+			{
+				Kind: "User",
+				Name: userName,
+			},
+		},
+		RoleRef: v1.RoleRef{
+			APIGroup: "rbac.authorization.k8s.io",
+			Kind:     "Role",
+			Name:     roleName,
+		},
+	}
+	ExpectWithOffset(1, k8sClient.Create(ctx, roleBinding)).Should(Succeed())
+}
+
+func hasRoleBinding(ctx context.Context, nsName, roleBindingName string) func() bool {
+	// `Eventually` only works with a fn that doesn't take any args
+	return func() bool {
+		nnm := types.NamespacedName{Namespace: nsName, Name: roleBindingName}
+		roleBinding := &v1.RoleBinding{}
+		err := k8sClient.Get(ctx, nnm, roleBinding)
+		return err == nil
+	}
+}
+
+func roleBindingInheritedFrom(ctx context.Context, nsName, roleBindingName string) string {
+	nnm := types.NamespacedName{Namespace: nsName, Name: roleBindingName}
+	roleBinding := &v1.RoleBinding{}
+	if err := k8sClient.Get(ctx, nnm, roleBinding); err != nil {
+		// should have been caught above
+		return err.Error()
+	}
+	if roleBinding.ObjectMeta.Labels == nil {
+		return ""
+	}
+	lif, _ := roleBinding.ObjectMeta.Labels["hnc.x-k8s.io/inheritedFrom"]
+	return lif
+}
+
+func makeResourceQuota(ctx context.Context, nsName, resourceQuotaName string) {
+	resourceQuota := &corev1.ResourceQuota{}
+	resourceQuota.Name = resourceQuotaName
+	resourceQuota.Namespace = nsName
+	ExpectWithOffset(1, k8sClient.Create(ctx, resourceQuota)).Should(Succeed())
+}
+
+func hasResourceQuota(ctx context.Context, nsName, resourceQuotaName string) bool {
+	// `Eventually` only works with a fn that doesn't take any args
+	nnm := types.NamespacedName{Namespace: nsName, Name: resourceQuotaName}
+	resourceQuota := &corev1.ResourceQuota{}
+	err := k8sClient.Get(ctx, nnm, resourceQuota)
+	return err == nil
+}

incubator/hnc/pkg/reconcilers/object_test.go

@@ -10,7 +10,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
 
@@ -28,7 +27,7 @@ var _ = Describe("Secret", func() {
 		barName = createNS(ctx, "bar")
 		bazName = createNS(ctx, "baz")
 
-		// Give them each a secret
+		// Give them each a role.
 		makeRole(ctx, fooName, "foo-role")
 		makeRole(ctx, barName, "bar-role")
 		makeRole(ctx, bazName, "baz-role")
@@ -60,7 +59,7 @@ var _ = Describe("Secret", func() {
 		// Creates an empty ConfigMap. We use ConfigMap for this test because the apiserver will not
 		// add additional fields to an empty ConfigMap object to make it non-empty.
 		makeConfigMap(ctx, fooName, "foo-config")
-		addConfigMapToHNCConfig(ctx)
+		addToHNCConfig(ctx, "v1", "ConfigMap", api.Propagate)
 
 		// "foo-config" should now be propagated from foo to bar.
 		Eventually(hasConfigMap(ctx, barName, "foo-config")).Should(BeTrue())
@@ -243,52 +242,6 @@ func newOrGetHierarchy(ctx context.Context, nm string) *api.HierarchyConfigurati
 	return hier
 }
 
-func makeRole(ctx context.Context, nsName, roleName string) {
-	role := &v1.Role{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      roleName,
-			Namespace: nsName,
-		},
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "Role",
-			APIVersion: "rbac.authorization.k8s.io/v1",
-		},
-		Rules: []v1.PolicyRule{
-			// Allow the users to read all secrets, namespaces and configmaps.
-			{
-				APIGroups: []string{""},
-				Resources: []string{"secrets", "namespaces", "configmaps"},
-				Verbs:     []string{"get", "watch", "list"},
-			},
-		},
-	}
-	ExpectWithOffset(1, k8sClient.Create(ctx, role)).Should(Succeed())
-}
-
-func hasRole(ctx context.Context, nsName, roleName string) func() bool {
-	// `Eventually` only works with a fn that doesn't take any args
-	return func() bool {
-		nnm := types.NamespacedName{Namespace: nsName, Name: roleName}
-		role := &v1.Role{}
-		err := k8sClient.Get(ctx, nnm, role)
-		return err == nil
-	}
-}
-
-func roleInheritedFrom(ctx context.Context, nsName, roleName string) string {
-	nnm := types.NamespacedName{Namespace: nsName, Name: roleName}
-	role := &v1.Role{}
-	if err := k8sClient.Get(ctx, nnm, role); err != nil {
-		// should have been caught above
-		return err.Error()
-	}
-	if role.ObjectMeta.Labels == nil {
-		return ""
-	}
-	lif, _ := role.ObjectMeta.Labels["hnc.x-k8s.io/inheritedFrom"]
-	return lif
-}
-
 func modifyRole(ctx context.Context, nsName, roleName string) {
 	nnm := types.NamespacedName{Namespace: nsName, Name: roleName}
 	role := &v1.Role{}
@@ -332,15 +285,6 @@ func removeRole(ctx context.Context, nsName, roleName string) {
 	ExpectWithOffset(1, k8sClient.Delete(ctx, role)).Should(Succeed())
 }
 
-func addConfigMapToHNCConfig(ctx context.Context) {
-	Eventually(func() error {
-		c := getHNCConfig(ctx)
-		configMap := api.TypeSynchronizationSpec{APIVersion: "v1", Kind: "ConfigMap", Mode: api.Propagate}
-		c.Spec.Types = append(c.Spec.Types, configMap)
-		return updateHNCConfig(ctx, c)
-	}).Should(Succeed())
-}
-
 // Makes an empty ConfigMap object.
 func makeConfigMap(ctx context.Context, nsName, configMapName string) {
 	configMap := &corev1.ConfigMap{}

incubator/hnc/pkg/reconcilers/setup.go

@@ -41,11 +41,22 @@ func Create(mgr ctrl.Manager, f *forest.Forest, maxReconciles int) error {
 		Log:                    ctrl.Log.WithName("reconcilers").WithName("HNCConfiguration"),
 		Manager:                mgr,
 		Forest:                 f,
+		Igniter:                make(chan event.GenericEvent),
 		HierarchyConfigUpdates: hcChan,
 	}
 	if err := cr.SetupWithManager(mgr); err != nil {
 		return fmt.Errorf("cannot create Config reconciler: %s", err.Error())
 	}
+	// Create a default singleton if there is no singleton in the cluster.
+	//
+	// The cache used by the client to retrieve objects might not be populated
+	// at this point. As a result, we cannot use r.Get() to determine the existence
+	// of the singleton and then use r.Create() to create the singleton if
+	// it does not exist. As a workaround, we decide to enforce reconciliation. The
+	// cache is populated at the reconciliation stage. A default singleton will be
+	// created during the reconciliation if there is no singleton in the cluster.
+	cr.forceInitialReconcile(cr.Log, "Enforce reconciliation to create a default"+
+		"HNCConfiguration singleton if it does not exist.")
 
 	return nil
 }