You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "bzp2010 (via GitHub)" <gi...@apache.org> on 2026/09/17 10:07:41 UTC
[PR] feat(cache): track which Kubernetes resource owns each service, ssl and consumer [apisix-ingress-controller]
bzp2010 opened a new pull request, #2883:
URL: https://github.com/apache/apisix-ingress-controller/pull/2883
<!-- Please answer these questions before submitting a pull request -->
### Type of change:
- [x] New feature provided
- [x] Refactor
### What this PR does / why we need it:
Part 2/6 of #2877, first of a further split of what used to be part 2/6 (store, ~900 lines) into three smaller PRs: 2a (this one), 2b, 2c.
Adds a per-cacheKey owner map to the store, recorded from `Insert`'s labels as they're written, plus `Lookup(name, resourceType, id)` and `OwnedEntities(name, owner)` to query it back: given a service, ssl or consumer's type and id, find the Kubernetes resource that produced it, or list everything a given resource owns.
No behavior change. `Insert`, `Delete` and `GetResources` do exactly what they did before for every existing caller; this only adds bookkeeping alongside them. Nothing in the codebase calls `Lookup` or `OwnedEntities` yet.
This is groundwork for attributing a rejected resource (one the data plane refuses to apply) back to the Kubernetes object that produced it, so later PRs in this series can isolate just that resource instead of failing the whole sync.
### Pre-submission checklist:
- [x] Did you explain what problem does this PR solve? Or what new features have been added?
- [x] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document?
- [x] Is this PR backward compatible?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cache): track which Kubernetes resource owns each service, ssl and consumer [apisix-ingress-controller]
Posted by "AlinsRan (via GitHub)" <gi...@apache.org>.
AlinsRan commented on code in PR #2883:
URL: https://github.com/apache/apisix-ingress-controller/pull/2883#discussion_r4042993582
##########
internal/adc/cache/store.go:
##########
@@ -26,24 +27,80 @@ import (
adctypes "github.com/apache/apisix-ingress-controller/api/adc"
"github.com/apache/apisix-ingress-controller/internal/controller/label"
+ "github.com/apache/apisix-ingress-controller/internal/types"
)
type Store struct {
cacheMap map[string]Cache
pluginMetadataMap map[string]adctypes.PluginMetadata
+ // owners maps each service, ssl and consumer a cacheKey holds to the Kubernetes
+ // resource that produced it, recorded from Insert's labels as it is written.
+ // global_rule and plugin_metadata don't go through this yet: see Lookup.
+ owners map[string]map[entityKey]types.NamespacedNameKind
Review Comment:
memdb already maintains this exact index, so this map is a second copy of state the DB keeps for us.
`KindLabelIndexer` (`internal/adc/cache/indexer.go:41`) is keyed on `(LabelKind, LabelNamespace, LabelName)` — the owner — and `list()` (`internal/adc/cache/memdb.go:244`) switches to it whenever a `KindLabelSelector` is passed. So:
- `OwnedEntities(name, owner)` is `ListServices`/`ListSSL`/`ListConsumers(&KindLabelSelector{Kind, Namespace, Name})` — an index lookup, instead of the current full scan over `s.owners[name]` under the store mutex.
- `Lookup(name, resourceType, id)` is `Get*(id)` + `ownerFromLabels(obj.GetLabels())`, which is what `GetResourceLabel` (`store.go:263`) already does and what `internal/provider/apisix/status.go:129` already consumes.
Beyond the 8 `setOwner`/`removeOwner` call sites this would remove, the bigger cost is having two sources of truth that can already disagree. `Insert` records the owner from the **caller-supplied `Labels` argument**, while `removeOwner` only ever fires for what `ListXxx(selector)` returns — i.e. the labels **stored on the object itself**. `TestLookupFindsTheOwnerOfEveryTopLevelType` constructs exactly that split: the SSL carries no labels of its own, yet an owner is recorded for it, so `Lookup` answers `ApisixTls/ns/tls` and `GetResourceLabel` answers empty for the same entity. Production happens to agree today because `provider.go:284` passes the same `label.GenLabel(obj)` the translator stamps on the objects, but nothing enforces it. When they do diverge, the entry is also unreachable for removal and leaks for the lifetime of the process — deriving the owner from the object's own labels removes that class of bug entirely.
Keeping `Entity` and both method signatures but backing them with the existing label index would leave 2b/2c's interface untouched and leave `Store` with no extra state to maintain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cache): track which Kubernetes resource owns each service, ssl and consumer [apisix-ingress-controller]
Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 merged PR #2883:
URL: https://github.com/apache/apisix-ingress-controller/pull/2883
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(cache): track which Kubernetes resource owns each service, ssl and consumer [apisix-ingress-controller]
Posted by "bzp2010 (via GitHub)" <gi...@apache.org>.
bzp2010 commented on code in PR #2883:
URL: https://github.com/apache/apisix-ingress-controller/pull/2883#discussion_r4044304501
##########
internal/adc/cache/store.go:
##########
@@ -26,24 +27,80 @@ import (
adctypes "github.com/apache/apisix-ingress-controller/api/adc"
"github.com/apache/apisix-ingress-controller/internal/controller/label"
+ "github.com/apache/apisix-ingress-controller/internal/types"
)
type Store struct {
cacheMap map[string]Cache
pluginMetadataMap map[string]adctypes.PluginMetadata
+ // owners maps each service, ssl and consumer a cacheKey holds to the Kubernetes
+ // resource that produced it, recorded from Insert's labels as it is written.
+ // global_rule and plugin_metadata don't go through this yet: see Lookup.
+ owners map[string]map[entityKey]types.NamespacedNameKind
Review Comment:
fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org