⚠️ Allow configuring more granular cache filtering #2421

alvaroaleman · 2023-07-24T22:28:53Z

This change implements most of the cache options design, in particular it is now possible to:

Configure Namespaces per type, fixes Cache: Support multiple namespaces for just one type #2254
Configure filtering per namespace

What is still missing is to allow configuring namespace-level default settings that will be used for all namespaces not explicitly configured. There is nothing in the way of doing that as a follow-up.

The implementation slightly derivates from the design document in that the filter settings in ByGVK do not use the Config struct but instead are directly embedded. This allows us to not break compatibility and is more ergonomic to use.

The implementation is based on a new and internal per-type delegating "meta cache".

k8s-ci-robot · 2023-07-24T22:28:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [alvaroaleman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sbueringer

First quick round of review

pkg/cache/internal/informers.go

pkg/cache/cache.go

sbueringer

Very nice, just nits from my side

pkg/cache/cache.go

pkg/cache/delegate_by_type.go

sbueringer · 2023-07-26T18:58:21Z

pkg/cache/delegate_by_type.go

+	synced := true
+	for _, cache := range append(maps.Values(dbt.caches), dbt.defaultCache) {
+		if !cache.WaitForCacheSync(ctx) {
+			synced = false


Suggested change

synced = false

return false

Q: Would this make sense here?

If yes, we have a few more places where we could do the same (but maybe better in a separate PR)

The reason it is the way that it is is that the interface is documented as "Blocks until caches are synced". If we return early, not all caches might be synced. I don't know if anyone would continue their binary if this returns false but odds are, someone somewhere will

Yeah let's better not change it. Was just thinking if we're hitting the case where some caches are definitely not synced we might as well return earlier. But no real reason to make a change

pkg/cache/delegate_by_type.go

pkg/cache/multi_namespace_cache.go

pkg/cache/defaulting_test.go

This change implements most of the [cache options design][0], in particular it is now possible to: * Configure Namespaces per type * Configure filtering per namespace What is still missing is to allow configuring namespace-level default settings that will be used for all namespaces not explicitly configured. There is nothing in the way of doing that as a follow-up. The implementation slightly derivates from the design document in that the filter settings in `ByGVK` do not use the `Config` struct but instead are directly embedded. This allows us to not break compatibility and is more ergonomic to use. The implementation is based on a new and internal per-type delegating "meta cache". [0]: https://github.com/kubernetes-sigs/controller-runtime/blob/main/designs/cache_options.md

k8s-ci-robot · 2023-07-26T21:08:58Z

@alvaroaleman: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-controller-runtime-apidiff	`3e35cab`	link	false	`/test pull-controller-runtime-apidiff`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

sbueringer · 2023-07-28T16:12:24Z

Thx!

/lgtm

- Bump Golang to 1.22: - `CGO_ENABLED=1` is required for `go test -race` (golang/go#51235). - Bump k8s.io/* modules to 0.30.3 and OpenShift API to the latest. - Bump Controller runtime bumped to 0.18.5: - Controller's `Watch` function now has a single generic source parameter (kubernetes-sigs/controller-runtime#2783). - Manager's `CertDir` option removed, now using the dedicated webhook server option (kubernetes-sigs/controller-runtime#2422). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421).

- Bump Golang to 1.22: - Update go.mod - Update Dockerfiles - Add `CGO_ENABLED=1` to `go test -race` (golang/go#51235). - Bump k8s.io/* modules to 0.30.3 and OpenShift API to the latest. - Bump Controller runtime bumped to 0.18.5: - Controller's `Watch` function now has a single generic source parameter (kubernetes-sigs/controller-runtime#2783). - Manager's `CertDir` option removed, now using the dedicated webhook server option (kubernetes-sigs/controller-runtime#2422). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421).

- Bump Golang to 1.22: - Update go.mod - Update Dockerfiles - Add `CGO_ENABLED=1` to `go test -race` (golang/go#51235). - Bump k8s.io/* modules to 0.30.3 and OpenShift API to the latest. - Bump Controller runtime bumped to 0.18.5: - Controller's `Watch` function now has a single generic source parameter (kubernetes-sigs/controller-runtime#2783). - Manager's `CertDir` option removed, now using the dedicated webhook server option (kubernetes-sigs/controller-runtime#2422). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Regenerate CRD and bundle manifests: - ExternalDNS API uses `metav1.LabelSelector` for the label filtering. It was updated with `+listType=atomic` marker which resulted in the addition of `x-kubernetes-list-type: atomic` to CRD.

- Bump Golang to 1.22: - Update go.mod - Update Dockerfiles - Add `CGO_ENABLED=1` to `go test -race` (golang/go#51235). - Bump k8s.io/* modules to 0.30.3 and OpenShift API to the latest. - Bump Controller runtime bumped to 0.18.5: - Controller's `Watch` function now has a single generic source parameter (kubernetes-sigs/controller-runtime#2783). - Manager's `CertDir` option removed, now using the dedicated webhook server option (kubernetes-sigs/controller-runtime#2422). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Regenerate CRD and bundle manifests: - ExternalDNS API uses `metav1.LabelSelector` for the label filtering. It was updated with `+listType=atomic` marker which resulted in the addition of `x-kubernetes-list-type: atomic` to CRD. - Bump `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package.

- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package.

- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633).

- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633).

- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633). - Migrated infrastructure CRD retrieval to the new package `zz_generated.crd-manifests`: - Updated infrastructure CRD for unit tests using `make update-vendored-crds` command.

- Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633). - Migrated infrastructure CRD retrieval to the new package `zz_generated.crd-manifests`: - Updated infrastructure CRD for unit tests using `make update-vendored-crds` command. - Updated envtest setup to use the downstream index. - Infrastructure CRD uses CEL functions backported from newer k8s API. - Upstream `envtest` is not ready to use newer CEL function. - Bumped `setup-envtest` to be able to use `--index` flag.

#143) - Bumped golang to 1.22: - Update go.mod - Update Dockerfiles - Bumped k8s.io/* modules to 0.30.3 and OpenShift API to 20240812094746-86145edb40cf. - Bumped controller-runtime to 0.18.5: - Manager's `Port` option removed, now using dedicated webhook server field (kubernetes-sigs/controller-runtime#2422). - Manager's `MetricsBindAddress` option removed, now using dedicated metrics server field (kubernetes-sigs/controller-runtime#2407). - Cache's `Namespaces` option replaced by `DefaultNamespaces` (kubernetes-sigs/controller-runtime#2421). - Bumped aws-load-balancer-controller to f39ae43121c3 to use latest CRD scheme in e2e tests. - Regenerated CRD and bundle manifests using `make bundle` command. - Bumped `kustomize` to v5 to fix a conflict caused by k8s.io bumps: - `kyaml` unable to use the bumped `github.com/google/gnostic-models/openapiv2` package. - Removed `TypeMeta` from expected deployment object when it's compared to structured one retrieved from fake client (kubernetes-sigs/controller-runtime#2633). - Migrated infrastructure CRD retrieval to the new package `zz_generated.crd-manifests`: - Updated infrastructure CRD for unit tests using `make update-vendored-crds` command. - Updated envtest setup to use the downstream index. - Infrastructure CRD uses CEL functions backported from newer k8s API. - Upstream `envtest` is not ready to use newer CEL function. - Bumped `setup-envtest` to be able to use `--index` flag.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 24, 2023

k8s-ci-robot requested review from inteon and vincepri July 24, 2023 22:28

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 24, 2023

alvaroaleman removed the request for review from inteon July 24, 2023 22:29

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 24, 2023

alvaroaleman force-pushed the cache branch from fbc6d28 to 8216805 Compare July 24, 2023 22:31

alvaroaleman changed the title ~~✨ Allow configuring more granular cache filtering~~ ⚠️ : Allow configuring more granular cache filtering Jul 24, 2023

alvaroaleman changed the title ~~⚠️ : Allow configuring more granular cache filtering~~ ⚠️ Allow configuring more granular cache filtering Jul 24, 2023

sbueringer reviewed Jul 25, 2023

View reviewed changes

alvaroaleman force-pushed the cache branch 6 times, most recently from 31e52e1 to 586bca7 Compare July 26, 2023 15:14

sbueringer reviewed Jul 26, 2023

View reviewed changes

alvaroaleman force-pushed the cache branch from 586bca7 to 0155dd4 Compare July 26, 2023 21:03

alvaroaleman force-pushed the cache branch from 0155dd4 to 3e35cab Compare July 26, 2023 21:07

k8s-ci-robot assigned sbueringer Jul 28, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 28, 2023

k8s-ci-robot merged commit 7f0c6dc into kubernetes-sigs:main Jul 28, 2023

alvaroaleman deleted the cache branch July 28, 2023 16:26

tiraboschi mentioned this pull request Aug 25, 2023

Cache doesn't start if it's config refers to types that are not installed #2456

Closed

irbekrm mentioned this pull request Sep 1, 2023

cmd/k8s-operator/manifests,cmd/k8s-nameserver: add a simple nameserver, makes operator populate its config tailscale/tailscale#9201

Closed

acumino mentioned this pull request Sep 12, 2023

Upgrade k8s.io/* to v0.28, sigs.k8s.io/controller-runtime to v0.16 gardener/gardener#8382

Closed

2 tasks

MarcosDY mentioned this pull request Oct 20, 2023

Upgrade controller-runtime to v0.16.3 spiffe/spire-controller-manager#235

Merged

guicassolato mentioned this pull request Nov 8, 2023

refactor: controller-runtime v0.16.3 Kuadrant/authorino#444

Merged

malayparida2000 mentioned this pull request Nov 16, 2023

Rook api & image update to latest master red-hat-storage/ocs-operator#2224

Merged

astefanutti mentioned this pull request Nov 22, 2023

Upgrade Kubernetes dependencies to v0.28.3 and Golang to 1.20 ray-project/kuberay#1648

Merged

4 tasks

GuptaManan100 mentioned this pull request Dec 28, 2023

Upgrade Kubernetes and controller-runtime dependencies planetscale/vitess-operator#516

Merged

RainbowMango mentioned this pull request Dec 29, 2023

Bump controller-runtime to v0.16.3 karmada-io/karmada#4486

Merged

carlkyrillos mentioned this pull request Mar 18, 2024

THREESCALE-10842 Preflights checks 3scale/3scale-operator#948

Merged

robbie-demuth mentioned this pull request Apr 30, 2024

DefaultLabelSelector overrides ByObject.Label in DefaultNamespaces #2804

Closed

regadas mentioned this pull request May 3, 2024

Update controller-runtime to v0.17.3 spotify/flink-on-k8s-operator#797

Merged

alebedev87 mentioned this pull request Aug 13, 2024

NE-1802: Bump Golang, k8s.io, OpenShift API, and controller-runtime openshift/external-dns-operator#225

Merged

alebedev87 mentioned this pull request Dec 11, 2024

NE-1808: Bump k8s.io, openshift, controller-runtime modules and golang openshift/aws-load-balancer-operator#143

Merged

This was referenced Jan 17, 2025

Upgrade k8s depencencies for Openshift 4.16 codeready-toolchain/member-operator#615

Open

Upgrade k8s depencencies in host for Openshift 4.16 codeready-toolchain/host-operator#1125

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚠️ Allow configuring more granular cache filtering #2421

⚠️ Allow configuring more granular cache filtering #2421

alvaroaleman commented Jul 24, 2023

k8s-ci-robot commented Jul 24, 2023

sbueringer left a comment

sbueringer left a comment

sbueringer Jul 26, 2023

alvaroaleman Jul 26, 2023

sbueringer Jul 28, 2023 •

edited

Loading

k8s-ci-robot commented Jul 26, 2023 •

edited

Loading

sbueringer commented Jul 28, 2023 •

edited

Loading

⚠️ Allow configuring more granular cache filtering #2421

⚠️ Allow configuring more granular cache filtering #2421

Conversation

alvaroaleman commented Jul 24, 2023

k8s-ci-robot commented Jul 24, 2023

sbueringer left a comment

Choose a reason for hiding this comment

sbueringer left a comment

Choose a reason for hiding this comment

sbueringer Jul 26, 2023

Choose a reason for hiding this comment

alvaroaleman Jul 26, 2023

Choose a reason for hiding this comment

sbueringer Jul 28, 2023 • edited Loading

Choose a reason for hiding this comment

k8s-ci-robot commented Jul 26, 2023 • edited Loading

sbueringer commented Jul 28, 2023 • edited Loading

sbueringer Jul 28, 2023 •

edited

Loading

k8s-ci-robot commented Jul 26, 2023 •

edited

Loading

sbueringer commented Jul 28, 2023 •

edited

Loading