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

Revisit identifiers spec #1216

Merged
merged 5 commits into from
Sep 16, 2014
Merged

Revisit identifiers spec #1216

merged 5 commits into from
Sep 16, 2014

Conversation

thockin
Copy link
Member

@thockin thockin commented Sep 8, 2014

Partially addresses #1135.

This is the result of discussions in the past few days about imprecision and confusion. Hopefully simpler. There are a few FIXME comments in the doc - discuss.

@thockin thockin mentioned this pull request Sep 8, 2014
@@ -1,14 +1,15 @@
# Identifiers and Names in Kubernetes

A summarization of the goals and recommendations for identifiers and names in Kubernetes. Described in [GitHub issue #199](https://github.com/GoogleCloudPlatform/kubernetes/issues/199).
A summarization of the goals and recommendations for IDs and names in Kubernetes. Described in [GitHub issue #199](https://github.com/GoogleCloudPlatform/kubernetes/issues/199).
Copy link
Member

Choose a reason for hiding this comment

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

s/IDs/uids/ (or UIDs?)

Copy link
Member Author

Choose a reason for hiding this comment

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

"identifiers" - generic meaning for the title.

On Mon, Sep 8, 2014 at 3:00 PM, bgrant0607 notifications@github.com wrote:

In docs/identifiers.md:

@@ -1,14 +1,15 @@

Identifiers and Names in Kubernetes

-A summarization of the goals and recommendations for identifiers and names in Kubernetes. Described in GitHub issue #199.
+A summarization of the goals and recommendations for IDs and names in Kubernetes. Described in GitHub issue #199.

s/IDs/uids/ (or UIDs?)

Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/1216/files#r17270167
.

## Design

1) Each apiserver has a Namespace string (a DNS_SUBDOMAIN) that is unique across all apiservers that share its configured minions.
1) Each apiserver must be assigned a Namespace string (a DNS_SUBDOMAIN).
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this?

We should not call this "namespace", since that currently collides with the namespace proposal.

Also, in the case of replicated apiservers, they should all share the same namespace. So, this is really a type of administrative domain. I suggest punting this topic to #1114.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes we still need this, I think. This use of namespace predates the other
form, but I don't really think they are too confusing. PodNamespace?

The use case here is this.

An apiserver can put a pod named "foo" on a machine. A config file can put
a pod named "foo" on the same machine. We can differentiate them as
"etcd.foo" and "file.foo" or something, but then the name that kubelet
reports is not the same as the name in the apiserver. It seems that we
should be able to re-assemble the information regardless of where it came
from.

Maybe PodNamespace should just be a kubelet-local thing, and if you need to
join data across kubelets and masters you have to use UID?

On Mon, Sep 8, 2014 at 3:13 PM, bgrant0607 notifications@github.com wrote:

In docs/identifiers.md:

Design

-1) Each apiserver has a Namespace string (a DNS_SUBDOMAIN) that is unique across all apiservers that share its configured minions.
+1) Each apiserver must be assigned a Namespace string (a DNS_SUBDOMAIN).

Do we still need this?

We should not call this "namespace", since that currently collides with
the namespace proposal.

Also, in the case of replicated apiservers, they should all share the same
namespace. So, this is really a type of administrative domain. I suggest
punting this topic to #1114
#1114.

Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/1216/files#r17270876
.

Copy link
Member

Choose a reason for hiding this comment

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

Drop this point, I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. There's still some pod-specific language to root out.

Copy link
Member

Choose a reason for hiding this comment

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

I'd put a FIXME here regarding this issue, and we should hash this out in #1114 . There are several possible solutions, such as host-specific namespaces or autosetName=true. Namespace per apiserver is definitely not right given other discussions about replicating and/or sharding the apiservers, as well as namespacing by user-oriented scopes rather than (or in addition to) system-oriented ones. We will effectively need to namespace by store/etcd cluster, since a shared index is the underlying mechanism for enforcing uniqueness. However, as discussed in #1184, we could shard the store by resource kind, in which case namespacing would be implicit in the path part of the URL.

@smarterclayton
Copy link
Contributor

+1, uid / name removes all confusion over identifier, better descriptions in general

@derekwaynecarr
Copy link
Member

LGTM

@derekwaynecarr
Copy link
Member

Just to clarify, do you believe the current ID field becomes "Name", and that we have a new as yet unset field UID?

@thockin
Copy link
Member Author

thockin commented Sep 9, 2014

@derekwaynecarr in short, yes

@thockin
Copy link
Member Author

thockin commented Sep 9, 2014

@smarterclayton Was previously arguing for Name being a optional (really defaultable) field. we discussed here and feel pretty strongly the opposite, but we want to nail down the use case for it and final decision. We're open to a compromise.

@derekwaynecarr
Copy link
Member

@thockin - this LGTM, I am just trying to work on a parallel WIP phased implementation of the namespaces work, so if phase 1 is ID (old terminology) is Name (new terminology) then it keeps things a little simpler.

@bgrant0607
Copy link
Member

@thockin @smarterclayton Re. optional Names: The one use case I could think of was repeated (e.g. periodic) creation via webhook. Clayton, what use case did you have in mind?


5. Kubelet validates the input.

6. Kubelet further namespaces the pod name with information about the source of the pod.
Copy link
Member

Choose a reason for hiding this comment

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

I thought we were not covering namespaces here yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

Kubelet HAS to have some concept of namespacing, or else we can not differentiate a user pod named "cadvisor" from the config-file carried pod named "cadvisor". At your request, I buried this into kubelet, but I don't think it can go away. If the "other" namespacing proposal imposes new fields, we can hoist it back up to the API.

Copy link
Member

Choose a reason for hiding this comment

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

If we're going to mention namespaces here, then I think we should reconcile with #1114.

Kubelet should not alter the namespaces of pods that go through the apiserver, since they should arrive with a namespace as per #1114. The apiserver should set a default namespace in the case that one is not provided.

So, in the case of a pod created from a file on the Kubelet, I think it's ok for Kubelet to assign a namespace using a hash or whatever if one is not specified. OTOH, I also think it should be legitimate for a file to specify a namespace, such as one derived from the minion's hostname and owner. Ensuring that such namespaces don't collide would be the responsibility of the cluster admin.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't have it both ways - I can not have this spec leave namespacing out of teh main portion and simultaneously say that kubelet can't apply namespaces.

If we start here, and #1114 flies, then we hoist namespace up and rejigger the rules a bit. Or I can go back to saying that all objects have a namespace string, and let #1114 be the decider of that string.

Copy link
Member

Choose a reason for hiding this comment

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

The current text says:

Kubelet further namespaces the pod name with information about the source of the pod.

The namespace concept shouldn't be specific to Kubelet. We should add namespaces generally.

apiserver should fill it in if it's unspecified. Kubelet should expect the namespace to be populated in pods from the apiserver.

The namespace may be populated in pods in files on minions, but if it's not, Kubelet can automatically fill it in.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's the larger namespaces proposal, which segments a cluster, and the general idea of namespacing. This is the latter. If we end up having the former, it can replace this.

I'd be happy to add it back now, but I removed it at your request, so I am confused :)

@thockin
Copy link
Member Author

thockin commented Sep 9, 2014

I think that was Clayton's use case too, and the only one we could dream up. The half-baked compromise we came up with was a new "I don't know my own name, please assign me one" flag. The goal being to encourage idempotent operation by default, but to allow an explicit opt-out for corner cases. Key here is EXPLICIT. How bad is this?

@smarterclayton
Copy link
Contributor

The half-baked compromise we came up with was a new "I don't know my own name, please assign me one" flag. The goal being to encourage idempotent operation by default, but to allow an explicit opt-out for corner cases.

Like I said in the other thread, we can always make Name defaultable later if we so decide. We should make a good client side random name generator for Go, and have some examples pointing to how I can randomize it. I don't like encouraging people to generate random things in general (would prefer to give them a name by default) but I won't hold out on it. Non-defaultable -> defaultable is an easier transition than the opposite.

name
: A human readable string intended to help an end user distinguish between similar but distinct entities
Name
: A non-empty string guaranteed to be unique within a given scope at a particular time; used in resource URLs; provided by clients at creation time and encouraged to be human friendly; intended to facilitate creation idempotence and space-uniqueness of singleton objects, distinguish distinct entities, and reference particular entities across operations.
Copy link
Member

Choose a reason for hiding this comment

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

This should be clarified to be:

"A non-empty string guaranteed to be unique to a resource type within a given scope at a particular time."

It should be a-ok to have different resource types that have the same name in a particular scope.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree with your point, but I think "within a given scope" captures that. It's already a mouthy sentence.

@smarterclayton
Copy link
Contributor

Is autosetName something that should be part of a resource, or outside of a resource (param, etc)? It's not really an attribute you save with an object. Making it explicit certainly seems reasonable and clear for clients.

@bgrant0607
Copy link
Member

@smarterclayton Yes, autosetName would be a parameter.

@thockin
Copy link
Member Author

thockin commented Sep 10, 2014

Documented auto-set names. Namespaces is the only open topic, I think?

@bgrant0607
Copy link
Member

Look at the formatted view. I think you need to further indent examples in order to create line breaks, or put them into code blocks, or something.

@thockin
Copy link
Member Author

thockin commented Sep 11, 2014

Hmm, I see proper formatting - what am I missing?

On Wed, Sep 10, 2014 at 10:39 AM, bgrant0607 notifications@github.com
wrote:

Look at the formatted view. I think you need to further indent examples in
order to create line breaks, or put them into code blocks, or something.

Reply to this email directly or view it on GitHub
#1216 (comment)
.

@thockin
Copy link
Member Author

thockin commented Sep 11, 2014

Ready for another round - namespace have been re-added more generically.


5) Provide human-friendly, memorable, semantically meaningful, short-ish references in container and pod operations
2. When an object is created via an api, a Namespace string (a DNS_SUBDOMAIN? format TBD via #114) may be specified. Depending on the API receiver, namespaces might be validated (e.g. apiserver might ensure that the namespace actually exists). If a namespace is not specified, one will be assigned by the API receiver. This assignment policy might vary across API receivers (e.g. apiserver might have a default, kubelet might generate something semi-random).
Copy link
Member

Choose a reason for hiding this comment

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

#1114 actually

Copy link
Member Author

Choose a reason for hiding this comment

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

oops

@bgrant0607
Copy link
Member

LGTM, except for the 2 minor issues I just commented on.

Are issue references auto-linked in .md files? They aren't in the formatted preview. Not a big deal if they aren't.

@thockin
Copy link
Member Author

thockin commented Sep 12, 2014

I don't know if xrefs auto-link. Never tried until now.

On Thu, Sep 11, 2014 at 2:32 PM, bgrant0607 notifications@github.com
wrote:

LGTM, except for the 2 minor issues I just commented on.

Are issue references auto-linked in .md files? They aren't in the
formatted preview. Not a big deal if they aren't.

Reply to this email directly or view it on GitHub
#1216 (comment)
.

@thockin
Copy link
Member Author

thockin commented Sep 12, 2014

@smarterclayton @derekwaynecarr Want a last look?

Example: "01234567-89ab-cdef-0123-456789abcdef_frontend_k8s.example.com"
2. Kubelet validates the input.
1. Since UID is not provided, kubelet generates one.
2. Since Namepsace is not provided, kubelet generates one.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, eagle eyes.

@smarterclayton
Copy link
Contributor

LGTM

@derekwaynecarr
Copy link
Member

LGTM - Thank you for adding the scenarios. Hopefully we can close on the Namespace proposal at the same time now.

@thockin
Copy link
Member Author

thockin commented Sep 12, 2014

Is it worth adding a #1114-invoking case to the last section?

On Fri, Sep 12, 2014 at 8:13 AM, Derek Carr notifications@github.com
wrote:

LGTM - Thank you for adding the scenarios. Hopefully we can close on the
Namespace proposal at the same time now.

Reply to this email directly or view it on GitHub
#1216 (comment)
.

thockin added a commit that referenced this pull request Sep 16, 2014
Revisit identifiers spec
@thockin thockin merged commit d6bb688 into kubernetes:master Sep 16, 2014
@thockin thockin deleted the ident branch October 17, 2014 06:17
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

5 participants