Multiple scoped HRQs with the same name

internal/hrq/hrqreconciler.go

@@ -96,7 +96,11 @@ func (r *HierarchicalResourceQuotaReconciler) Reconcile(ctx context.Context, req
 
 	rqName := api.ResourceQuotaSingletonName
 	if r.Forest.IsMarkedAsScopedHRQ(req.NamespacedName) {
-		rqName = utils.ScopedRQName(inst.GetName())
+		rqName, err = utils.ScopedRQName(inst.GetNamespace(), inst.GetName())
+		if err != nil {
+			log.Error(err, "Couldn't get the scoped RQ name")
+			return ctrl.Result{}, err
+		}
 	}
 
 	// Enqueue ResourceQuota objects in the current namespace and its descendants
@@ -116,6 +120,8 @@ func (r *HierarchicalResourceQuotaReconciler) Reconcile(ctx context.Context, req
 // forest. The first return value is true if the HRQ object is updated; the
 // second return value is true if the forest is updated.
 func (r *HierarchicalResourceQuotaReconciler) syncWithForest(log logr.Logger, inst *api.HierarchicalResourceQuota) (bool, bool, error) {
+	var err error
+
 	r.Forest.Lock()
 	defer r.Forest.Unlock()
 
@@ -126,10 +132,16 @@ func (r *HierarchicalResourceQuotaReconciler) syncWithForest(log logr.Logger, in
 	if isScopedHRQ {
 		log.Info("Marking HRQ as scoped", "name", inst.GetName(), "namespace", inst.GetNamespace())
 		r.Forest.MarkScopedRQ(nn)
-		rqName = utils.ScopedRQName(inst.GetName())
+		rqName, err = utils.ScopedRQName(inst.GetNamespace(), inst.GetName())
+		if err != nil {
+			return false, false, err
+		}
 	} else if r.Forest.IsMarkedAsScopedHRQ(nn) {
 		log.Info("Detect the Scoped HRQ because of the mark")
-		rqName = utils.ScopedRQName(inst.GetName())
+		rqName, err = utils.ScopedRQName(inst.GetNamespace(), inst.GetName())
+		if err != nil {
+			return false, false, err
+		}
 	} else {
 		log.Info("HRQ is a singleton")
 	}

internal/hrq/hrqreconciler_test.go

@@ -150,7 +150,11 @@ var _ = Describe("HRQ reconciler tests", func() {
 
 		// Scoped HRQs don't affect the result of TestCheckHRQDrift.
 		forestOverrideSubtreeUsages("hrq-selector", "cpu", "3")
-		updateRQUsage(ctx, fooName, utils.ScopedRQName("hrq-selector"), "cpu", "5")
+
+		scopedRQ, err := utils.ScopedRQName(fooName, "hrq-selector")
+		Expect(err).NotTo(HaveOccurred())
+		updateRQUsage(ctx, fooName, scopedRQ, "cpu", "5")
+
 		drift, err = TestCheckHRQDrift()
 		Expect(err).NotTo(HaveOccurred())
 		Expect(drift).Should(BeFalse())

internal/hrq/rqreconciler.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+	"time"
 
 	"github.com/go-logr/logr"
 	v1 "k8s.io/api/core/v1"
@@ -83,6 +84,12 @@ func (r *ResourceQuotaReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	}
 
 	isSingleton := utils.IsSingletonRQ(inst)
+	isLegacyScoped := utils.IsLegacyScopedRQ(inst)
+
+	// Handle migration from legacy scoped RQ to new format
+	if !notFound && isLegacyScoped {
+		return r.migrateLegacyScopedRQ(ctx, log, inst)
+	}
 
 	r.Forest.Lock()
 	ns := r.Forest.Get(inst.ObjectMeta.Namespace)
@@ -100,43 +107,31 @@ func (r *ResourceQuotaReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 
 	log.Info("Reconciling ResourceQuota", "name", fmt.Sprintf("%s/%s", inst.GetNamespace(), inst.GetName()), "limits", inst.Spec.Hard, "usages", inst.Status.Used, "updated", updated)
 
+	var hrq *api.HierarchicalResourceQuota
+
 	// Delete the obsolete singleton and early exit if the new limits are empty.
 	if inst.Spec.Hard == nil {
 		return ctrl.Result{}, r.deleteRQ(ctx, log, inst)
 	} else if !isSingleton && notFound {
-		hrq := &api.HierarchicalResourceQuota{}
-		hrqName, err := utils.ScopedHRQNameFromHRQName(inst.Name)
+		hrqnnm, err := r.findHRQNameForRQ(inst)
 		if err != nil {
-			return ctrl.Result{}, fmt.Errorf("while getting hrq name: %w", err)
+			return ctrl.Result{}, fmt.Errorf("while finding hrq name for new RQ: %w", err)
 		}
 
-		cursorNm := ns
-		var found bool
-		for {
-			if cursorNm == nil {
-				break
-			}
-
-			hrqnnm := types.NamespacedName{Namespace: cursorNm.Name(), Name: hrqName}
-			err := r.Get(ctx, hrqnnm, hrq)
-			if err == nil {
-				found = true
-				break
-			}
+		hrq = &api.HierarchicalResourceQuota{}
+		err = r.Get(ctx, hrqnnm, hrq)
+		if err != nil {
 			if apierrors.IsNotFound(err) {
-				cursorNm = cursorNm.Parent()
-				continue
+				return ctrl.Result{}, fmt.Errorf("the parent hrq not found: %s", hrqnnm)
+			} else {
+				return ctrl.Result{}, fmt.Errorf("getting hrq %s: %w", hrqnnm, err)
 			}
-
-			return ctrl.Result{}, fmt.Errorf("while getting hrq: %w", err)
-		}
-		if !found {
-			return ctrl.Result{}, fmt.Errorf("the parent hrq not found: %s", hrqName)
 		}
 
 		log.Info("Found the parent HRQ", "namespace", hrq.Namespace, "name", hrq.Name)
 
 		inst.Spec.ScopeSelector = hrq.Spec.ScopeSelector
+		utils.SetLabelsAnnotationsForScopedRQ(inst, hrq.Namespace, hrq.Name)
 	}
 
 	// We only need to write back to the apiserver if the spec has changed
@@ -166,6 +161,7 @@ func (r *ResourceQuotaReconciler) writeRQ(ctx context.Context, log logr.Logger,
 	if inst.CreationTimestamp.IsZero() {
 		// Set the cleanup label so the singleton can be deleted by selector later.
 		metadata.SetLabel(inst, api.HRQLabelCleanup, "true")
+
 		// Add a non-propagate exception annotation to the instance so that it won't
 		// be overwritten by ancestors, when the resource quota type is configured
 		// as Propagate mode in HNCConfig.
@@ -229,6 +225,38 @@ func (r *ResourceQuotaReconciler) getAncestorHRQs(inst *v1.ResourceQuota) []type
 	return names
 }
 
+// findHRQNameForRQ finds HRQ name and namespace for a new RQ by searching through namespace and ancestor HRQs
+func (r *ResourceQuotaReconciler) findHRQNameForRQ(inst *v1.ResourceQuota) (types.NamespacedName, error) {
+	hrqnnm, err := utils.ScopedHRQNameFromRQ(inst)
+	if err == nil {
+		return hrqnnm, nil
+	}
+
+	// New RQs that don't have the labels yet, so we need to search through the forest
+	r.Forest.Lock()
+	defer r.Forest.Unlock()
+
+	ns := r.Forest.Get(inst.Namespace)
+
+	// Check current namespace and all ancestors
+	namespaces := []string{inst.Namespace}
+	namespaces = append(namespaces, ns.AncestryNames()...)
+
+	for _, nsnm := range namespaces {
+		ancestorNS := r.Forest.Get(nsnm)
+		for _, hrqName := range ancestorNS.HRQNames() {
+			expectedRQName, err := utils.ScopedRQName(nsnm, hrqName)
+			if err != nil {
+				continue
+			}
+			if expectedRQName == inst.Name {
+				return types.NamespacedName{Namespace: nsnm, Name: hrqName}, nil
+			}
+		}
+	}
+	return types.NamespacedName{}, fmt.Errorf("no matching HRQ found for RQ: %s", inst.Name)
+}
+
 // deleteRQ deletes a resource quota on the apiserver and a quota in on-memory if it exists. Otherwise,
 // do nothing.
 func (r *ResourceQuotaReconciler) deleteRQ(ctx context.Context, log logr.Logger, inst *v1.ResourceQuota) error {
@@ -313,6 +341,89 @@ func (r *ResourceQuotaReconciler) syncResourceLimits(ns *forest.Namespace, inst
 	return true
 }
 
+// migrateLegacyScopedRQ handles migration from legacy scoped RQ format to new format
+func (r *ResourceQuotaReconciler) migrateLegacyScopedRQ(ctx context.Context, log logr.Logger, legacyRQ *v1.ResourceQuota) (ctrl.Result, error) {
+	// Double-check that this is actually an HNC-managed legacy RQ
+	if !utils.IsHNCManagedRQ(legacyRQ) {
+		log.Info("Skipping migration for non-HNC managed RQ", "rqName", legacyRQ.Name)
+		return ctrl.Result{}, nil
+	}
+
+	// Extract HRQ name from legacy RQ name
+	hrqName, err := utils.HRQNameFromLegacyRQName(legacyRQ.Name)
+	if err != nil {
+		log.Error(err, "Failed to extract HRQ name from legacy RQ", "rqName", legacyRQ.Name)
+		return ctrl.Result{}, err
+	}
+
+	// Find the corresponding HRQ in current namespace or ancestors
+	hrq, err := r.findHRQForMigration(ctx, log, legacyRQ.Namespace, hrqName)
+	if err != nil {
+		log.Error(err, "Failed to find HRQ for migration", "hrqName", hrqName)
+		return ctrl.Result{}, err
+	}
+
+	if hrq == nil {
+		log.Info("No corresponding HRQ found for legacy RQ", "namespace", legacyRQ.Namespace, "rqName", legacyRQ.Name, "hrqName", hrqName)
+		return ctrl.Result{}, nil
+	}
+
+	// Generate new RQ name
+	newRQName, err := utils.ScopedRQName(hrq.Namespace, hrq.Name)
+	if err != nil {
+		log.Error(err, "Failed to generate new RQ name")
+		return ctrl.Result{}, err
+	}
+
+	// Check if new RQ already exists
+	_, err = r.getRQ(ctx, legacyRQ.Namespace, newRQName)
+	if err != nil {
+		if apierrors.IsNotFound(err) {
+			// New RQ doesn't exist yet - the normal reconcile process will create it
+			// Requeue after a short delay to check again
+			log.Info("New format RQ not yet created, requeuing for retry", "legacyName", legacyRQ.Name, "expectedNewName", newRQName)
+			return ctrl.Result{RequeueAfter: time.Second * 5}, nil
+		} else {
+			log.Error(err, "Failed to check new RQ existence")
+			return ctrl.Result{}, err
+		}
+	}
+
+	// New RQ exists, safe to delete legacy RQ
+	log.Info("New format RQ exists, deleting legacy RQ", "legacyName", legacyRQ.Name, "newName", newRQName)
+	return ctrl.Result{}, r.deleteRQ(ctx, log, legacyRQ)
+}
+
+// findHRQForMigration finds the HRQ that corresponds to the legacy RQ
+func (r *ResourceQuotaReconciler) findHRQForMigration(ctx context.Context, log logr.Logger, namespace, hrqName string) (*api.HierarchicalResourceQuota, error) {
+	r.Forest.Lock()
+	ns := r.Forest.Get(namespace)
+	ancestryNames := ns.AncestryNames()
+	r.Forest.Unlock()
+
+	// Check current namespace first, then ancestors
+	namespaces := []string{namespace}
+	namespaces = append(namespaces, ancestryNames...)
+
+	for _, nsnm := range namespaces {
+		hrq := &api.HierarchicalResourceQuota{}
+		hrqKey := types.NamespacedName{Namespace: nsnm, Name: hrqName}
+
+		err := r.Get(ctx, hrqKey, hrq)
+		if err == nil && hrq.Spec.ScopeSelector != nil {
+			// Found scoped HRQ
+			log.Info("Found HRQ for migration", "hrqNamespace", nsnm, "hrqName", hrqName)
+			return hrq, nil
+		}
+
+		if err != nil && !apierrors.IsNotFound(err) {
+			return nil, fmt.Errorf("error checking HRQ %s/%s: %w", nsnm, hrqName, err)
+		}
+	}
+
+	return nil, nil
+}
+
 // OnChangeNamespace enqueues the singleton in a specific namespace to trigger the reconciliation of
 // the singleton for a given reason .  This occurs in a goroutine so the caller doesn't block; since
 // the reconciler is never garbage-collected, this is safe.

internal/hrq/rqreconciler_test.go

@@ -68,7 +68,8 @@ var _ = Describe("RQ reconciler tests", func() {
 		setHRQ(ctx, barHRQName, barName, nil, "secrets", "100", "cpu", "50")
 		setHRQ(ctx, bazHRQName, bazName, nil, "pods", "1")
 		hrqWithSelectorName := "hrq-with-selector"
-		rqName := fmt.Sprintf("%s-%s", api.ResourceQuotaSingletonName, hrqWithSelectorName)
+		rqName, err := utils.ScopedRQName(fooName, hrqWithSelectorName)
+		Expect(err).NotTo(HaveOccurred())
 		setHRQ(ctx, hrqWithSelectorName, fooName, &highPrioritySelector, "cpu", "4", "pods", "2")
 
 		Eventually(getRQHard(ctx, fooName, api.ResourceQuotaSingletonName)).Should(equalRL("secrets", "6", "pods", "3"))
@@ -89,7 +90,8 @@ var _ = Describe("RQ reconciler tests", func() {
 		setHRQ(ctx, bazHRQName, bazName, nil, "pods", "1")
 
 		hrqWithSelectorName := "hrq-with-selector"
-		rqName := fmt.Sprintf("%s-%s", api.ResourceQuotaSingletonName, hrqWithSelectorName)
+		rqName, err := utils.ScopedRQName(fooName, hrqWithSelectorName)
+		Expect(err).NotTo(HaveOccurred())
 		setHRQ(ctx, hrqWithSelectorName, fooName, &highPrioritySelector, "cpu", "6", "pods", "3")
 		setHRQ(ctx, hrqWithSelectorName, barName, &highPrioritySelector, "cpu", "100", "pods", "50")
 		setHRQ(ctx, hrqWithSelectorName, bazName, &highPrioritySelector, "pods", "1")

internal/hrq/utils/scopedhrq.go

@@ -1,12 +1,25 @@
 package utils
 
 import (
+	"crypto/md5"
+	"encoding/hex"
 	"fmt"
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
-
+	"k8s.io/apimachinery/pkg/types"
 	api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
+	"sigs.k8s.io/hierarchical-namespaces/internal/metadata"
+)
+
+const (
+	hrqNameLabel          = "hnc.x-k8s.io/hrq-name"
+	hrqNamespaceLabel     = "hnc.x-k8s.io/hrq-namespace"
+	newScopedRQAnnotation = "hnc.x-k8s.io/new-scoped-rq"
+)
+
+const (
+	maxRQNameLength = 253
 )
 
 func IsSingletonRQ(rq *v1.ResourceQuota) bool {
@@ -17,15 +30,76 @@ func IsScopedRQ(rq *v1.ResourceQuota) bool {
 	return !IsSingletonRQ(rq)
 }
 
-func ScopedRQName(hrqName string) string {
+// IsHNCManagedRQ checks if an RQ has the HNC cleanup label
+func IsHNCManagedRQ(rq *v1.ResourceQuota) bool {
+	if label, ok := metadata.GetLabel(rq, api.HRQLabelCleanup); ok {
+		return label == "true"
+	}
+	return false
+}
+
+// IsLegacyScopedRQ checks if an RQ is a legacy scoped RQ (old format without namespace)
+// and is managed by HNC
+func IsLegacyScopedRQ(rq *v1.ResourceQuota) bool {
+	if IsSingletonRQ(rq) {
+		return false
+	}
+	// Only consider RQs managed by HNC for migration
+	if !IsHNCManagedRQ(rq) {
+		return false
+	}
+	if v, ok := metadata.GetAnnotation(rq, newScopedRQAnnotation); ok && v == "true" {
+		return false
+	}
+	return true
+}
+
+// LegacyScopedRQName generates the legacy RQ name for backward compatibility
+func LegacyScopedRQName(hrqName string) string {
 	return api.ResourceQuotaSingletonName + "-" + hrqName
 }
 
-func ScopedHRQNameFromHRQName(rqName string) (string, error) {
+// HRQNameFromLegacyRQName extracts HRQ name from legacy RQ name
+func HRQNameFromLegacyRQName(rqName string) (string, error) {
+	if rqName == api.ResourceQuotaSingletonName {
+		return "", fmt.Errorf("invalid legacy RQ name: %s", rqName)
+	}
+
 	hrqName := strings.TrimPrefix(rqName, api.ResourceQuotaSingletonName+"-")
-	if hrqName == api.ResourceQuotaSingletonName {
-		return "", fmt.Errorf("invalid ScopedHRQ name: %s", hrqName)
+	if hrqName == rqName {
+		return "", fmt.Errorf("not a legacy scoped RQ name: %s", rqName)
 	}
 
 	return hrqName, nil
 }
+
+func ScopedRQName(hrqNamespace string, hrqName string) (string, error) {
+	hash := md5.Sum([]byte(fmt.Sprintf("%s/%s", hrqNamespace, hrqName)))
+	hashStr := hex.EncodeToString(hash[:])
+
+	namespaceAndName := truncate(fmt.Sprintf("%s-%s", hrqNamespace, hrqName), maxRQNameLength-len(hashStr)-len(api.ResourceQuotaSingletonName)-2)
+
+	return fmt.Sprintf("%s-%s-%s", api.ResourceQuotaSingletonName, namespaceAndName, hashStr), nil
+}
+
+func ScopedHRQNameFromRQ(rq *v1.ResourceQuota) (types.NamespacedName, error) {
+	namespace, nsOK := metadata.GetLabel(rq, hrqNamespaceLabel)
+	name, nameOK := metadata.GetLabel(rq, hrqNameLabel)
+	if nsOK && nameOK {
+		return types.NamespacedName{Namespace: namespace, Name: name}, nil
+	}
+	return types.NamespacedName{}, fmt.Errorf("no matching HRQ found for RQ: %s", rq.Name)
+}
+
+func SetLabelsAnnotationsForScopedRQ(rq *v1.ResourceQuota, hrqNamespace string, hrqName string) {
+	metadata.SetLabel(rq, hrqNamespaceLabel, hrqNamespace)
+	metadata.SetLabel(rq, hrqNameLabel, hrqName)
+	metadata.SetAnnotation(rq, newScopedRQAnnotation, "true")
+}
+
+func truncate(s string, n int) string {
+	if len(s) <= n {
+		return s
+	}
+	return s[:n]
+}