Skip to content

chore: Get resource from cache instead of APIServer #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ mocks: install-mockery ## Build mocks
@echo -n "building mocks for sigs.k8s.io/controller-runtime/pkg/client ... "
@bin/mockery --quiet --name="(Object|Client|Status|Reader|SubResourceWriter)" --case=underscore --output=mocks/controller-runtime/pkg/client --dir="$(CONTROLLER_RUNTIME_DIR)/pkg/client"
@echo "ok."
@echo -n "building mocks for sigs.k8s.io/controller-runtime/pkg/cache ... "
@bin/mockery --quiet --name="(Cache)" --case=underscore --output=mocks/controller-runtime/pkg/cache --dir="$(CONTROLLER_RUNTIME_DIR)/pkg/cache"
@echo "ok."

help: ## Show this help.
@grep -F -h "##" $(MAKEFILE_LIST) | grep -F -v grep | sed -e 's/\\$$//' \
Expand Down
2 changes: 1 addition & 1 deletion apis/core/v1alpha1/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const (
// ACK service controller.
AnnotationReadOnly = AnnotationPrefix + "read-only"
// AnnotationAdoptionPolicy is an annotation whose value is the identifier for whether
// we will attempt adoption only (value = adopt-only) or attempt a create if resource
// we will attempt adoption only (value = adopt-only) or attempt a create if resource
// is not found (value adopt-or-create).
//
// NOTE (michaelhtm): Currently create-or-adopt is not supported
Expand Down
231 changes: 231 additions & 0 deletions mocks/controller-runtime/pkg/cache/cache.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 47 additions & 0 deletions mocks/controller-runtime/pkg/cache/new_cache_func.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/condition/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ var (
NotManagedReason = "This resource already exists but is not managed by ACK. " +
"To bring the resource under ACK management, you should explicitly adopt " +
"the resource by enabling the ResourceAdoption feature gate and populating " +
"the `services.k8s.aws/adoption-policy` and `services.k8s.aws/adoption-fields` " +
"the `services.k8s.aws/adoption-policy` and `services.k8s.aws/adoption-fields` " +
"annotations."
UnknownSyncedMessage = "Unable to determine if desired resource state matches latest observed state"
NotSyncedMessage = "Resource not synced"
Expand Down
25 changes: 13 additions & 12 deletions pkg/runtime/adoption_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
ctrlrt "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
k8sctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/predicate"
Expand Down Expand Up @@ -57,7 +58,7 @@ type adoptionReconciler struct {
// of an upstream controller-runtime.Manager
func (r *adoptionReconciler) BindControllerManager(mgr ctrlrt.Manager) error {
r.kc = mgr.GetClient()
r.apiReader = mgr.GetAPIReader()
r.ackResourceCache = mgr.GetCache()
return ctrlrt.NewControllerManagedBy(
mgr,
).For(
Expand Down Expand Up @@ -251,7 +252,7 @@ func (r *adoptionReconciler) Sync(

// Only create the described resource if it does not already exist
// in k8s cluster.
if err := r.apiReader.Get(ctx, types.NamespacedName{
if err := r.ackResourceCache.Get(ctx, types.NamespacedName{
Namespace: described.MetaObject().GetNamespace(),
Name: described.MetaObject().GetName(),
}, described.RuntimeObject()); err != nil {
Expand Down Expand Up @@ -306,15 +307,15 @@ func (r *adoptionReconciler) getAdoptedResource(
req ctrlrt.Request,
) (*ackv1alpha1.AdoptedResource, error) {
ro := &ackv1alpha1.AdoptedResource{}
// Here we use k8s APIReader to read the k8s object by making the
// Here we use k8s ACKResourceCache to read the k8s object by making the
// direct call to k8s apiserver instead of using k8sClient.
// The reason is that k8sClient uses a cache and sometimes k8sClient can
// return stale copy of object.
// It is okay to make direct call to k8s apiserver because we are only
// making single read call for complete reconciler loop.
// See following issue for more details:
// https://github.com/aws-controllers-k8s/community/issues/894
if err := r.apiReader.Get(ctx, req.NamespacedName, ro); err != nil {
if err := r.ackResourceCache.Get(ctx, req.NamespacedName, ro); err != nil {
return nil, err
}
return ro, nil
Expand Down Expand Up @@ -615,17 +616,17 @@ func NewAdoptionReconcilerWithClient(
metrics *ackmetrics.Metrics,
cache ackrtcache.Caches,
kc client.Client,
apiReader client.Reader,
ackResourceCache cache.Cache,
) acktypes.AdoptedResourceReconciler {
return &adoptionReconciler{
reconciler: reconciler{
sc: sc,
log: log.WithName("adopted-reconciler"),
cfg: cfg,
metrics: metrics,
cache: cache,
kc: kc,
apiReader: apiReader,
sc: sc,
log: log.WithName("adopted-reconciler"),
cfg: cfg,
metrics: metrics,
cache: cache,
kc: kc,
ackResourceCache: ackResourceCache,
},
}
}
Loading