play kube: set defaults to container resources

[ydayagi]

this fixes #13115

the change tries to imitate k8s behavior.
when limits are not set the container's limits are all CPU and all RAM
when requests are missing then they are equal to limits

Signed-off-by: Yaron Dayagi ydayagi@redhat.com

[rhatdan]

Thanks @ydayagi the linter does not like your tests.
@cdoern @vrothberg @umohnani8 PTAL

[ydayagi]

/retest

[openshift-ci]

@ydayagi: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ydayagi]

Thanks @ydayagi the linter does not like your tests. @cdoern @vrothberg @umohnani8 PTAL

@rhatdan @umohnani8 @vrothberg @cdoern
I think that the tests are failing due to some temporary/flaky timeout. can u please retest?

[TomSweeneyRedHat]

All kinds of red test unhappiness @ydayagi

[rhatdan]

I told it to retest, will see how it does.

[ydayagi]

@TomSweeneyRedHat I think it is related to
https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-cpu-limits-fails-with-a-permissions-error

[ydayagi]

@TomSweeneyRedHat @rhatdan
looking at troubleshooting-cpu-limits....
could it be that Podman tests do not take into account memory and cpu limits? perhaps we don't test 'play kube' with limits?
or maybe, we should avoid setting limits (here) if user does not have permissions for it?

[rhatdan]

@cdoern @mheon PTAL

[Luap99]

Is a limit really what most people want by default? I don't have a opinion on that but I just want to make us aware that this will be a breaking change for users.

[rhatdan]

Well the guide should be to match what kubernetes does. @umohnani8 @saschagrunert @haircommander @mrunalp WDYT?

[cdoern]

I am pretty sure for most of these fields a limit of 0 is the default which means no limit, or at least that is how podman parses a limit of 0 or nil in some circumstances. why would we place a resource limit that users aren't aware of?

[haircommander]

I'm actually having a bit of trouble finding the default limits here. mostly because poking around the documentation keeps pointing to limit ranges which allow you to customize the default limits and requests. @ydayagi do you have a pointer to where the defaults are specified in kube

[ydayagi]

I'm actually having a bit of trouble finding the default limits here. mostly because poking around the documentation keeps pointing to limit ranges which allow you to customize the default limits and requests. @ydayagi do you have a pointer to where the defaults are specified in kube

thank you all for your replies. I did not check the documentation. I tested a few times with an existing cluster. I think that as Dan mentioned we have to match the k8s behavior. Another reason to set default is the env var. If env var references the limit field but the field has no value then var value is wrong.

[ydayagi]

@rhatdan ?

[rhatdan]

If you generate a Pod with this, what does the podman generate kube YAML look like? Do we need to strip all of these fields back out?

[ydayagi]

If you generate a Pod with this, what does the podman generate kube YAML look like? Do we need to strip all of these fields back out?

indeed the result of 'generate kube' changed. I will fix that.

[ydayagi]

@rhatdan I removed the default setting in the container object. Instead, I use default values when setting the env var value (func envVarValue).

[ydayagi]

@rhatdan

1. I could not figure out the reason for the failed tests. Can we restest?
2. I think I'll have to rename the PR and bug to something like: defaults for env from resource

[umohnani8]

LGTM

[rhatdan]

/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan, ydayagi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment