To add Informer for ServiceAccount #11330
Conversation
|
[test] |
|
LGTM @soltysh PTAL |
|
should this go upstream in the SharedInformerFactory instead? |
Yes, but since we're running an entire release behind, I'd be ok having a very similar interface downstream until we get there. |
| return nil, err | ||
| } | ||
| if !exists { | ||
| return nil, kapierrors.NewNotFound(kapi.Resource("ServiceAccount"), name) |
There was a problem hiding this comment.
Resource names are usually lowercased.
| @@ -175,6 +176,10 @@ func (f *sharedInformerFactory) KubernetesInformers() informers.SharedInformerFa | |||
| return kubernetesSharedInformer{f} | |||
| } | |||
|
|
|||
| func (f *sharedInformerFactory) ServiceAccounts() ServiceAccountInformer { | |||
There was a problem hiding this comment.
Add here todo, with a link to you upstream PR, so that we know we should use the upstream version upon next rebase (to 1.5).
| // If the indexed list fails then we will fallback to listing from all namespaces and filter | ||
| // by the namespace we want. | ||
| func (s storeServiceAccountsNamespacer) List(selector labels.Selector) ([]*kapi.ServiceAccount, error) { | ||
| configs := []*kapi.ServiceAccount{} |
There was a problem hiding this comment.
s/configs/serviceAccounts/
|
Thanks, going to modify accordingly. |
sdminonne
force-pushed
the
informer_sa
branch
from
4dcd791
to
443bdad
Compare
|
PTAL |
|
LGTM, squash and we can merge it |
sdminonne
force-pushed
the
informer_sa
branch
from
443bdad
to
aa9efad
Compare
|
@Kargakis sqashed |
|
[test] |
|
Looking at #11159 it seems that you also need to plug the cache in the pspReview* rest |
|
@Kargakis sure. But I'd like to keep this PR as is and later on plug the cache where and if needed. I've already tested this PR plugging the code. If you prefer to put all together I can do anyway. |
|
@sdminonne I am fine with merging this as is, but let's keep #11159 open until all the required work is done. LGTM [merge] |
|
@Kargakis thanks |
Automatic merge from submit-queue To add service account informer @deads2k this PR adds ServiceAccount informer as discussed [here](openshift/origin#11330 (comment))
|
@Kargakis what should I do for this? I tried to launch the same script and locally it's not failing (tested both golang 1.7 and 1.6). |
|
continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 12 |
|
#11094 [test] |
|
#11240 [test] |
|
Evaluated for origin test up to aa9efad |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10246/) (Base Commit: ab7d5fe) |
|
Flake #11240 [merge] |
|
Flake #11004 [merge] |
|
Evaluated for origin merge up to aa9efad |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10346/) (Base Commit: 1ec25ef) (Image: devenv-rhel7_5211) |
To fix #11159