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

Commit

Permalink
Rename ns.RequiredChildOf to ns.Owner in forest
Browse files Browse the repository at this point in the history
Rename ns.Owner field so that it can be used in the cascading deletion
implementation.

Tested by 'make test' and 'make test HNS=1'
  • Loading branch information
yiqigao217 committed Mar 18, 2020
1 parent c026647 commit 608289d
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 54 deletions.
7 changes: 3 additions & 4 deletions incubator/hnc/pkg/forest/forest.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,9 @@ type Namespace struct {
// on this namespace.
conditions conditions

// TODO rename it to Owner. See issue - https://github.com/kubernetes-sigs/multi-tenancy/issues/469
// RequiredChildOf indicates that this namespace is being or was created solely to live as a
// Owner indicates that this namespace is being or was created solely to live as a
// subnamespace of the specified parent.
RequiredChildOf string
Owner string
}

type condition struct {
Expand Down Expand Up @@ -271,7 +270,7 @@ func (ns *Namespace) OwnedNames() []string {
}
nms := []string{}
for nm, ns := range ns.forest.namespaces {
if ns.RequiredChildOf == ns.name {
if ns.Owner == ns.name {
nms = append(nms, nm)
}
}
Expand Down
6 changes: 2 additions & 4 deletions incubator/hnc/pkg/reconcilers/hierarchical_namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,7 @@ func (r *HierarchicalNamespaceReconciler) syncMissing(log logr.Logger, inst *api

// Set the "Owner" in the forest of the hierarchical namespace to the current namespace.
log.Info("Setting the subnamespace's owner in the forest", "owner", pnm, "namespace", nm)
// TODO rename RequiredChildOf to Owner in the forest. See issue:
// https://github.com/kubernetes-sigs/multi-tenancy/issues/469
ns.RequiredChildOf = pnm
ns.Owner = pnm

// Enqueue the not-yet existent self-serve subnamespace. The HierarchyConfig reconciler will
// create the namespace and the HierarchyConfig instances on apiserver accordingly.
Expand All @@ -158,7 +156,7 @@ func (r *HierarchicalNamespaceReconciler) syncExisting(log logr.Logger, inst *ap
// or a human manually created the namespace with the right annotation but no HC.
// Both cases meant to create this namespace as HNS.
log.Info("Setting the subnamespace's owner in the forest", "owner", pnm, "namespace", nm)
ns.RequiredChildOf = pnm
ns.Owner = pnm
r.hcr.enqueueAffected(log, "updated subnamespace", nm)

// We will set it as "Conflict" though it's just a short transient state. Once the hc is
Expand Down
76 changes: 38 additions & 38 deletions incubator/hnc/pkg/reconcilers/hierarchy_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (r *HierarchyConfigReconciler) syncWithForest(log logr.Logger, nsInst *core

r.markExisting(log, ns)

r.syncRequiredChildOf(log, inst, ns)
r.syncOwner(log, inst, ns)
r.syncParent(log, inst, ns)

// Update the list of actual children, then resolve it versus the list of required children.
Expand All @@ -185,23 +185,23 @@ func (r *HierarchyConfigReconciler) onMissingNamespace(log logr.Logger, ns *fore
}

if !ns.Exists() {
// The namespace doesn't exist on the server, but it's been requested for a parent. Initialize
// The namespace doesn't exist on the server, but its owner expects it to be there. Initialize
// it so it gets created; once it is, it will be reconciled again.
if ns.RequiredChildOf != "" {
log.Info("Will create missing namespace", "forParent", ns.RequiredChildOf)
if ns.Owner != "" {
log.Info("Will create missing namespace", "forOwner", ns.Owner)
nsInst.Name = ns.Name()
// Set "api.AnnotationOwner" annotation to the non-existent yet namespace.
metadata.SetAnnotation(nsInst, api.AnnotationOwner, ns.RequiredChildOf)
metadata.SetAnnotation(nsInst, api.AnnotationOwner, ns.Owner)
}
return true
}

// Remove it from the forest and notify its relatives
r.enqueueAffected(log, "relative of deleted namespace", ns.RelativesNames()...)
// Enqueue the HNS if the self-serve subnamespace is deleted.
// Enqueue the HNS if the owned namespace is deleted.
if r.HNSReconcilerEnabled {
if ns.RequiredChildOf != "" {
r.hnsr.enqueue(log, ns.Name(), ns.RequiredChildOf, "hns for the deleted self-serve subnamespace")
if ns.Owner != "" {
r.hnsr.enqueue(log, ns.Name(), ns.Owner, "hns for the deleted owned namespace")
}
}
ns.UnsetExists()
Expand All @@ -215,42 +215,42 @@ func (r *HierarchyConfigReconciler) markExisting(log logr.Logger, ns *forest.Nam
if ns.SetExists() {
log.Info("Reconciling new namespace")
r.enqueueAffected(log, "relative of newly synced/created namespace", ns.RelativesNames()...)
if ns.RequiredChildOf != "" {
r.enqueueAffected(log, "parent of newly synced/created required subnamespace", ns.RequiredChildOf)
if ns.Owner != "" {
r.enqueueAffected(log, "owner of the newly synced/created namespace", ns.Owner)
}
}
}

// syncRequiredChildOf propagates the required child value from the forest to the spec if possible
// (the spec itself will be synced next), or removes the requiredChildOf value if there's a problem
// syncOwner propagates the required child value from the forest to the spec if possible
// (the spec itself will be synced next), or removes the owner value if there's a problem
// and notifies the would-be parent namespace.
func (r *HierarchyConfigReconciler) syncRequiredChildOf(log logr.Logger, inst *api.HierarchyConfiguration, ns *forest.Namespace) {
if ns.RequiredChildOf == "" {
func (r *HierarchyConfigReconciler) syncOwner(log logr.Logger, inst *api.HierarchyConfiguration, ns *forest.Namespace) {
if ns.Owner == "" {
return
}

switch inst.Spec.Parent {
case "":
log.Info("Required subnamespace: initializing", "parent", ns.RequiredChildOf)
inst.Spec.Parent = ns.RequiredChildOf
case ns.RequiredChildOf:
log.Info("Owned namespace: initializing", "owner", ns.Owner)
inst.Spec.Parent = ns.Owner
case ns.Owner:
// ok
if r.HNSReconcilerEnabled {
// Enqueue HNS to update the state to "Ok".
r.hnsr.enqueue(log, ns.Name(), ns.RequiredChildOf, "the HNS state should be updated to ok")
r.hnsr.enqueue(log, ns.Name(), ns.Owner, "the HNS state should be updated to ok")
}
default:
if r.HNSReconcilerEnabled {
// Enqueue the HNS to report the conflict in hierarchicalnamespace.Status.State and enqueue the
// owner namespace to report the "SubnamespaceConflict" condition.
log.Info("Self-serve subnamespace: conflict with parent", "owner", ns.RequiredChildOf, "parent", inst.Spec.Parent)
r.hnsr.enqueue(log, ns.Name(), ns.RequiredChildOf, "self-serve subnamespace has a parent but it's not the owner")
r.enqueueAffected(log, "required child already has a parent", ns.RequiredChildOf)
log.Info("Owned namespace: conflict with parent", "owner", ns.Owner, "parent", inst.Spec.Parent)
r.hnsr.enqueue(log, ns.Name(), ns.Owner, "owned namespace has a parent but it's not the owner")
r.enqueueAffected(log, "owned namespace already has a parent", ns.Owner)
} else {
// This should never happen unless there's some crazy race condition.
log.Info("Required subnamespace: conflict with existing parent", "requiredChildOf", ns.RequiredChildOf, "actual", inst.Spec.Parent)
r.enqueueAffected(log, "required child already has a parent", ns.RequiredChildOf)
ns.RequiredChildOf = "" // get back in sync with apiserver
log.Info("Owned namespace: conflict with existing parent", "owner", ns.Owner, "actualParent", inst.Spec.Parent)
r.enqueueAffected(log, "owned namespace already has a parent", ns.Owner)
ns.Owner = "" // get back in sync with apiserver
}
}

Expand Down Expand Up @@ -355,17 +355,17 @@ func (r *HierarchyConfigReconciler) syncChildren(log logr.Logger, inst *api.Hier
for _, cn := range inst.Status.Children {
cns := r.Forest.Get(cn)

if cns.RequiredChildOf != "" && cns.RequiredChildOf != ns.Name() {
if cns.Owner != "" && cns.Owner != ns.Name() {
// Since the list of children of this namespace came from the forest, this implies that the
// in-memory forest is out of sync with itself: requiredChildOf != parent. Obviously, this
// in-memory forest is out of sync with itself: owner != parent. Obviously, this
// should never happen.
//
// Let's just log an error and enqueue the other namespace so it can report the condition.
// The forest will be reset to the correct value, below.
log.Error(forest.OutOfSync, "While syncing children", "child", cn, "requiredChildOf", cns.RequiredChildOf)
r.enqueueAffected(log, "forest out-of-sync: requiredChildOf != parent", cns.RequiredChildOf)
log.Error(forest.OutOfSync, "While syncing children", "child", cn, "owner", cns.Owner)
r.enqueueAffected(log, "forest out-of-sync: owner != parent", cns.Owner)
if r.HNSReconcilerEnabled {
r.hnsr.enqueue(log, cn, cns.RequiredChildOf, "forest out-of-sync: owner != parent")
r.hnsr.enqueue(log, cn, cns.Owner, "forest out-of-sync: owner != parent")
}
}

Expand All @@ -375,23 +375,23 @@ func (r *HierarchyConfigReconciler) syncChildren(log logr.Logger, inst *api.Hier
}
} else {
if isRequired[cn] {
// This child is actually required. Remove it from the set so we know we found it. Also, the
// This child is actually owned. Remove it from the set so we know we found it. Also, the
// forest is almost certainly already in sync, but just set it again in case something went
// wrong (eg, the error shown above).
delete(isRequired, cn)
cns.RequiredChildOf = ns.Name()
cns.Owner = ns.Name()
} else {
// This isn't a required child, but it looks like it *used* to be a required child of this
// namespace. Clear the RequiredChildOf field from the forest to bring our state in line with
// This isn't an owned child, but it looks like it *used* to be an owned child of this
// namespace. Clear the Owner field from the forest to bring our state in line with
// what's on the apiserver.
cns.RequiredChildOf = ""
cns.Owner = ""
}
}
}

if r.HNSReconcilerEnabled {
for cn := range isRequired {
r.hnsr.enqueue(log, cn, ns.Name(), "parent of the self-serve subnamespace is not the owner")
r.hnsr.enqueue(log, cn, ns.Name(), "parent of the owned namespace is not the owner")
}
} else {
// Anything that's still in isRequired at this point is a required child according to our own
Expand All @@ -410,15 +410,15 @@ func (r *HierarchyConfigReconciler) syncChildren(log logr.Logger, inst *api.Hier

// If this child isn't claimed by another parent, claim it and make sure it gets reconciled
// (which is when it will be created).
if cns.Parent() == nil && (cns.RequiredChildOf == "" || cns.RequiredChildOf == ns.Name()) {
cns.RequiredChildOf = ns.Name()
if cns.Parent() == nil && (cns.Owner == "" || cns.Owner == ns.Name()) {
cns.Owner = ns.Name()
r.enqueueAffected(log, "required child is missing", cn)
if !cns.Exists() {
msg = fmt.Sprintf("required subnamespace %s does not exist", cn)
}
} else {
// Someone else got it first. This should never happen if the validator is working correctly.
other := cns.RequiredChildOf
other := cns.Owner
if other == "" {
other = cns.Parent().Name()
}
Expand Down
8 changes: 4 additions & 4 deletions incubator/hnc/pkg/validators/hierarchy.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ func (v *Hierarchy) checkParent(ns, curParent, newParent *forest.Namespace) admi
return deny(metav1.StatusReasonConflict, "Illegal parent: "+reason)
}

// Prevent changing parent of a required child
if ns.RequiredChildOf != "" && ns.RequiredChildOf != newParent.Name() {
reason := fmt.Sprintf("Cannot set the parent of %q to %q because it's a required child of %q", ns.Name(), newParent.Name(), ns.RequiredChildOf)
// Prevent changing parent of an owned child
if ns.Owner != "" && ns.Owner != newParent.Name() {
reason := fmt.Sprintf("Cannot set the parent of %q to %q because it's a self-serve subnamespace of %q", ns.Name(), newParent.Name(), ns.Owner)
return deny(metav1.StatusReasonConflict, "Illegal parent: "+reason)
}

Expand All @@ -186,7 +186,7 @@ func (v *Hierarchy) checkRequiredChildren(ns *forest.Namespace, requiredChildren
continue
}
// If this is already a child, or is about to be, no problem.
if cns.Parent() == ns || (cns.Parent() == nil && cns.RequiredChildOf == ns.Name()) {
if cns.Parent() == ns || (cns.Parent() == nil && cns.Owner == ns.Name()) {
continue
}
reason := fmt.Sprintf("Cannot set %q as the required child of %q because it already exists and is not a child of %q", cns.Name(), ns.Name(), ns.Name())
Expand Down
8 changes: 4 additions & 4 deletions incubator/hnc/pkg/validators/hierarchy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestStructure(t *testing.T) {
// -> fooRC (RequiredChild of foo)
foo := createNS(f, "foo", nil)
bar := createNS(f, "bar", nil)
createRCNS(f, "fooRC", foo)
createOwnedNS(f, "fooRC", foo)
createNS(f, "baz", nil)
bar.SetParent(foo)
h := &Hierarchy{Forest: f}
Expand Down Expand Up @@ -162,12 +162,12 @@ func createNS(f *forest.Forest, nnm string, ue []string) *forest.Namespace {
return ns
}

// createRCNS creates rc (requiredChild) and sets its parent and requiredChildOf to nm and sets it
// createOwnedNS creates a namespace and sets its parent and owner to nm and sets it
// to existing.
func createRCNS(f *forest.Forest, rc string, p *forest.Namespace) *forest.Namespace {
func createOwnedNS(f *forest.Forest, rc string, p *forest.Namespace) *forest.Namespace {
ns := f.Get(rc)
ns.SetParent(p)
ns.RequiredChildOf = p.Name()
ns.Owner = p.Name()
ns.SetExists()
return ns
}
Expand Down

0 comments on commit 608289d

Please sign in to comment.