Skip to content

Conversation

@ZYecho
Copy link

@ZYecho ZYecho commented Feb 28, 2019

Signed-off-by: zhangyue zy675793960@yeah.net
add warn log when memory resource request greater than limit
fix #418

Copy link
Member

@sdudoladov sdudoladov left a comment

Choose a reason for hiding this comment

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

Please also add this warning to the code that configures sidecars and init containers as they are also affected. Note that a Scalyr sidecar is configured separately.

@ZYecho
Copy link
Author

ZYecho commented Mar 5, 2019

Please also add this warning to the code that configures sidecars and init containers as they are also affected. Note that a Scalyr sidecar is configured separately.

they already use RequestIsSmallerThanLimit to judge and log a warn, what else do you want?

Signed-off-by: zhangyue <zy675793960@yeah.net>
@sdudoladov
Copy link
Member

Please also add this warning to the code that configures sidecars and init containers as they are also affected. Note that a Scalyr sidecar is configured separately.

they already use RequestIsSmallerThanLimit to judge and log a warn, what else do you want?

logging a warning when a resource request as seen by the operator is greater than the limit for all the containers that an operator can potentially create. This includes up to 4 container types:

  1. Postgres container - already covered by this PR
  2. Scalyr sidecar
  3. init containers
  4. other sidecars

the whole idea is to make visible in the logs a situation where a container cannot start because the memory request > limit . Right now finding this is the case involves running kubectl describe on various k8s resources. Logging such misconfiguration shall improve the operational experience for the operator users.


isSmaller, err := util.RequestIsSmallerThanLimit(spec.ResourceRequests.Memory, spec.ResourceLimits.Memory)
if err == nil && !isSmaller {
notAllowed("memoryRequest", "higher than resource limit")
Copy link
Member

Choose a reason for hiding this comment

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

That won't work because that will reject the legitimate case of request being equal to limit.

In RequestIsSmallerThanLimit, the line

 return request.Cmp(limit) == -1, nil

is responsible for that . request.Cmp(limit) will return zero if quantities are equal. Please check Kubernetes source code / docs for that method.

@ZYecho ZYecho closed this Mar 9, 2019
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.

Warn if memory request > memory limit

2 participants