-
Notifications
You must be signed in to change notification settings - Fork 11
🐛 Start informers for objects that they haven't been started for yet #9
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
Conversation
On-behalf-of: SAP <[email protected]> Signed-off-by: Marvin Beckers <[email protected]>
examples/apiexport/main.go
Outdated
| return reconcile.Result{}, err | ||
| } | ||
|
|
||
| log.Info("Found Secret objects in Workspace of ConfigMap", "count", len(secrets.Items)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this here in the example. We need e2e tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I can remove it from the example, I just needed a way to validate it. With the sdk changes to include a testing framework, we can write e2e tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sttts removed the example code changes.
On-behalf-of: SAP <[email protected]> Signed-off-by: Marvin Beckers <[email protected]>
|
From now on, after this PR, we need tests. |
|
LGTM label has been added. Git tree hash: 5d934ec788e4e16ec9261b0a747f1482b7002d0c
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts 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 |
Summary
As per the example in #7, we were unable to
ListorGetobjects that are not watched by the controller. This change attempts to emulate what upstream controller-runtime's cache implementation does: When an informer has not been started yet, we should try to start one.The way we start a new informer is a bit hacky due to our custom
NewInformerlogic, but this should do the trick.Related issue(s)
Fixes #8
Release Notes