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

WIP: Remove metallb-system namespace #194

Closed
wants to merge 1 commit into from

Conversation

simt2
Copy link

@simt2 simt2 commented Mar 14, 2018

What this PR does / why we need it:
This is a first draft of how to remove the hardcoded metallb-system namespace and make metallb namespace-agnostic.

  • Adds a function InClusterNamespace to internal/k8s/ to detect the namespace the pod is currently running in
  • Uses above function to determine the namespace for config and leader election.
  • Removes the --config-ns parameter.
  • Updates the manifests in manifests/ to use the default namespace. This is just as POC we might just keep the metallb-namespace in there - it does not matter with the changes anymore. I did not update the helm chart because it will probably be maintained in kubernetes/charts in the near future.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #193

Special notes for your reviewer:
Please elaborate on why the --config-ns parameter is necessary. Should the metallb controller be able to run outside of cluster? I can not think of any other scenarios. If this is not the case, it should probably be removed and default to namespace metallb is running in.


// InClusterNamespace retrieves the namespace metallb is running in, when
// running in-cluster.
func InClusterNamespace() (string, error) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is based on kubernetes/client-go. I have briefly investigated if we should build the k8s client in a way that gives access to this function but did not want to alter too much code in internal/k8s/. I believe that this is currently the better solution.

@danderson
Copy link
Contributor

Submitted something similar, with some tweaks to code layout and (not) updating the static manifests. Thank you for the PR though! You showed me the correct way of getting at the "current" namespace.

@danderson danderson closed this Mar 17, 2018
@simt2
Copy link
Author

simt2 commented Mar 17, 2018

I don't see what keeping the custom namespace in the development setup achieves except that you need to specify it when debugging via kubectl, but I guess it's your project.

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.

Remove hardcoded namespace - make metallb namespace-agnostic
2 participants