-
Notifications
You must be signed in to change notification settings - Fork 40k
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
REST api - "env" section seems to be structured differently than other key value pair attributes #5490
Comments
cc @oschreib |
Env is a list. Long-debated topic. https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/api-conventions.md#lists-of-named-subobjects-preferred-over-maps @ghodss Any preference for list vs. map with respect to Env specifically? |
Labels are represented internally as a map, so it doesn't suffer from all the issues of trying to represent a list of subobjects as a map. EnvVars are currently represented as objects, so they need to be a list. If they were fully converted to be a map[string]string and not a list of subobjects, then there's no issue turning the API representation into a map as well. One question we should ask ourselves before converting to a map is, will env vars ever need additional attributes or metadata on a per-env-var basis? Maybe you could indicate that a given env var is encrypted, or some other metadata that you want to associate. That seems unlikely, and Docker even represents it just as a list of strings, so unless someone can think of something I would not be opposed to removing the EnvVar object and changing EnvVars to be maps both internally and in the API. This does have the negative effect of having more YAML keys that don't represent API fields (discussed in #2004 (comment)), but if we're going to be strict on that, then labels need to be converted to a list of Label objects as well, which I personally am not opposed to, but I don't know if anyone would get on board with that. |
@thockin reminded me of the proposal to add a flag to each env. var. that indicates whether expansion should be performed: #560 (comment) The alternative would be 2 separate lists/maps. I don't think we're going to have a 100% consistent policy about lists vs. maps. |
We should make a call on this, at least for the scope of v1 API, and close the issue. My bias is to leave it as is, but if others feel strongly we can change it to a map. |
There's a PR in flight to add a third field, making this no long On Fri, Apr 24, 2015 at 4:31 PM, Sam Ghods notifications@github.com wrote:
|
FYI, that PR is #6739. |
The "env" section in containers in pod has a different structure than other key/value structures as labels.
For example, labels is a hash with key:value pairs.
"labels": {
"role": "pod",
"mylabel": "mylabelvalue"
}
But "env" for some reason is an array of hashes where there are always 2 key names hardcoded:
"env": [
{
"name": "STORAGE",
"value": "local"
},
{
"name": "STORAGE_PATH",
"value": "/var/lib/docker-registry"
}
]
Why are the "name" and the "value" attributes required and making this an array of hashes, rather than making it like labels a single hash with key:value structure without hardcoding name/value attribute names?
for example:
the current:
"name": "STORAGE_PATH",
"value": "/var/lib/docker-registry"
can become:
"STORAGE":"local"
It will make it consistent with other key value pairs in the project, but will also reduce dramatically the json size when it comes to many containers per pod.
The text was updated successfully, but these errors were encountered: