Allow users to specify namespace for Kubernetes informers #851
Conversation
|
Hi @ywskycn. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
1 similar comment
|
Hi @ywskycn. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
| @@ -83,4 +91,5 @@ func main() { | |||
| func init() { | |||
| flag.StringVar(&kubeconfig, "kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.") | |||
| flag.StringVar(&masterURL, "master", "", "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.") | |||
| flag.StringVar(&informerNamespace, "informer-namespace", "", "The namespace name used for Kubernetes informers to obtain the listers.") | |||
There was a problem hiding this comment.
Maybe just namespace is enough? (Same for the other instances)
| kubeconfig string | ||
| masterURL string | ||
| kubeconfig string | ||
| informerNamespace string |
There was a problem hiding this comment.
Could you please also do it for the viewer CRD so that folks that implement a new CRD have a consistent pattern to follow?
There was a problem hiding this comment.
@vicaire Viewer CRD uses another approach (SimpleController) to implement. Not sure how to plugin a namespace into it.. Do you happen to know? @neuromage
|
/cc @IronPan @neuromage |
|
If it's complicated to figure out how to do it for the viewer CRD, we can do this as a separate PR. Let me approve this one. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vicaire 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vicaire 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 |
* Allow users to specify namespace for Kubernetes informers * Change variable informerNamespace to namespace
To support run within a namespace, instead of requesting ClusterRole.
This change is