feat(limits): create limits for each deployment from generate_params

[smothiki]

components with deployments have resource limits set to deployments objects .

[deis-bot]

@bacongobbler, @jchauncey and @krancour are potential reviewers of this pull request based on my analysis of git blame information. Thanks @smothiki!

[krancour]

I'm worried about the conditional logic that compares to the placeholder string "set". I feel its obvious that it's a placeholder now, but if someone replaces "set" with a value, then comes back to it in the future to remove the limit, they might just delete the line. In such a case, the limit will neither be "set" nor a valid value and will result in failures, I believe.

Maybe we should check for values exist and are not "set"?

[smothiki]

@krancour I will add more logic to see if I can check if a variable exists or not.

[krancour]

Maybe, exists, not "", and not "set"? That starts to sound a little complicated, tho. idk.

[smothiki]

@krancour updated the chart and just ran with custom cpu and memory setting. Every thing works fne. PTAL

[krancour]

@smothiki this looks good. One question... since we've eliminated the placeholders, do you think there should be a commented example in each section of the toml to show how it's done? Anyone wanting to set can just uncomment and modify?

[krancour]

Either way... LGTM. Up to you whether you address previous comment or not.

[smothiki]

@krancour I have added docs for the changes But I think there should be one commented example to show how to set limits

[smothiki]

Manually tested multiple times with workflow-dev chart

[helgi]

Thoughts on the naming, since we are only doing limits for now and not requests, are we painting ourselves into corner by not mentioning limits in the toml? what will it look like if we supported requests

[smothiki]

Thoughts on the naming, since we are only doing limits

cpu and memory are for limits if user have to set requestes we should come up with another variable with request_cpu , request_memory

[helgi]

Alright, I'm fine with that, mostly wondering if we should namespace it right now to be more explicit upfront. .limits.cpu off the bat or similar

[smothiki]

That's also a good approach there is more to limits than just setting memory and CPU would like to namespace limits into it's own category during developing requests limits.

[helgi]

Isn't that too late tho since it would already be out in the wild? you'd have to support this approach and whatever limits namespacing you do going forward

[smothiki]

As per conversation with @helgi comments made changes to cpu and memory variables prefixed with limits_