Update HC Spec and HCR 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".

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 + "/owner"
 )
 
 // 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

@@ -263,15 +263,12 @@ func (ns *Namespace) ChildNames() []string {
 	return nms
 }
 
-// OwnedNames returns a list of names of the owned namespaces or nil if there's none.
+// OwnedNames returns a list of names of the owned namespaces.
 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)
+	for cnm, cns := range ns.forest.namespaces {
+		if cns.Owner == ns.name {
+			nms = append(nms, cnm)
 		}
 	}
 	return nms

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,7 +45,7 @@ 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"
 			} else {

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
-}

incubator/hnc/pkg/kubectl/set.go

@@ -18,7 +18,6 @@ package kubectl
 import (
 	"fmt"
 	"os"
-	"sort"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -31,8 +30,6 @@ type hcUpdates struct {
 	root                 bool
 	parent               string
 	allowCascadingDelete bool
-	requiredChildren     []string
-	optionalChildren     []string
 }
 
 var setCmd = &cobra.Command{
@@ -44,17 +41,11 @@ var setCmd = &cobra.Command{
 		flags := cmd.Flags()
 		parent, _ := flags.GetString("parent")
 		allowCascadingDelete, _ := flags.GetBool("allowCascadingDelete")
-		requiredChildren, _ := flags.GetStringArray("requiredChild")
-		optionalChildren, _ := flags.GetStringArray("optionalChild")
-		requiredChildren = normalizeStringArray(requiredChildren)
-		optionalChildren = normalizeStringArray(optionalChildren)
 
 		updates := hcUpdates{
 			root:                 flags.Changed("root"),
 			parent:               parent,
 			allowCascadingDelete: allowCascadingDelete,
-			requiredChildren:     requiredChildren,
-			optionalChildren:     optionalChildren,
 		}
 
 		updateHC(client, updates, nnm)
@@ -82,14 +73,6 @@ func updateHC(cl Client, updates hcUpdates, nnm string) {
 
 	setAllowCascadingDelete(hc, nnm, oldacd, updates.allowCascadingDelete, &numChanges)
 
-	if len(updates.requiredChildren) != 0 {
-		setRequiredChildren(hc, updates.requiredChildren, nnm, &numChanges)
-	}
-
-	if len(updates.optionalChildren) != 0 {
-		setOptionalChildren(hc, updates.optionalChildren, nnm, &numChanges)
-	}
-
 	if numChanges > 0 {
 		cl.updateHierarchy(hc, fmt.Sprintf("update the hierarchical configuration of %s", nnm))
 		word := "property"
@@ -136,47 +119,6 @@ func setAllowCascadingDelete(hc *api.HierarchyConfiguration, nnm string, oldacd,
 	}
 }
 
-func setRequiredChildren(hc *api.HierarchyConfiguration, rcns []string, nnm string, numChanges *int) {
-	for _, rcn := range rcns {
-		if childNamespaceExists(hc, rcn) {
-			fmt.Printf("Required child %s already present in %s\n", rcn, nnm)
-			continue
-		}
-		hc.Spec.RequiredChildren = append(hc.Spec.RequiredChildren, rcn)
-		fmt.Printf("Adding required child (subnamespace) %s to %s\n", rcn, nnm)
-		*numChanges++
-	}
-	sort.Strings(hc.Spec.RequiredChildren)
-}
-
-func setOptionalChildren(hc *api.HierarchyConfiguration, ocns []string, nnm string, numChanges *int) {
-	existingRCs := map[string]bool{}
-	for _, rc := range hc.Spec.RequiredChildren {
-		existingRCs[rc] = true
-	}
-
-	for _, oc := range ocns {
-		if existingRCs[oc] {
-			fmt.Printf("Making %s a regular child of %s\n", oc, nnm)
-			delete(existingRCs, oc)
-			*numChanges++
-		} else {
-			fmt.Printf("%s is not a required child of %s\n", oc, nnm)
-		}
-	}
-
-	newRCs := []string{}
-	for k, _ := range existingRCs {
-		newRCs = append(newRCs, k)
-	}
-	if len(newRCs) == 0 {
-		hc.Spec.RequiredChildren = nil
-	} else {
-		sort.Strings(newRCs)
-		hc.Spec.RequiredChildren = newRCs
-	}
-}
-
 func normalizeStringArray(in []string) []string {
 	out := []string{}
 	for _, val := range in {
@@ -191,7 +133,5 @@ func newSetCmd() *cobra.Command {
 	setCmd.Flags().Bool("root", false, "Turns namespace into root namespace")
 	setCmd.Flags().String("parent", "", "Parent namespace")
 	setCmd.Flags().Bool("allowCascadingDelete", false, "Allows cascading deletion of its self-serve subnamespaces.")
-	setCmd.Flags().StringArray("requiredChild", []string{""}, "Specifies a required child (subnamespace). If the child does not exist, it will be created. Multiple values may be comma delimited.")
-	setCmd.Flags().StringArray("optionalChild", []string{""}, "Turns a required child namespaces into optional child namespaces. Multiple values may be comma delimited.")
 	return setCmd
 }