Update HCR & HNSR to move NS creation to HNSR

This commit removes the hc.Spec.RequiredChildren field and the
enable-hns-reconciler flag. It also moves the creation of the owned
(previously requiredChild) namespaces from HC Reconciler (HCR) to HNS
Reconciler (HNSR). The logic in HCR is cleaned up to adapt this change.

Tested with the current and updated integration tests by "make test".
Also manually tested the 'k hns describe'. The HNS_MISSING condition was
set when I deleted the HNS instance in the owner. The owned namespace
got recreated after I deleted it, which means the new watch in HNSR is
working.

incubator/hnc/Makefile

@@ -27,11 +27,6 @@ CRD_OPTIONS ?= "crd:trivialVersions=true"
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 GOBIN ?= $(shell go env GOPATH)/bin
 
-# Set default to not use HNS reconciler
-ifeq (,${HNS})
-HNS=0
-endif
-
 # Get check sum value of krew archive
 KREW_CKSM=$(shell sha256sum bin/kubectl-hierarchical_namespaces.tar.gz | cut -d " " -f 1)
 
@@ -41,12 +36,7 @@ all: test docker-build
 
 # Run tests
 test: build
-	# run tests in all directories except ./pkg/reconcilers/
-	go test ./api/... ./cmd/... `go list ./pkg/... | grep -v reconcilers` -coverprofile cover.out
-	# separately run tests in ./pkg/reconcilers/
-	# by default it will not use the HNS reconciler
-	# with HNS=1 it will use the new HNS reconciler
-	go test ./pkg/reconcilers/... -enable-hierarchicalnamespace-reconciler=${HNS} -coverprofile cover.out
+	go test ./api/... ./cmd/... ./pkg/... -coverprofile cover.out
 
 # Builds all binaries (manager and kubectl) and manifests
 build: generate fmt vet manifests

incubator/hnc/api/v1alpha1/hierarchical_namespace.go

@@ -21,8 +21,10 @@ import (
 
 // Constants for the hierarchicalnamespaces resource type and namespace annotation.
 const (
-	HierarchicalNamespaces = "hierarchicalnamespaces"
-	AnnotationOwner        = MetaGroup + "/owner"
+	HierarchicalNamespaces           = "hierarchicalnamespaces"
+	HierarchicalNamespacesKind       = "HierarchicalNamespace"
+	HierarchicalNamespacesAPIVersion = MetaGroup + "/v1alpha1"
+	AnnotationOwner                  = MetaGroup + "/ownerReference"
 )
 
 // HNSState describes the state of a hierarchical namespace. The state could be

incubator/hnc/api/v1alpha1/hierarchy_types.go

@@ -39,6 +39,7 @@ const (
 	CritParentInvalid    Code = "CRIT_PARENT_INVALID"
 	CritAncestor         Code = "CRIT_ANCESTOR"
 	SubnamespaceConflict Code = "SUBNAMESPACE_CONFLICT"
+	HNSMissing           Code = "HNS_MISSING"
 	CannotUpdate         Code = "CANNOT_UPDATE_OBJECT"
 	CannotPropagate      Code = "CANNOT_PROPAGATE_OBJECT"
 )
@@ -68,11 +69,6 @@ type HierarchyConfigurationSpec struct {
 	// AllowCascadingDelete indicates if the self-serve subnamespaces of this namespace are allowed
 	// to cascading delete.
 	AllowCascadingDelete bool `json:"allowCascadingDelete"`
-
-	// RequiredChildren indicates the required subnamespaces of this namespace. If they do not exist,
-	// the HNC will create them, allowing users without privileges to create namespaces to get child
-	// namespaces anyway.
-	RequiredChildren []string `json:"requiredChildren,omitempty"`
 }
 
 // HierarchyStatus defines the observed state of Hierarchy

incubator/hnc/cmd/manager/main.go

@@ -67,7 +67,6 @@ func main() {
 		debugLogs            bool
 		testLog              bool
 		qps                  int
-		enableHNSReconciler  bool
 	)
 	flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
 	flag.BoolVar(&enableLeaderElection, "enable-leader-election", false,
@@ -79,7 +78,6 @@ func main() {
 	flag.BoolVar(&testLog, "enable-test-log", false, "Enables test log.")
 	flag.IntVar(&maxReconciles, "max-reconciles", 1, "Number of concurrent reconciles to perform.")
 	flag.IntVar(&qps, "apiserver-qps-throttle", 50, "The maximum QPS to the API server.")
-	flag.BoolVar(&enableHNSReconciler, "enable-hierarchicalnamespace-reconciler", false, "Enables hierarchicalnamespace reconciler.")
 	flag.Parse()
 
 	// Enable OpenCensus exporters to export metrics
@@ -140,7 +138,7 @@ func main() {
 	// Create all reconciling controllers
 	f := forest.NewForest()
 	setupLog.Info("Creating controllers", "maxReconciles", maxReconciles)
-	if err := reconcilers.Create(mgr, f, maxReconciles, enableHNSReconciler); err != nil {
+	if err := reconcilers.Create(mgr, f, maxReconciles); err != nil {
 		setupLog.Error(err, "cannot create controllers")
 		os.Exit(1)
 	}

incubator/hnc/config/crd/bases/hnc.x-k8s.io_hierarchyconfigurations.yaml

@@ -41,14 +41,6 @@ spec:
             parent:
               description: Parent indicates the parent of this namespace, if any.
               type: string
-            requiredChildren:
-              description: RequiredChildren indicates the required subnamespaces of
-                this namespace. If they do not exist, the HNC will create them, allowing
-                users without privileges to create namespaces to get child namespaces
-                anyway.
-              items:
-                type: string
-              type: array
           required:
           - allowCascadingDelete
           type: object

incubator/hnc/pkg/forest/forest.go

@@ -154,9 +154,14 @@ type Namespace struct {
 	// on this namespace.
 	conditions conditions
 
+	// Todo remove Owner field or replace it with isOwned bool field. See issue:
+	//  https://github.com/kubernetes-sigs/multi-tenancy/issues/552
 	// Owner indicates that this namespace is being or was created solely to live as a
 	// subnamespace of the specified parent.
 	Owner string
+
+	// HNSes store a list of HNS instances in the namespace.
+	HNSes []string
 }
 
 type condition struct {
@@ -263,20 +268,6 @@ func (ns *Namespace) ChildNames() []string {
 	return nms
 }
 
-// OwnedNames returns a list of names of the owned namespaces or nil if there's none.
-func (ns *Namespace) OwnedNames() []string {
-	if len(ns.forest.namespaces) == 0 {
-		return nil
-	}
-	nms := []string{}
-	for nm, ns := range ns.forest.namespaces {
-		if ns.Owner == ns.name {
-			nms = append(nms, nm)
-		}
-	}
-	return nms
-}
-
 // RelativesNames returns the children and parent.
 func (ns *Namespace) RelativesNames() []string {
 	a := []string{}
@@ -316,6 +307,30 @@ func (ns *Namespace) AncestryNames(other *Namespace) []string {
 	return append(ancestry, ns.name)
 }
 
+// SetHNSes updates the HNSes and returns a difference between the new/old list.
+func (ns *Namespace) SetHNSes(hnsnms []string) (diff []string) {
+	add := make(map[string]bool)
+	for _, nm := range hnsnms {
+		add[nm] = true
+	}
+	for _, nm := range ns.HNSes {
+		if add[nm] {
+			delete(add, nm)
+		} else {
+			// This old HNS is not in the new HNS list.
+			diff = append(diff, nm)
+		}
+	}
+
+	for nm, _ := range add {
+		// This new HNS is not in the old HNS list.
+		diff = append(diff, nm)
+	}
+
+	ns.HNSes = hnsnms
+	return
+}
+
 // SetOriginalObject updates or creates the original object in the namespace in the forest.
 func (ns *Namespace) SetOriginalObject(obj *unstructured.Unstructured) {
 	gvk := obj.GroupVersionKind()

incubator/hnc/pkg/kubectl/describe.go

@@ -31,6 +31,7 @@ var describeCmd = &cobra.Command{
 		nnm := args[0]
 		fmt.Printf("Hierarchy configuration for namespace %s\n", nnm)
 		hier := client.getHierarchy(nnm)
+		hnsnms := client.getHierarchicalNamespacesNames(nnm)
 
 		// Parent
 		if hier.Spec.Parent != "" {
@@ -44,11 +45,11 @@ var describeCmd = &cobra.Command{
 		for _, cn := range hier.Status.Children {
 			childrenAndStatus[cn] = ""
 		}
-		for _, cn := range hier.Spec.RequiredChildren {
+		for _, cn := range hnsnms {
 			if _, ok := childrenAndStatus[cn]; ok {
-				childrenAndStatus[cn] = "required"
+				childrenAndStatus[cn] = "Owned"
 			} else {
-				childrenAndStatus[cn] = "MISSING"
+				childrenAndStatus[cn] = "Missing"
 			}
 		}
 		if len(childrenAndStatus) > 0 {

incubator/hnc/pkg/kubectl/root.go

@@ -23,6 +23,7 @@ import (
 	"github.com/spf13/cobra"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/client-go/kubernetes"
@@ -44,6 +45,7 @@ type Client interface {
 	getHierarchy(nnm string) *api.HierarchyConfiguration
 	updateHierarchy(hier *api.HierarchyConfiguration, reason string)
 	createHierarchicalNamespace(nnm string, hnnm string)
+	getHierarchicalNamespacesNames(nnm string) []string
 }
 
 func init() {
@@ -87,7 +89,6 @@ func init() {
 	rootCmd.AddCommand(newSetCmd())
 	rootCmd.AddCommand(newDescribeCmd())
 	rootCmd.AddCommand(newTreeCmd())
-	rootCmd.AddCommand(newCreateCmd())
 	rootCmd.AddCommand(newCreate2Cmd())
 }
 
@@ -121,6 +122,27 @@ func (cl *realClient) getHierarchy(nnm string) *api.HierarchyConfiguration {
 	return hier
 }
 
+func (cl *realClient) getHierarchicalNamespacesNames(nnm string) []string {
+	var hnsnms []string
+
+	// List all the hns instance in the namespace.
+	ul := &unstructured.UnstructuredList{}
+	ul.SetKind(api.HierarchicalNamespacesKind)
+	ul.SetAPIVersion(api.HierarchicalNamespacesAPIVersion)
+	err := hncClient.Get().Resource(api.HierarchicalNamespaces).Namespace(nnm).Do().Into(ul)
+	if err != nil && !errors.IsNotFound(err) {
+		fmt.Printf("Error listing hierarchicalNamespaces for %s: %s\n", nnm, err)
+		os.Exit(1)
+	}
+
+	// Create a list of strings of the hns names.
+	for _, inst := range ul.Items {
+		hnsnms = append(hnsnms, inst.GetName())
+	}
+
+	return hnsnms
+}
+
 func (cl *realClient) updateHierarchy(hier *api.HierarchyConfiguration, reason string) {
 	nnm := hier.Namespace
 	var err error
@@ -146,12 +168,3 @@ func (cl *realClient) createHierarchicalNamespace(nnm string, hnnm string) {
 	}
 	fmt.Printf("Successfully created \"%s\" hierarchicalnamespace instance in \"%s\" namespace\n", hnnm, nnm)
 }
-
-func childNamespaceExists(hier *api.HierarchyConfiguration, cn string) bool {
-	for _, n := range hier.Spec.RequiredChildren {
-		if cn == n {
-			return true
-		}
-	}
-	return false
-}