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

Add a couple of useful environment variables to the container. #997

Closed
wants to merge 1 commit into from

Conversation

brendandburns
Copy link
Contributor

No description provided.

func makeContainerEnvironmentVariables(pod *Pod, container *api.Container) []string {
return []string{
fmt.Sprintf("K8S_POD_ID=%s", pod.Manifest.ID),
fmt.Sprintf("K8S_CONTAINER_NAME=%s", container.Name),
Copy link
Member

Choose a reason for hiding this comment

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

also apiserver address might be nice!

@lavalamp
Copy link
Member

This really needs documentation somewhere to be complete, I think. Not quite sure where the best place is. In an example somewhere?

@smarterclayton
Copy link
Contributor

So in #386 we made an argument not to do this because it's a point of coupling that has bad repercussions. New arguments?

@lavalamp
Copy link
Member

Hm. I didn't recall that issue, I think I probably hadn't read it completely before. Sigh, I guess we need some sort of replacement scheme? :( :(

@smarterclayton
Copy link
Contributor

I can think of quite a few use cases for replacement / gen, but all of them need a good deal of thought (rename service variable, generate token that allows access to k8s API for current scope, volume path or other info).

Can we set hostname to pod id at a minimum?

@thockin
Copy link
Member

thockin commented Aug 23, 2014

LGTM

@thockin
Copy link
Member

thockin commented Aug 23, 2014

I re-read #386 and I can't disagree with it.

I was thinking of something like:

Create a ShardController. This is very much like a ReplicaController except it takes two arguments N (number of shards) and M (replicas per shard). It would add a label and env var indicating which shard ID a given pod is.

We probably need to handle re-sharding, but maybe we can get away without doing that "in flight"?

@smarterclayton
Copy link
Contributor

That makes sense to me generally. Resharding generally is either planned (where automatic decisions are dangerous) or dynamic (where containers can gossip to the cluster).

@smarterclayton
Copy link
Contributor

Another option would be to make a ReplControllerController that can vary the desired pod template. Not sure that's superior

@lavalamp
Copy link
Member

OK, I have a proposal to make this work before we figure out how we're going to do replacement in general.

a) kubelet's HTTP server gets an endpoint that serves info useful to a pod (what state it should be in, master address, its ID, other stuff we think of).
b) We add a link to this as an ENV var
c) We do some IP tables magic to hook up the link we give to the pod to the endpoint in kubelet, which prevents pods from seeing each other's info.

This will continue to be useful even when we get replacement.

@dbcode
Copy link
Contributor

dbcode commented Aug 26, 2014

Something like a per-pod metadata server / key-value store?

@smarterclayton
Copy link
Contributor

I think the argument in the other thread though was that getting access to this info via any ENV var is coupling (whether it's a discovery link in an ENV var or just an ENV var). It shouldn't be that you can't couple.... it's just that you shouldn't be depending on the system to couple you unless you ask it to (from Kubernetes).

No matter what magic we do, the schema of the response is an implicit API. I think we were hoping that would be more of a generic "container info pattern" (in Docker, in general use) rather than "you can fetch a REST endpoint to get info about yourself" which you can do today.

@lavalamp
Copy link
Member

Can we just define a static IP/path then, so there's no need to express in an ENV var what it is?

I think this is OK anyway, because the environment variable won't be useful unless you've specifically written code to dial in and get the info, at which point I think it's clear that you're accepting the coupling.

We still need env var replacement to enable non-coupled tasks.

@lavalamp
Copy link
Member

Assigning to @smarterclayton-- I would close it but I want to hear his response first. It seems clear that this isn't going to go in as-is.

@smarterclayton
Copy link
Contributor

Following with @thockin's proposal on stable IPs for services, we could declare a "special" IP (or name in hostfile) that corresponds to the kubelet. I feel like there's security implications there, so we would want to implement the necessary work in the kubelet server to only honor those requests that come from the appropriate container (otherwise it's a security hole waiting to happen if we forget about it). Is now the right time to do that? I don't know, depends on how many use cases are depending on talking to the kubelet to get this info. What's the use case?

@dbcode
Copy link
Contributor

dbcode commented Aug 29, 2014

Just checking in on this. The original request was just to make the pod ID available within the container somehow. One simple way to do this is to put it in the environment of the container. There are other, equivalently simple ways such as an /etc file and so on. Does this request really warrant a more complex solution than that? The diff is 4 lines. It cries out to be committed.

@smarterclayton
Copy link
Contributor

Speaking to the concern, there are significant coupling issues associated with magic env vars only available on one platform. For instance, a docker image that depends on K8S_POD_ID now no longer runs on any system that doesn't auto set that variable.

Brendan's pull sets the pod id as the hostname. If you know you're on Kubernetes, use the hostname as your locator. Is there an additional attribute you need?

@lavalamp
Copy link
Member

@dbcode We did change the host name to the pod id in another PR. That was uncontroversial.

@thockin
Copy link
Member

thockin commented Aug 29, 2014

I sort of like the idea of exposing the kubelet as a metadata server, but I am worried that this is rube-goldberg in the making.

It's fairly simple to install a "magic" IP and redirect it to the kubelet on localhost (bonus, I am 80% sure the incoming traffic comes from 127.0.0.1, so you can check the source IP for safety). But that doesn't solve the problem that pods can see each other, unless we do different names and iptables magic per pod, which I'd rather avoid. And if we don't do that depth of magic, is an env var ("K8S_AGENT=1.2.3.4:10250" or something) so bad?

I think there are probably cool things that k8s-aware apps can do, if they can talk to kubelet.

@dbcode
Copy link
Contributor

dbcode commented Aug 29, 2014

Is the hostname guaranteed always to serve as a unique pod identifier? Is this convention going to be adopted by all other platforms? What happens if someone makes a platform and decides that every pod should have the same hostname?

From my perspective, this is actually a bit worse than having a separate environment variable, since I now have to rely on the platform overloading the hostname to supply the pod ID, as Kubernetes does. But I am guessing this is not in any standard or specification, nor will it be, so it is no better than a magic environment variable to me.

It reminds me of LD_LIBRARY_PATH, SHLIB_PATH, DYLD_LIBRARY_PATH, and so on. Sure, not every platform chose to name it the same thing. But I've used libltdl for many years and it has worked fine.

@thockin
Copy link
Member

thockin commented Aug 29, 2014

Dave: I agree hostname is just as much a coupling as an env var. Even more
subtle, really.

Let's keep working through this. I think we're close to either having
something workable or else deciding that env vars are not so bad.

As another straw man, since we really don't want to do textual substitution
on pod specs :

What if we added a new field per-pod or per-container that was something
like "export my pod ID as an env var named ..."

envPodID: "MY_POD" => an env var named "MY_POD" with value=pod ID.

It's gross because we probably have 3 or 4 such things we might want to
expose, but is it worse than a metadata server? Simpler by far...

On Fri, Aug 29, 2014 at 12:52 AM, Dave Bailey notifications@github.com
wrote:

Is the hostname guaranteed always to serve as a unique pod identifier? Is
this convention going to be adopted by all other platforms? What happens if
someone makes a platform and decides that every pod should have the same
hostname?

From my perspective, this is actually a bit worse than having a separate
environment variable, since I now have to rely on the platform overloading
the hostname to supply the pod ID, as Kubernetes does. But I am guessing
this is not in any standard or specification, nor will it be, so it is no
better than a magic environment variable to me.

It reminds me of LD_LIBRARY_PATH, SHLIB_PATH, DYLD_LIBRARY_PATH, and so
on. Sure, not every platform chose to name it the same thing. But I've used
libltdl for many years and it has worked fine.

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

@dbcode
Copy link
Contributor

dbcode commented Aug 29, 2014

That seems simple enough to me. Or maybe an env object in the spec, like

platformEnv : {
podId : "MY_POD",
containerName : "MY_CONTAINER",
...
}

where the thing that processes the spec iterates the key-value pairs in the platformEnv object and basically does the equivalent of "export value=platformEnvLookup(key)" (or prints some sort of warning or error if the platform environment doesn't understand a given key).

At least in this scenario (or the one you proposed), the name of the environment variable isn't "magical". But to be honest, at least for something like pod ID, which strikes me as something for which every platform will have an equivalent, I would think you could just drop the K8S_ prefix and use something more neutral for the name (e.g. POD_ID or NODE_ID, CONTAINER_NAME, whatever). Then just document that these are the things we put into the container environment, these are the concepts to which they correspond, and if you are going to have your platform put similar things in the container environment, consider adopting the conventions we've set with our (platform-neutral) names. It might be good to just say, look, here is the de facto standard for these things, and everyone else will be relieved that they don't have to deal with the decision fatigue that comes from naming (which is, after all, the hardest problem...).

@lavalamp
Copy link
Member

What if we added a new field per-pod or per-container that was something like "export my pod ID as an env var named ..."

O_o

I've resigned myself to waiting for the generators from the config proposal to arrive. :(

@smarterclayton
Copy link
Contributor

So I still haven't seen the use case we're trying to solve here. The point of being skittish about ENV coupling is to prevent people from coupling images to kube specifically. Why does a pod need to know who it is? The point of host name = pod id was to define a stable name across restarts or container metadata changes, which is roughly the same promise the operating system provides. It was also to be unique in the cluster, which networks generally provide.

Gimme a concrete use case and let's solve that specifically.

@smarterclayton
Copy link
Contributor

On Aug 29, 2014, at 1:52 AM, Dave Bailey notifications@github.com wrote:

Is the hostname guaranteed always to serve as a unique pod identifier?

We can certainly make that guarantee in Kubernetes (unless you ask for the hostname to be something different).
Is this convention going to be adopted by all other platforms?

Why does it matter unless the other platforms expose the same ID and API endpoints? If you are using another platform, what does knowing pod id give you?
What happens if someone makes a platform and decides that every pod should have the same hostname?

From my perspective, this is actually a bit worse than having a separate environment variable, since I now have to rely on the platform overloading the hostname to supply the pod ID, as Kubernetes does. But I am guessing this is not in any standard or specification, nor will it be, so it is no better than a magic environment variable to me.

It reminds me of LD_LIBRARY_PATH, SHLIB_PATH, DYLD_LIBRARY_PATH, and so on. Sure, not every platform chose to name it the same thing. But I've used libltdl for many years and it has worked fine.


Reply to this email directly or view it on GitHub.

@brendandburns
Copy link
Contributor Author

closing this, the hostname is made to be the pod identifier in a different CL, and we've been discussing other alternatives for discovering the API endpoint.

b3atlesfan pushed a commit to b3atlesfan/kubernetes that referenced this pull request Feb 5, 2021
Allow changing subnet/kube annotation prefix
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.

5 participants