[WIP] - Standardize identifier behavior across apiserver #253
Conversation
Autogenerate an ID if not specified
| @@ -57,6 +57,7 @@ type EnvVar struct { | |||
|
|
|||
| // Container represents a single container that is expected to be run on the host. | |||
| type Container struct { | |||
| ID string `yaml:"id,omitempty" json:"id,omitempty"` | |||
| Name string `yaml:"name,omitempty" json:"name,omitempty"` | |||
There was a problem hiding this comment.
I am not a fan of having both Name and ID. Very confusing. Not sure if there's an alternative. :/
There was a problem hiding this comment.
For example, what if we required container's name field, and generate a unique ID for the pod/manifest, and then have kubelet combine them?
There was a problem hiding this comment.
That seems much cleaner to me - reject containers with the same name in the pod and ensure pod ID exists or generate it.
There was a problem hiding this comment.
Please see my email from this evening discussing. I'm concerned that there
are a lot of forked efforts here, and I want to make sure we don't step on
each other or accept a solution we don't like because it happens to be
implemented :)
On Wed, Jun 25, 2014 at 8:34 PM, Clayton Coleman notifications@github.com
wrote:
In pkg/api/types.go:
@@ -57,6 +57,7 @@ type EnvVar struct {
// Container represents a single container that is expected to be run on the host.
type Container struct {
- ID string
yaml:"id,omitempty" json:"id,omitempty"
Name stringyaml:"name,omitempty" json:"name,omitempty"That seems much cleaner to me - reject containers with the same name in
the pod and ensure pod ID exists or generate it.Reply to this email directly or view it on GitHub
https://github.com/GoogleCloudPlatform/kubernetes/pull/253/files#r14224118
.
There was a problem hiding this comment.
I'm not sure that a container within a manifest needs an ID since I think we can assume (and enforce) that name be unique within the manifest. My work on IDs was mostly internal cleanup and still maintaining the the docker name as mangle(manifest.id, container.name)
There was a problem hiding this comment.
I agree - containers withing a pod should only need a name.
|
@jjhuff is working on fixing up the kubelet's container naming scheme, which is the other half of this. See: jjhuff@ff131a9 |
|
My internal Id cleanup is in #254 |
|
Would like to include on top of #280 |
smarterclayton
changed the title
smarterclayton
changed the title
|
Currently I'm using full UUID - in practice and for readability I could drop it down to 64 bits and then do a base64 (omitting - and _). That would make generated ids immediately more human readable from the apiserver and kubelet, at the risk of moving to a 1 in 10 million chance of a collision with ~5 million containers. |
|
Are you ready for a thorough review on this one? On Tue, Jul 1, 2014 at 7:50 AM, Clayton Coleman notifications@github.com
|
|
General approach and uuid type (full uuid vs shorter random segment) would be helpful |
|
It would help a lot for review if you sent a PR just for the Id -> ID change On Tue, Jul 1, 2014 at 9:59 AM, Clayton Coleman notifications@github.com
|
|
Done |
|
Directionally good. On Tue, Jul 1, 2014 at 10:18 AM, Clayton Coleman notifications@github.com
|
Also handle SIGTERM.
change the grafana image and add our new monitoring job to the monitoring addon.
change the grafana image and add our new monitoring job to the monitoring addon.
change the grafana image and add our new monitoring job to the monitoring addon.
change the grafana image and add our new monitoring job to the monitoring addon.
change the grafana image and add our new monitoring job to the monitoring addon.
add setprefixname command
Empty LogPath will use journald's default path.
Depends on #321