Skip to content
This repository was archived by the owner on Apr 17, 2025. It is now read-only.

[WIP] Propagation of labels #48

Closed
wants to merge 2 commits into from
Closed

[WIP] Propagation of labels #48

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dba121e
[WIP] using typed-ssa with k8s 1.21
zoetrope Jun 15, 2021
1f7d58d
[WIP] fix
zoetrope Jun 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 51 additions & 25 deletions internal/reconcilers/anchor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,19 @@ import (
"context"
"errors"
"fmt"
"strings"

"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"

api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
"sigs.k8s.io/hierarchical-namespaces/internal/config"
"sigs.k8s.io/hierarchical-namespaces/internal/forest"
Expand Down Expand Up @@ -106,17 +107,11 @@ func (r *AnchorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, err
}

// If the subnamespace doesn't exist, create it.
if inst.Status.State == api.Missing {
if err := r.writeNamespace(ctx, log, nm, pnm); err != nil {
// Write the "Missing" state to the anchor status if the subnamespace
// cannot be created for some reason. Without it, the anchor status will
// remain empty by default.
if anchorErr := r.writeInstance(ctx, log, inst); anchorErr != nil {
log.Error(anchorErr, "while setting anchor state", "state", api.Missing, "reason", err)
}
return ctrl.Result{}, err
if err := r.updateNamespace(ctx, log, nm, pnm); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this change only affects subnamespaces (e.g., child namespaces created via an Anchor), and not all descendant namespaces (e.g., child namespaces that are created normally and then assigned a parent). See here for the difference.

As a result, this should probably go in hierarchy_config.go, not here.

if anchorErr := r.writeInstance(ctx, log, inst); anchorErr != nil {
log.Error(anchorErr, "while setting anchor state", "state", api.Missing, "reason", err)
}
return ctrl.Result{}, err
}

// Add finalizers on all non-forbidden anchors to ensure it's not deleted until
Expand Down Expand Up @@ -369,18 +364,33 @@ func (r *AnchorReconciler) getNamespace(ctx context.Context, nm string) (*corev1
return ns, nil
}

func (r *AnchorReconciler) writeNamespace(ctx context.Context, log logr.Logger, nm, pnm string) error {
inst := &corev1.Namespace{}
func (r *AnchorReconciler) updateNamespace(ctx context.Context, log logr.Logger, nm, pnm string) error {
pnmInst := &corev1.Namespace{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this should be pnsInst ("Parent NameSpace Instance").

pnnm := types.NamespacedName{Name: pnm}
if err := r.Get(ctx, pnnm, pnmInst); err != nil {
return err
}

inst := &corev1.Namespace{
TypeMeta: metav1.TypeMeta{
Kind: "Namespace",
APIVersion: corev1.SchemeGroupVersion.String(),
},
}
inst.ObjectMeta.Name = nm
metadata.SetAnnotation(inst, api.SubnamespaceOf, pnm)
for k, v := range pnmInst.Labels {
if strings.Contains(k, api.MetaGroup) {
continue
}
metadata.SetLabel(inst, k, v)
}

// It's safe to use create here since if the namespace is created by someone
// else while this reconciler is running, returning an error will trigger a
// retry. The reconciler will set the 'Conflict' state instead of recreating
// this namespace. All other transient problems should trigger a retry too.
log.Info("Creating subnamespace")
if err := r.Create(ctx, inst); err != nil {
log.Error(err, "While creating subnamespace")
log.Info("Updating subnamespace")
if err := r.Patch(ctx, inst, client.Apply, &client.PatchOptions{
FieldManager: api.MetaGroup,
}); err != nil {
Comment on lines -377 to +392
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of things here:

  • This was a very important comment, are you sure it's safe to delete?
  • Where does the subnamespace get created if you've taken away the "Create" call?
  • We use "r.Update" everywhere in HNC, why are you using "r.Patch" here? If this is important, there should be a very clear comment explaining why, otherwise someone will "fix" it in the future.

log.Error(err, "While updating subnamespace")
return err
}
return nil
Expand All @@ -397,15 +407,31 @@ func (r *AnchorReconciler) deleteNamespace(ctx context.Context, log logr.Logger,
func (r *AnchorReconciler) SetupWithManager(mgr ctrl.Manager) error {
// Maps an subnamespace to its anchor in the parent namespace.
nsMapFn := func(obj client.Object) []reconcile.Request {
if obj.GetAnnotations()[api.SubnamespaceOf] == "" {
return nil
result := make([]reconcile.Request, 0)
nss := &corev1.NamespaceList{}
err := r.List(context.Background(), nss)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will result in every namespace being processed every time any namespace is changed, with no timeout. Is this really what you want? Note that the Forest already has all the namespaces in memory.

if err != nil {
return result
}
// Fetch subnamespaces
for _, ns := range nss.Items {
if obj.GetName() == ns.Name {
continue
}
if _, ok := ns.Labels[obj.GetName()+api.LabelTreeDepthSuffix]; ok {
result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{
Name: ns.Name,
Namespace: ns.GetAnnotations()[api.SubnamespaceOf],
}})
}
}
return []reconcile.Request{
{NamespacedName: types.NamespacedName{
if obj.GetAnnotations()[api.SubnamespaceOf] != "" {
result = append(result, reconcile.Request{NamespacedName: types.NamespacedName{
Name: obj.GetName(),
Namespace: obj.GetAnnotations()[api.SubnamespaceOf],
}},
}})
}
return result
}
return ctrl.NewControllerManagedBy(mgr).
For(&api.SubnamespaceAnchor{}).
Expand Down