Skip to content
This repository has been archived by the owner on Jun 26, 2023. It is now read-only.

Commit

Permalink
Merge pull request #1139 from adrianludwin/allow-ns-delete
Browse files Browse the repository at this point in the history
Let subns with propagated objects be deleted
  • Loading branch information
k8s-ci-robot authored Sep 23, 2020
2 parents 62424dc + 2f2de46 commit 3ac6a10
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
16 changes: 16 additions & 0 deletions incubator/hnc/internal/reconcilers/hierarchy_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
19 changes: 19 additions & 0 deletions incubator/hnc/test/e2e/issues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 3ac6a10

Please sign in to comment.