Skip to content

Commit da74cfd

Browse files
oliverbaehleroliverbaehler
authored
fix: allow replications to own replications without blocking controller updates (#2107)
* chore * perfromance improvements Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * perfromance improvements Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat(performance): removed duplicate client calls from all admission paths Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat(performance): removed duplicate client calls from all admission paths Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat(performance): removed duplicate client calls from all admission paths Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat(performance): removed duplicate client calls from all admission paths Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat(performance): removed duplicate client calls from all admission paths Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat(performance): removed duplicate client calls from all admission paths Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat(performance): removed duplicate client calls from all admission paths Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat: add globalresourcequota api Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat: add globalresourcequota api Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat: add globalresourcequota api Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat: add globalresourcequota api Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat: add globalresourcequota api Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * feat: add globalresourcequota api Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * fix: remove resource rejections checks Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * fix: allow replications to own replications without blocking controller updates Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * fix: allow replications to own replications without blocking controller updates Signed-off-by: Oliver Baehler <oliver@sudo-i.net> * fix: allow replications to own replications without blocking controller updates Signed-off-by: Oliver Baehler <oliver@sudo-i.net> --------- Signed-off-by: Oliver Baehler <oliver@sudo-i.net>
1 parent 7ee293a commit da74cfd

25 files changed

Lines changed: 951 additions & 110 deletions

‎Makefile‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,12 @@ dev-setup: dev-setup-flux-handoff
205205
--set 'certManager.generateCertificates=false' \
206206
--set 'tls.enableController=false' \
207207
--set 'tls.create=false' \
208+
--set rbac.resources.create=true \
209+
--set-string 'rbac.resources.labels.rbac\.authorization\.k8s\.io/aggregate-to-admin=true' \
210+
--set rbac.resourcepoolclaims.create=true \
211+
--set-string 'rbac.resourcepoolclaims.labels.rbac\.authorization\.k8s\.io/aggregate-to-admin=true' \
212+
--set rbac.customquotas.create=true \
213+
--set-string 'rbac.customquotas.labels.rbac\.authorization\.k8s\.io/aggregate-to-admin=true' \
208214
--set "webhooks.exclusive=true"\
209215
--set "webhooks.service.url=$${WEBHOOK_URL}" \
210216
--set "webhooks.service.caBundle=$${CA_BUNDLE}" \
@@ -480,6 +486,12 @@ e2e-install: helm-controller-version ko-build-all dev-install-gw-api-crds
480486
--set 'manager.options.leaderElection.leaseDuration=60s' \
481487
--set 'manager.options.leaderElection.renewDeadline=40s' \
482488
--set 'manager.rbac.minimal=true' \
489+
--set rbac.resources.create=true \
490+
--set-string 'rbac.resources.labels.rbac\.authorization\.k8s\.io/aggregate-to-admin=true' \
491+
--set rbac.resourcepoolclaims.create=true \
492+
--set-string 'rbac.resourcepoolclaims.labels.rbac\.authorization\.k8s\.io/aggregate-to-admin=true' \
493+
--set rbac.customquotas.create=true \
494+
--set-string 'rbac.customquotas.labels.rbac\.authorization\.k8s\.io/aggregate-to-admin=true' \
483495
--set 'webhooks.hooks.nodes.enabled=true' \
484496
--set "webhooks.exclusive=true"\
485497
--set 'webhooks.hooks.calculations.enabled=true' \

‎e2e/global_resource_quota_test.go‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ var _ = Describe("GlobalResourceQuota", Ordered, Label("globalresourcequota", "r
512512
))
513513
})
514514

515-
It("rejects direct hard-limit reductions and removals below allocated usage", func() {
515+
It("rejects direct hard-limit reductions below allocated usage and allows removals", func() {
516516
quotaKey := client.ObjectKey{Name: ephemeralQuotaName}
517517
Eventually(func(g Gomega) {
518518
current := &capsulev1beta2.GlobalResourceQuota{}
@@ -532,29 +532,33 @@ var _ = Describe("GlobalResourceQuota", Ordered, Label("globalresourcequota", "r
532532
)))
533533
})
534534

535-
By("rejecting removal of a resource with usage", func() {
535+
By("allowing a decrease exactly to allocated usage", func() {
536536
current := &capsulev1beta2.GlobalResourceQuota{}
537537
Expect(k8sClient.Get(ctx, quotaKey, current)).To(Succeed())
538-
delete(current.Spec.Quota.Hard, corev1.ResourceRequestsEphemeralStorage)
538+
current.Spec.Quota.Hard[corev1.ResourceRequestsEphemeralStorage] = resource.MustParse("600Mi")
539539

540-
err := k8sClient.Update(ctx, current)
541-
Expect(err).To(MatchError(ContainSubstring(
542-
`spec.quota.hard["requests.ephemeral-storage"] cannot be removed while 600Mi is allocated`,
543-
)))
540+
Expect(k8sClient.Update(ctx, current)).To(Succeed())
541+
Eventually(func(g Gomega) {
542+
reconciled := &capsulev1beta2.GlobalResourceQuota{}
543+
g.Expect(k8sClient.Get(ctx, quotaKey, reconciled)).To(Succeed())
544+
g.Expect(reconciled.Status.ObservedGeneration).To(Equal(reconciled.Generation))
545+
hard := reconciled.Status.Total.Hard[corev1.ResourceRequestsEphemeralStorage]
546+
g.Expect(hard.Cmp(resource.MustParse("600Mi"))).To(Equal(0))
547+
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
544548
})
545549

546-
By("allowing a decrease exactly to allocated usage", func() {
550+
By("allowing removal of a resource with usage", func() {
547551
current := &capsulev1beta2.GlobalResourceQuota{}
548552
Expect(k8sClient.Get(ctx, quotaKey, current)).To(Succeed())
549-
current.Spec.Quota.Hard[corev1.ResourceRequestsEphemeralStorage] = resource.MustParse("600Mi")
553+
delete(current.Spec.Quota.Hard, corev1.ResourceRequestsEphemeralStorage)
550554

551555
Expect(k8sClient.Update(ctx, current)).To(Succeed())
552556
Eventually(func(g Gomega) {
553557
reconciled := &capsulev1beta2.GlobalResourceQuota{}
554558
g.Expect(k8sClient.Get(ctx, quotaKey, reconciled)).To(Succeed())
555559
g.Expect(reconciled.Status.ObservedGeneration).To(Equal(reconciled.Generation))
556-
hard := reconciled.Status.Total.Hard[corev1.ResourceRequestsEphemeralStorage]
557-
g.Expect(hard.Cmp(resource.MustParse("600Mi"))).To(Equal(0))
560+
g.Expect(reconciled.Spec.Quota.Hard).NotTo(HaveKey(corev1.ResourceRequestsEphemeralStorage))
561+
g.Expect(reconciled.Status.Total.Hard).NotTo(HaveKey(corev1.ResourceRequestsEphemeralStorage))
558562
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
559563
})
560564
})

‎e2e/rules_quota_admission_test.go‎

Lines changed: 81 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -160,94 +160,87 @@ var _ = Describe("rule-generated GlobalResourceQuota admission", Ordered,
160160
EventuallyDeletion(tamperRole)
161161
})
162162

163-
It("rejects a Tenant rule update that decreases or removes a hard limit while changing scope", func() {
164-
for _, test := range []struct {
165-
name string
166-
mutate func(corev1.ResourceList)
167-
message string
168-
}{
169-
{
170-
name: "decrease",
171-
mutate: func(hard corev1.ResourceList) {
172-
hard[corev1.ResourceLimitsCPU] = resource.MustParse("0")
173-
},
174-
message: `rules[0].quota[0].hard["limits.cpu"] cannot be reduced from 8 to 0 while namespace selectors are changing`,
175-
},
176-
{
177-
name: "removal",
178-
mutate: func(hard corev1.ResourceList) {
179-
delete(hard, corev1.ResourceLimitsCPU)
180-
},
181-
message: `rules[0].quota[0].hard["limits.cpu"] cannot be removed while namespace selectors are changing`,
182-
},
183-
} {
184-
By(test.name, func() {
185-
Eventually(func() error {
186-
current := &capsulev1beta2.Tenant{}
187-
if err := k8sClient.Get(ctx, client.ObjectKey{Name: tenantName}, current); err != nil {
188-
return err
189-
}
190-
191-
updated := current.DeepCopy()
192-
updated.Spec.Rules[0].NamespaceSelector = nil
193-
test.mutate(updated.Spec.Rules[0].Quota[0].Hard)
194-
195-
return k8sClient.Update(ctx, updated)
196-
}, defaultTimeoutInterval, defaultPollInterval).Should(MatchError(ContainSubstring(test.message)))
197-
})
198-
}
163+
It("rejects a Tenant rule decrease while changing scope and allows removal", func() {
164+
By("rejecting the explicit decrease", func() {
165+
Eventually(func() error {
166+
current := &capsulev1beta2.Tenant{}
167+
if err := k8sClient.Get(ctx, client.ObjectKey{Name: tenantName}, current); err != nil {
168+
return err
169+
}
170+
171+
updated := current.DeepCopy()
172+
updated.Spec.Rules[0].NamespaceSelector = nil
173+
updated.Spec.Rules[0].Quota[0].Hard[corev1.ResourceLimitsCPU] = resource.MustParse("0")
174+
175+
return k8sClient.Update(ctx, updated)
176+
}, defaultTimeoutInterval, defaultPollInterval).Should(MatchError(ContainSubstring(
177+
`rules[0].quota[0].hard["limits.cpu"] cannot be reduced from 8 to 0 while namespace selectors are changing`,
178+
)))
179+
})
199180

200-
persisted := &capsulev1beta2.Tenant{}
201-
Expect(k8sClient.Get(ctx, client.ObjectKey{Name: tenantName}, persisted)).To(Succeed())
202-
Expect(persisted.Spec.Rules[0].NamespaceSelector).NotTo(BeNil())
203-
Expect(persisted.Spec.Rules[0].NamespaceSelector.MatchLabels).To(HaveKeyWithValue(selectorKey, "application"))
204-
persistedLimit := persisted.Spec.Rules[0].Quota[0].Hard[corev1.ResourceLimitsCPU]
205-
Expect(persistedLimit.Cmp(resource.MustParse("8"))).To(Equal(0))
181+
By("allowing the resource limit to be removed", func() {
182+
Eventually(func() error {
183+
current := &capsulev1beta2.Tenant{}
184+
if err := k8sClient.Get(ctx, client.ObjectKey{Name: tenantName}, current); err != nil {
185+
return err
186+
}
187+
188+
updated := current.DeepCopy()
189+
updated.Spec.Rules[0].NamespaceSelector = nil
190+
delete(updated.Spec.Rules[0].Quota[0].Hard, corev1.ResourceLimitsCPU)
191+
192+
return k8sClient.Update(ctx, updated)
193+
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
194+
195+
persisted := &capsulev1beta2.Tenant{}
196+
Expect(k8sClient.Get(ctx, client.ObjectKey{Name: tenantName}, persisted)).To(Succeed())
197+
Expect(persisted.Spec.Rules[0].NamespaceSelector).To(BeNil())
198+
Expect(persisted.Spec.Rules[0].Quota[0].Hard).NotTo(HaveKey(corev1.ResourceLimitsCPU))
199+
})
200+
201+
applicationSelector := &metav1.LabelSelector{MatchLabels: map[string]string{selectorKey: "application"}}
202+
Eventually(func() error {
203+
return setTenantQuota(applicationSelector, "8")
204+
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
205+
expectGeneratedQuota("application", "8", "0")
206206
})
207207

208-
It("rejects the same unsafe scope and hard-limit changes on the generated quota", func() {
208+
It("rejects a generated quota decrease while changing scope and allows removal", func() {
209209
controllerClient := impersonationClient(ControllerServiceAccountFull, nil)
210-
for _, test := range []struct {
211-
name string
212-
mutate func(corev1.ResourceList)
213-
message string
214-
}{
215-
{
216-
name: "decrease",
217-
mutate: func(hard corev1.ResourceList) {
218-
hard[corev1.ResourceLimitsCPU] = resource.MustParse("0")
219-
},
220-
message: `spec.quota.hard["limits.cpu"] cannot be reduced from 8 to 0 while namespace selectors are changing`,
221-
},
222-
{
223-
name: "removal",
224-
mutate: func(hard corev1.ResourceList) {
225-
delete(hard, corev1.ResourceLimitsCPU)
226-
},
227-
message: `spec.quota.hard["limits.cpu"] cannot be removed while namespace selectors are changing`,
228-
},
229-
} {
230-
By(test.name, func() {
231-
Eventually(func() error {
232-
current := &capsulev1beta2.GlobalResourceQuota{}
233-
if err := controllerClient.Get(ctx, quotaKey, current); err != nil {
234-
return err
235-
}
236-
237-
updated := current.DeepCopy()
238-
delete(updated.Spec.NamespaceSelectors[0].LabelSelector.MatchLabels, selectorKey)
239-
test.mutate(updated.Spec.Quota.Hard)
240-
241-
return controllerClient.Update(ctx, updated)
242-
}, defaultTimeoutInterval, defaultPollInterval).Should(MatchError(ContainSubstring(test.message)))
243-
})
244-
}
245210

246-
persisted := &capsulev1beta2.GlobalResourceQuota{}
247-
Expect(k8sClient.Get(ctx, quotaKey, persisted)).To(Succeed())
248-
Expect(persisted.Spec.NamespaceSelectors[0].LabelSelector.MatchLabels).To(HaveKeyWithValue(selectorKey, "application"))
249-
persistedLimit := persisted.Spec.Quota.Hard[corev1.ResourceLimitsCPU]
250-
Expect(persistedLimit.Cmp(resource.MustParse("8"))).To(Equal(0))
211+
By("rejecting the explicit decrease", func() {
212+
Eventually(func() error {
213+
current := &capsulev1beta2.GlobalResourceQuota{}
214+
if err := controllerClient.Get(ctx, quotaKey, current); err != nil {
215+
return err
216+
}
217+
218+
updated := current.DeepCopy()
219+
delete(updated.Spec.NamespaceSelectors[0].LabelSelector.MatchLabels, selectorKey)
220+
updated.Spec.Quota.Hard[corev1.ResourceLimitsCPU] = resource.MustParse("0")
221+
222+
return controllerClient.Update(ctx, updated)
223+
}, defaultTimeoutInterval, defaultPollInterval).Should(MatchError(ContainSubstring(
224+
`spec.quota.hard["limits.cpu"] cannot be reduced from 8 to 0 while namespace selectors are changing`,
225+
)))
226+
})
227+
228+
By("allowing the resource limit to be removed", func() {
229+
Eventually(func() error {
230+
current := &capsulev1beta2.GlobalResourceQuota{}
231+
if err := controllerClient.Get(ctx, quotaKey, current); err != nil {
232+
return err
233+
}
234+
235+
updated := current.DeepCopy()
236+
delete(updated.Spec.NamespaceSelectors[0].LabelSelector.MatchLabels, selectorKey)
237+
delete(updated.Spec.Quota.Hard, corev1.ResourceLimitsCPU)
238+
239+
return controllerClient.Update(ctx, updated)
240+
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
241+
})
242+
243+
expectGeneratedQuota("application", "8", "0")
251244
})
252245

253246
It("allows equal or increased limits with a scope change and a later same-scope decrease", func() {
@@ -304,9 +297,11 @@ var _ = Describe("rule-generated GlobalResourceQuota admission", Ordered,
304297
delete(current.Spec.Rules[0].Quota[0].Hard, corev1.ResourceLimitsCPU)
305298

306299
return k8sClient.Update(ctx, current)
307-
}, defaultTimeoutInterval, defaultPollInterval).Should(MatchError(ContainSubstring(
308-
`rules[0].quota[0].hard["limits.cpu"] cannot be removed while 300m is allocated`,
309-
)))
300+
}, defaultTimeoutInterval, defaultPollInterval).Should(Succeed())
301+
302+
persisted := &capsulev1beta2.Tenant{}
303+
Expect(k8sClient.Get(ctx, client.ObjectKey{Name: tenantName}, persisted)).To(Succeed())
304+
Expect(persisted.Spec.Rules[0].Quota[0].Hard).NotTo(HaveKey(corev1.ResourceLimitsCPU))
310305

311306
applicationSelector := &metav1.LabelSelector{MatchLabels: map[string]string{selectorKey: "application"}}
312307
Eventually(func() error {

‎internal/controllers/resources/collect.go‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ type CollectorOptions struct {
4545
AllowCrossNamespaceSelection bool
4646
Accumulator processor.Accumulator
4747
Iterator CollectorIteratorOptions
48+
ReplicationContext map[string]any
4849
ValidatorNamespaces tpl.NamespaceValidator
50+
preserveOwnerReferences bool
4951
}
5052

5153
type CollectorIteratorOptions struct {
@@ -130,6 +132,10 @@ func (co *Collector) Collect(
130132
}
131133
}
132134

135+
if opts.ReplicationContext != nil {
136+
tplContext[replicationContextKey] = opts.ReplicationContext
137+
}
138+
133139
if tnt != nil {
134140
tCtx, err := tenant.NewTenantContext(tnt, c.Scheme(), co.contextSanitizeOptions)
135141
if err != nil {
@@ -155,6 +161,9 @@ func (co *Collector) Collect(
155161

156162
log.V(7).Info("available context", "context", tplContext)
157163

164+
authoredOpts := opts
165+
authoredOpts.preserveOwnerReferences = true
166+
158167
// Run Raw Items
159168
for rawIndex, item := range spec.RawItems {
160169
log.V(5).Info("processing raw item", "index", rawIndex)
@@ -168,7 +177,7 @@ func (co *Collector) Collect(
168177

169178
log.V(7).Info("evaluated raw item", "object", p)
170179

171-
rawError = co.AddToAccumulation(tnt, ns, opts, spec, p, resourceIndex+"/raw-"+strconv.Itoa(rawIndex), true)
180+
rawError = co.AddToAccumulation(tnt, ns, authoredOpts, spec, p, resourceIndex+"/raw-"+strconv.Itoa(rawIndex), true)
172181
if rawError != nil {
173182
syncErr = errors.Join(syncErr, rawError)
174183

@@ -190,7 +199,7 @@ func (co *Collector) Collect(
190199
log.V(5).Info("loaded resources", "amount", len(p))
191200

192201
for i, o := range p {
193-
genError = co.AddToAccumulation(tnt, ns, opts, spec, o, resourceIndex+"/generator-"+strconv.Itoa(generatorIndex)+"-"+strconv.Itoa(i), true)
202+
genError = co.AddToAccumulation(tnt, ns, authoredOpts, spec, o, resourceIndex+"/generator-"+strconv.Itoa(generatorIndex)+"-"+strconv.Itoa(i), true)
194203
if genError != nil {
195204
syncErr = errors.Join(syncErr, genError)
196205

@@ -261,7 +270,12 @@ func (co *Collector) AddToAccumulation(
261270
obj.SetAnnotations(dst)
262271
}
263272

264-
sanitize.SanitizeUnstructured(obj, co.objectSanitizeOptions)
273+
sanitizeOptions := co.objectSanitizeOptions
274+
if opts.preserveOwnerReferences {
275+
sanitizeOptions.StripOwnerreferences = false
276+
}
277+
278+
sanitize.SanitizeUnstructured(obj, sanitizeOptions)
265279

266280
processor.AccumulatorAdd(opts.Accumulator, resource, processor.AccumulatorObject{
267281
Object: obj,

0 commit comments

Comments
 (0)