Reconcile deleted namespaces out of cluster quota status

Author: liggitt

pkg/quota/controller/clusterquotamapping/mapper.go

@@ -133,8 +133,13 @@ func (m *clusterQuotaMapper) removeQuota(quotaName string) {
 	delete(m.requiredQuotaToSelector, quotaName)
 	delete(m.completedQuotaToSelector, quotaName)
 	delete(m.quotaToNamespaces, quotaName)
-	for _, quotas := range m.namespaceToQuota {
-		quotas.Delete(quotaName)
+	for namespaceName, quotas := range m.namespaceToQuota {
+		if quotas.Has(quotaName) {
+			quotas.Delete(quotaName)
+			for _, listener := range m.listeners {
+				listener.RemoveMapping(quotaName, namespaceName)
+			}
+		}
 	}
 }
 
@@ -176,8 +181,13 @@ func (m *clusterQuotaMapper) removeNamespace(namespaceName string) {
 	delete(m.requiredNamespaceToLabels, namespaceName)
 	delete(m.completedNamespaceToLabels, namespaceName)
 	delete(m.namespaceToQuota, namespaceName)
-	for _, namespaces := range m.quotaToNamespaces {
-		namespaces.Delete(namespaceName)
+	for quotaName, namespaces := range m.quotaToNamespaces {
+		if namespaces.Has(namespaceName) {
+			namespaces.Delete(namespaceName)
+			for _, listener := range m.listeners {
+				listener.RemoveMapping(quotaName, namespaceName)
+			}
+		}
 	}
 }
 

pkg/quota/controller/clusterquotareconciliation/reconciliation_controller.go

@@ -179,8 +179,19 @@ func (c *ClusterQuotaReconcilationController) calculateAll() {
 	}
 
 	for _, quota := range quotas {
+		// If we have namespaces we map to, force calculating those namespaces
 		namespaces, _ := c.clusterQuotaMapper.GetNamespacesFor(quota.Name)
-		c.forceCalculation(quota.Name, namespaces...)
+		if len(namespaces) > 0 {
+			c.forceCalculation(quota.Name, namespaces...)
+			continue
+		}
+
+		// If the quota status has namespaces when our mapper doesn't think it should,
+		// add it directly to the queue without any work items
+		if quota.Status.Namespaces.OrderedKeys().Front() != nil {
+			c.queue.AddWithData(quota.Name)
+			continue
+		}
 	}
 }
 
@@ -285,6 +296,17 @@ func (c *ClusterQuotaReconcilationController) syncQuotaForNamespaces(originalQuo
 		quota.Status.Namespaces.Insert(namespaceName, recalculatedStatus)
 	}
 
+	// Remove any namespaces from quota.status that no longer match.
+	// Needed because we will never get workitems for namespaces that no longer exist if we missed the delete event (e.g. on startup)
+	for e := quota.Status.Namespaces.OrderedKeys().Front(); e != nil; e = e.Next() {
+		namespaceName := e.Value.(string)
+		namespaceTotals, _ := quota.Status.Namespaces.Get(namespaceName)
+		if !matchingNamespaceNames.Has(namespaceName) {
+			quota.Status.Total.Used = utilquota.Subtract(quota.Status.Total.Used, namespaceTotals.Used)
+			quota.Status.Namespaces.Remove(namespaceName)
+		}
+	}
+
 	quota.Status.Total.Hard = quota.Spec.Quota.Hard
 
 	// if there's no change, no update, return early.  NewAggregate returns nil on empty input

pkg/quota/controller/clusterquotareconciliation/reconciliation_controller_test.go

@@ -161,7 +161,7 @@ func TestSyncFunc(t *testing.T) {
 			expectedRetries: []workItem{},
 		},
 		{
-			name: "update one, remove two, ignore three, fail four",
+			name: "update one, remove two, ignore three, fail four, remove deleted",
 			startingQuota: func() *quotaapi.ClusterResourceQuota {
 				ret := defaultQuota()
 				ret.Status.Total.Hard = ret.Spec.Quota.Hard
@@ -178,6 +178,10 @@ func TestSyncFunc(t *testing.T) {
 					Hard: ret.Spec.Quota.Hard,
 					Used: kapi.ResourceList{kapi.ResourcePods: resource.MustParse("15")},
 				})
+				ret.Status.Namespaces.Insert("deleted", kapi.ResourceQuotaStatus{
+					Hard: ret.Spec.Quota.Hard,
+					Used: kapi.ResourceList{kapi.ResourcePods: resource.MustParse("0")},
+				})
 				return ret
 			},
 			workItems: []workItem{

test/cmd/quota.sh

@@ -29,9 +29,9 @@ os::cmd::expect_success 'oc new-project asmail [email protected]'
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n bar --as deads -o name' "for-deads-by-annotation"
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n foo --as deads -o name' "for-deads-by-annotation"
 os::cmd::try_until_text 'oc get appliedclusterresourcequota -n asmail --as [email protected] -o name' "for-deads-email-by-annotation"
-os::cmd::try_until_text 'oc describe appliedclusterresourcequota/for-deads-by-annotation -n bar --as deads' "secrets.*[1-4][0-9]"
-
+os::cmd::try_until_text 'oc describe appliedclusterresourcequota/for-deads-by-annotation -n bar --as deads' "secrets.*1[0-9]"
 os::cmd::expect_success 'oc delete project foo'
+os::cmd::try_until_text 'oc get clusterresourcequota/for-deads-by-annotation -o jsonpath="{.status.namespaces[*].Namespace}"' '^bar$'
 os::cmd::expect_success 'oc delete project bar'
 os::cmd::expect_success 'oc delete project asmail'