Let subns with propagated objects be deleted

See issue kubernetes-retired#1130. The HNC object validator was preventing the K8s
namespace controller from cleaning out a namespace to allow it to be
deleted. This change lets HNC understand that when a namespace is being
deleted, we should ignore the subnamespaceOf annotation. This allows the
HierarchyConfiguration object to be reset to its initial state (like a
full namespace would when it's being deleted), orphaning the namespace
and allowing all its propagated objects to be removed.

Tested: added new tests and ran them multiple times while watching the
logs. The subnamespace test failed without this change and passed every
time with it, with the logs showing the expected behaviour.

incubator/hnc/internal/reconcilers/hierarchy_config.go

@@ -273,6 +273,21 @@ func (r *HierarchyConfigReconciler) syncSubnamespaceParent(log logr.Logger, inst
 	}
 
 	pnm := nsInst.Annotations[api.SubnamespaceOf]
+
+	// Issue #1130: as a subnamespace is being deleted (e.g. because its anchor was deleted), ignore
+	// the annotation. K8s will remove the HC, which will effectively orphan this namespace, prompting
+	// HNC to remove all propagated objects, allowing it to be deleted cleanly. Without this, HNC
+	// would continue to think that all propagated objects needed to be protected from deletion and
+	// would prevent K8s from emptying and removing the namespace.
+	//
+	// We could also add an exception to allow K8s SAs to override the object validator (and we
+	// probably should), but this prevents us from getting into a war with K8s and is sufficient for
+	// v0.5.
+	if pnm != "" && !nsInst.DeletionTimestamp.IsZero() {
+		log.Info("Subnamespace is being deleted; ignoring SubnamespaceOf annotation", "parent", inst.Spec.Parent, "annotation", pnm)
+		pnm = ""
+	}
+
 	if pnm == "" {
 		ns.IsSub = false
 		return
@@ -546,6 +561,7 @@ func (r *HierarchyConfigReconciler) writeNamespace(ctx context.Context, log logr
 
 // updateObjects calls all type reconcillers in this namespace.
 func (r *HierarchyConfigReconciler) updateObjects(ctx context.Context, log logr.Logger, ns string) error {
+	log.Info("Namespace modified; updating all objects")
 	// Use mutex to guard the read from the types list of the forest to prevent the ConfigReconciler
 	// from modifying the list at the same time.
 	r.Forest.Lock()

incubator/hnc/test/e2e/issues_test.go

@@ -31,6 +31,25 @@ var _ = Describe("Issues", func() {
 			nsSubSub2, nsSubChild, nsSubSubChild)
 	}) 
 
+	// Note that this was never actually a problem (only subnamespaces were affected) but it seems
+	// like a good thing to test anyway.
+	It("Should delete full namespaces with propagated objects - issue #1130", func() {
+		MustRun("kubectl create ns", nsParent)
+		MustRun("kubectl create ns", nsChild)
+		MustRun("kubectl hns set", nsChild, "--parent", nsParent)
+		MustRun("kubectl create rolebinding admin-rb -n", nsParent,
+			"--clusterrole=admin --serviceaccount="+nsParent+":default")
+		MustRun("kubectl delete ns", nsChild)
+	})
+
+	It("Should delete subnamespaces with propagated objects - issue #1130", func() {
+		MustRun("kubectl create ns", nsParent)
+		MustRun("kubectl hns create", nsChild, "-n", nsParent)
+		MustRun("kubectl create rolebinding admin-rb -n", nsParent,
+			"--clusterrole=admin --serviceaccount="+nsParent+":default")
+		MustRun("kubectl delete subns", nsChild, "-n", nsParent)
+	})
+
 	It("Should remove obsolete conditions CannotPropagateObject and CannotUpdateObject - issue #328", func() {
 		// Setting up hierarchy with rolebinding that HNC doesn't have permission to copy.
 		MustRun("kubectl create ns", nsParent)