Wait for the Namespace and Account informers to sync before controller startup
#154
Conversation
|
/lgtm |
a-hilaly
changed the title
Namespace and Account cacheNamespace and Account informers to sync before controller startup
|
/retest |
| @@ -233,6 +234,10 @@ func (c *serviceController) BindControllerManager(mgr ctrlrt.Manager, cfg ackcfg | |||
| // Run the caches. This will not block as the caches are run in | |||
| // separate goroutines. | |||
| cache.Run(clientSet) | |||
| // Wait for the caches to sync | |||
| ctx := context.TODO() | |||
There was a problem hiding this comment.
can we do 1 minute for this instead
There was a problem hiding this comment.
I think indeed this will solve the failing unit tests, but imposes risk of race condition
There was a problem hiding this comment.
maybe we can configure that specific failing unit test to not use CARM caches? it is only supposed to tests that the controller build happen correctly
There was a problem hiding this comment.
what's happening here is that: the unit tests attempt to initialize a cache, but WaitForCachesToSync requires communication with the api server, which is not running during unit tests. so... the function waits indefinitely an eventually leads to timeouts..
informers to sync before proceeding with the creation of the reconcilers. Key changes: - Publish `informer.HasSynced` to the top level structs for namespace and accout caches - Use these `informer.HasSynced` to wait for the caches to sync using `"k8s.io/client-go/tools/cache"`. - We call the wait mechanism right after the function that spins the informers a.k.a `caches.Run`. Last to note, we are using a `context.TODO()` context, as a temporary workaround until we figure out a better mechanism for context cancellation.
a-hilaly
force-pushed
the
race-condition-fix
branch
from
c2e7be4 to
43c2f75
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: a-hilaly, michaelhtm 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:
Approvers can indicate their approval by writing |
Mainly bringing the new bug fixes from runtime to the iam controller: - aws-controllers-k8s/runtime#154 - aws-controllers-k8s/runtime#153 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Add a new functionality to help waiting for the
NamespaceandAccountcacheinformers to sync before proceeding with the creation of the reconcilers.
Key changes:
informer.HasSyncedto the top level structs for namespace andaccout caches
informer.HasSyncedto wait for the caches to sync using"k8s.io/client-go/tools/cache".a.k.a
caches.Run.Last to note, we are using a
context.TODO()context, as a temporary workarounduntil we figure out a better mechanism for context cancellation.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.