Skip to content
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

Fix Service Account Namespace #8

Merged
merged 1 commit into from
May 22, 2020
Merged

Conversation

weiwu-sre
Copy link
Contributor

No description provided.

@joe-elliott
Copy link
Member

I was purposefully trying not to bind this to a specific namespace, but now that I'm reviewing the yaml I see that the cluster role binding requires a namespace for the service account binding.

Should we just explicitly put everything in default?

@weiwu-sre
Copy link
Contributor Author

I was purposefully trying not to bind this to a specific namespace, but now that I'm reviewing the yaml I see that the cluster role binding requires a namespace for the service account binding.

Should we just explicitly put everything in default?

Deployment and service account need to be on same namespace. Also clusterrolebinding requires a namespace.

Either default or kube-system should be fine in my opinion.

@joe-elliott
Copy link
Member

I like default. Do you mind adding namespace to all objects? If not I can get it done.

@weiwu-sre
Copy link
Contributor Author

I like default. Do you mind adding namespace to all objects? If not I can get it done.

I think this is the only place missing namespace. clusterrole and clusterrolebinding are cluster level resources, don't require namespace.

@joe-elliott
Copy link
Member

Oh yeah, you're right. For some reason I thought deployment and other resources were lacking namespace too.

Merging. Thanks for the contribution!

@joe-elliott joe-elliott merged commit 4e47b9f into grafana:master May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants