⚠️ DynamicRESTMapper that reloads on REST cache miss #554
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@estroz: GitHub didn't allow me to request PR reviews from the following users: joelanford, hasbro17. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
k8s-ci-robot
added
cncf-cla: yes
estroz
force-pushed
the
dynamic-restmapper
branch
from
eec9aa8
to
0cc0bc9
Compare
|
Awesome, doing a review now, but from the description this seems to cover most of what I want. |
DirectXMan12
suggested changes
Aug 8, 2019
joelanford
reviewed
Aug 8, 2019
estroz
force-pushed
the
dynamic-restmapper
branch
from
081b8c6
to
99a4ddb
Compare
DirectXMan12
suggested changes
Aug 8, 2019
estroz
commented
Aug 8, 2019
mengqiy
reviewed
Aug 9, 2019
estroz
force-pushed
the
dynamic-restmapper
branch
2 times, most recently
from
1c617f5
to
6d7d994
Compare
DirectXMan12
suggested changes
Aug 13, 2019
estroz
force-pushed
the
dynamic-restmapper
branch
from
c202f9d
to
1aecda6
Compare
justinsb
reviewed
Aug 14, 2019
estroz
commented
Aug 14, 2019
DirectXMan12
suggested changes
Aug 15, 2019
estroz
force-pushed
the
dynamic-restmapper
branch
from
959d90d
to
d72fc0d
Compare
ironcladlou
reviewed
Aug 22, 2019
estroz
force-pushed
the
dynamic-restmapper
branch
from
d72fc0d
to
7f767e3
Compare
DirectXMan12
suggested changes
Sep 3, 2019
estroz
force-pushed
the
dynamic-restmapper
branch
from
7f767e3
to
becbbaf
Compare
6 tasks
This was referenced Mar 5, 2020
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 6, 2020
Reason for upgrade: The new version uses [DynamicRESTMapper](https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/client/apiutil/dynamicrestmapper.go) as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: - Upgrade go version in `go.mod` and `Dockerfile` to 1.13. The [errors.As](https://golang.org/pkg/errors/#As) in [dynamicrestmapper.go](https://github.com/kubernetes-sigs/controller-runtime/blob/bfc982769615817ee15db8a543662652d900d27b/pkg/client/apiutil/dynamicrestmapper.go#L48) requires go 1.13. - A higher version for k8s.io/cli-runtime and k8s.io/client-go are required after upgrading controller-runtime to v0.5.0 - Version changes of other packages in `go.mod` are updated automatically. - serializer.DirectCodecFactory was renamed to [serializer.WithoutConversionCodecFactory](https://godoc.org/k8s.io/apimachinery/pkg/runtime/serializer#WithoutConversionCodecFactory) after k8s.io/apimachinery 1.16 (see [here](kubernetes/apimachinery@ed8af17), and [here](kubernetes/apimachinery@4fac835)) - Default [LeaderElectionID](https://github.com/kubernetes-sigs/controller-runtime/blob/bfc982769615817ee15db8a543662652d900d27b/pkg/leaderelection/leader_election.go#L46) in controller-runtime manager is removed after kubernetes-sigs/controller-runtime#446. - [NewlineReporter](https://godoc.org/sigs.k8s.io/controller-runtime/pkg/envtest/printer) is moved from `sigs.k8s.io/controller-runtime/pkg/envtest/` to `https://godoc.org/sigs.k8s.io/controller-runtime/pkg/envtest/printer` by [this](kubernetes-sigs/controller-runtime@748f55d#diff-42de1d59fbe8f8b90154f01dd01f5748) commit. - In controller-runtime v0.2.2, if a resource does not exist, a controller cannot be created successfully. After update controller-runtime to v0.5.0, a controller can be created without error. However, when the `Reconcile` method is triggered, there will be an error complaining the resource does not exist. Therefore, we will explicitly check if a resource exists before creating the corresponding object reconciler in `createObjectReconciler` in `hnc_config.go` (see details in kubernetes-sigs/controller-runtime#840) Tested: - Unit tests. - Went through [demo script](https://docs.google.com/document/d/1tKQgtMSf0wfT3NOGQx9ExUQ-B8UkkdVZB6m4o3Zqn64/edit#) to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 6, 2020
Reason for upgrade: The new version uses [DynamicRESTMapper](https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/client/apiutil/dynamicrestmapper.go) as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through [demo script](https://docs.google.com/document/d/1tKQgtMSf0wfT3NOGQx9ExUQ-B8UkkdVZB6m4o3Zqn64/edit#) to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 6, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 6, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 6, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 6, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 7, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 7, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 7, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 7, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 9, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 9, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 9, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 9, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 9, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 9, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 9, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. - Manually test if the PR solves the restart problem as described in kubernetes-retired#488 with following workflow: - Install HNC - Install a new CRD - Config the new type in `config` singleton Before this PR, corresponding object reconciler for the new type will not be created unless we restart HNC. After the change, corresponding object reconciler can be created and it reconciles objects of the new type as expected without restarting HNC. This partly solve: kubernetes-retired#488
sophieliu15
added a commit
to sophieliu15/multi-tenancy
that referenced
this pull request
Mar 9, 2020
Reason for upgrade: The new version uses DynamicRESTMapper as default `RESTMapper` for controller runtime manager. `DynamicRESTMapper` will "reload the delegated `meta.RESTMapper` on a cache miss" (see kubernetes-sigs/controller-runtime#554), which can solve problem that we need to restart HNC after adding a new CRD to create corresponding object reconciler when using controller-runtime v0.2.2 (see details in kubernetes-retired#488). Incompatibility issues addressed in this PR: see PR description Tested: - Unit tests. - Went through demo script to make sure HNC behaves as expected on a GKE cluster. This partly solve: kubernetes-retired#488
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pkg/client/apiutil:
DynamicRESTMapperwill reload the delegatedmeta.RESTMapperon a cache miss. API calls are rate-limited by a token bucket, and will return anErrRateLimitedcontaining atime.Durationsignifying a wait time.See operator-framework/operator-sdk#1329 for a discussion of why reloading on a cache miss is desirable.
Discussion points:
Closes #321
/cc @DirectXMan12 @mengqiy @joelanford @hasbro17