Skip to content
This repository has been archived by the owner on May 22, 2020. It is now read-only.

What is the proper scoping of our API types? #463

Closed
krousey opened this issue Dec 21, 2017 · 9 comments
Closed

What is the proper scoping of our API types? #463

krousey opened this issue Dec 21, 2017 · 9 comments
Assignees

Comments

@krousey
Copy link
Contributor

krousey commented Dec 21, 2017

This was brought up in the cluster API review, and I think it was discussed in last week's meeting. I'm creating this issue so that we have a concentrated and recorded place to discuss this.

Do we want to have machines and clusters namespaced or not?

Currently we have them cluster scoped because the prototype was designed for the types to be stored in (or aggregated with) the cluster they represented. The other use case is to store the types in another cluster for remote management. If we leave the types cluster scoped, we can only manage one cluster remotely. This is not a desirable limitation.

So the first major question: Do we want to support both local and remote management? I think the answer to this is obviously yes, but if not, we should go with the scoping that best solved the most desired use-case.

If we want to support both, I think we have a few of options. Feel free to propose others that are drastically different from what I present below.

Option 1 (Do it both ways!)

We support both namespace and cluster-scoped types in the API aggregation server. I imagine we could implement it in such a way that the aggregated API server either register the types as namespaced or not based on a flag.

Pros

  • Supports both use-cases

Cons

  • Most likely can't support both use-cases simultaneously (can't locally manage a cluster that is remotely managing others).
  • Would require 2 different clients to be generated (namespaced and not namespaced).
  • Controllers would also have to be coded to work with both scopes.
  • Actually anything that uses it would have to be coded to work with both scopes.

Option 2 (Just namespaces)

We put every cluster in a namespace including local ones. Each namespace would have it's own Cluster object and several Machine objects.

Pros

  • Supports both use-cases
  • There's no difference between managing locally and remotely

Cons

  • There's no difference between managing locally and remotely. Local management likely has special considerations so you don't pull the rug out from under yourself. Being able to distinguish the scenarios might be useful at the machine-controller level. Higher-level algorithms shouldn't care.
  • Not as clean for local cluster management.

Option 3 (Just namespaces, with special ones)

Just like option 2, we make all the types namespace-scoped. The only difference is we reserve (by convention or somehow codify it) a special namespace for local cluster management. I propose one of the following:

  • default: kubectl get <any cluster type> would return the ones representing the local cluster by default.
  • kube-system: Puts it in the namespace that most other cluster level components are running in.
  • local: Just a special keyword that signifies that this cluster is local.

Pros

  • Supports both use-cases
  • There's no difference in representation and can be operated on the same way for either use-case
  • Contains enough signal that management tools can do something different for local or remote management if they need to.

Cons

  • Relies on convention that may be unenforceable.
@krousey
Copy link
Contributor Author

krousey commented Dec 21, 2017

Option 3 with kube-system namespace is my personal preference.

@roberthbailey
Copy link
Contributor

I like option 3 with the default namespace, so that a query that doesn't include a namespace gives you information about the current cluster.

e.g. kubectl get machines tells you the machines that represent this cluster. If we go with the kube-system namespace, then the query for "show me my machines" becomes the more cumbersome kubectl get machines -n kube-system.

Also, if we go with kube-system but allow machines to be namespaced, what happens when you add machines to the default namespace? It would be awkward if listing machines in the default namespace is actually describing machines for a remote cluster.

@staebler
Copy link

Option 4 (Namespaces, with cluster field)
Just like Option 3, except that the differentiator between local and remote cluster management is done via a field on the cluster object.

Pros

  • All the same pros as Option 3.
  • Is more clear to the end user what the behavior is going to be. With Option 3, the end user needs to know about a special namespace, where the cluster object behaves differently. The user that does not have this knowledge may accidentally put a cluster that is meant to be remote in the special namespace. Or the user may be confused when they move their local-management cluster object to a different namespace and get very different results.

Cons

  • May be difficult to enforce that there is only a single cluster with the field set for local management. We could overcome this by somewhat combining Option 3 and 4 together. We could add validation that requires the cluster object to be in the special namespace if and only if the field is set for local management. This would also have an added benefit of making it easier to search for the local-management cluster.

@dgoodwin
Copy link

+1 for the 4, possibly with validation to ensure that one and only one cluster with local=true goes into "default". (and nowhere else)

Adjusting for any local behavior based on a cluster boolean does feel like a slight improvement vs comparing the namespace to a constant.

@krousey
Copy link
Contributor Author

krousey commented Jan 22, 2018

Those are actually good suggestions. How about both? Locally manager clusters have their types stored in the default namespace, and they have a "LocallyManaged" status field that is set to true by deployment tool. Remote clusters live in other namespaces and have a "LocallyManaged" status field set to false. Seem good?

@staebler
Copy link

@krousey I'm not sure that having the field in the status buys us anything. We would still be dependent upon the user putting the object in the correct namespace for their desired behavior, without any field that the user has to set to declare their intention. A field in the status would be nothing more than an after-the-fact indicator to the user about what happened.

@mvladev
Copy link
Contributor

mvladev commented Jan 22, 2018

@krousey I would avoid setting any fields for that option. More suitable approach is to use annotation.

I would choose option 2, because it's the most flexible one - you can name your namespaces however you want, put your machines in them and there is no magic or requirements. It's boring and simple.

@krousey
Copy link
Contributor Author

krousey commented Jan 22, 2018

@staebler I envisioned the deployment tool would know if it's deploying for remote management or local management, and it could properly set the status. But then again, the whole point of this is to put all inputs in the spec. I want to punt on the local versus non-local right now. It can be done with annotations if needed at first.

So we will go with option 2. Namespaces.

@krousey
Copy link
Contributor Author

krousey commented Jan 25, 2018

Closing this issue. The new types were created with namespace scoping.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants