Skip to content

Commit

Permalink
[TRB-54994]: Fixed a few issues that could cause resize actions to fa…
Browse files Browse the repository at this point in the history
…il in namespaces with a LimitRange. (turbonomic#91)

* [TRB-54994]: Fixed a few issues that could cause resize actions to fail in namespaces with a LimitRange.

* Added comments indicating that we have updated code taken from upstream Kubernetes.
  • Loading branch information
amd-ibm authored and GitHub Enterprise committed Jan 16, 2024
1 parent 5e860b8 commit 0c52a40
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 26 deletions.
51 changes: 29 additions & 22 deletions pkg/action/executor/limitrange_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,16 +155,18 @@ func PodValidateLimitFunc(limitRange *corev1.LimitRange, pod *corev1.Pod) error
return utilerrors.NewAggregate(errs)
}

// ===========================================================================
// NOTE: This method has deviated from the upstream kubernetes code.
// If we ever pull the latest changes from upstream, we may need to manually
// update this method.
// ===========================================================================
// minConstraint enforces the min constraint over the specified resource
func minConstraint(limitType string, resourceName string, enforced resource.Quantity, request corev1.ResourceList, limit corev1.ResourceList) error {
req, reqExists := request[corev1.ResourceName(resourceName)]
lim, limExists := limit[corev1.ResourceName(resourceName)]
observedReqValue, observedLimValue, enforcedValue := requestLimitEnforcedValues(req, lim, enforced)

if !reqExists {
return fmt.Errorf("minimum %s usage per %s is %s. No request is specified", resourceName, limitType, enforced.String())
}
if observedReqValue < enforcedValue {
if reqExists && observedReqValue < enforcedValue {
return fmt.Errorf("minimum %s usage per %s is %s, but request is %s", resourceName, limitType, enforced.String(), req.String())
}
if limExists && (observedLimValue < enforcedValue) {
Expand All @@ -173,16 +175,18 @@ func minConstraint(limitType string, resourceName string, enforced resource.Quan
return nil
}

// ===========================================================================
// NOTE: This method has deviated from the upstream kubernetes code.
// If we ever pull the latest changes from upstream, we may need to manually
// update this method.
// ===========================================================================
// maxConstraint enforces the max constraint over the specified resource
func maxConstraint(limitType string, resourceName string, enforced resource.Quantity, request corev1.ResourceList, limit corev1.ResourceList) error {
req, reqExists := request[corev1.ResourceName(resourceName)]
lim, limExists := limit[corev1.ResourceName(resourceName)]
observedReqValue, observedLimValue, enforcedValue := requestLimitEnforcedValues(req, lim, enforced)

if !limExists {
return fmt.Errorf("maximum %s usage per %s is %s. No limit is specified", resourceName, limitType, enforced.String())
}
if observedLimValue > enforcedValue {
if limExists && observedLimValue > enforcedValue {
return fmt.Errorf("maximum %s usage per %s is %s, but limit is %s", resourceName, limitType, enforced.String(), lim.String())
}
if reqExists && (observedReqValue > enforcedValue) {
Expand Down Expand Up @@ -315,24 +319,27 @@ func mergePodResourceRequirements(pod *corev1.Pod, defaultRequirements *corev1.R
}
}

// ===========================================================================
// NOTE: This method has deviated from the upstream kubernetes code.
// If we ever pull the latest changes from upstream, we may need to manually
// update this method.
// ===========================================================================
// mergeContainerResources handles defaulting all of the resources on a container.
func mergeContainerResources(container *corev1.Container, defaultRequirements *corev1.ResourceRequirements) {
if container.Resources.Limits == nil {
container.Resources.Limits = corev1.ResourceList{}
}
if container.Resources.Requests == nil {
container.Resources.Requests = corev1.ResourceList{}
}
for k, v := range defaultRequirements.Limits {
_, found := container.Resources.Limits[k]
if !found {
container.Resources.Limits[k] = v.DeepCopy()
if container.Resources.Limits != nil && len(container.Resources.Limits) > 0 {
for k, v := range defaultRequirements.Limits {
_, found := container.Resources.Limits[k]
if !found {
container.Resources.Limits[k] = v.DeepCopy()
}
}
}
for k, v := range defaultRequirements.Requests {
_, found := container.Resources.Requests[k]
if !found {
container.Resources.Requests[k] = v.DeepCopy()
if container.Resources.Requests != nil && len(container.Resources.Requests) > 0 {
for k, v := range defaultRequirements.Requests {
_, found := container.Resources.Requests[k]
if !found {
container.Resources.Requests[k] = v.DeepCopy()
}
}
}
}
8 changes: 4 additions & 4 deletions pkg/action/executor/workload_controller_resizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,16 +310,16 @@ func buildDesiredPod4QuotaEvaluation(namespace string, resizeSpecs []*containerR
if len(spec.NewCapacity) > 0 && len(podSpec.Containers[spec.Index].Resources.Limits) == 0 {
podSpec.Containers[spec.Index].Resources.Limits = make(k8sapi.ResourceList)
}
if newVal, found := spec.NewRequest[k8sapi.ResourceCPU]; found {
if newVal, found := spec.NewRequest[k8sapi.ResourceCPU]; found && !newVal.IsZero() {
podSpec.Containers[spec.Index].Resources.Requests[k8sapi.ResourceCPU] = newVal
}
if newVal, found := spec.NewRequest[k8sapi.ResourceMemory]; found {
if newVal, found := spec.NewRequest[k8sapi.ResourceMemory]; found && !newVal.IsZero() {
podSpec.Containers[spec.Index].Resources.Requests[k8sapi.ResourceMemory] = newVal
}
if newVal, found := spec.NewCapacity[k8sapi.ResourceCPU]; found {
if newVal, found := spec.NewCapacity[k8sapi.ResourceCPU]; found && !newVal.IsZero() {
podSpec.Containers[spec.Index].Resources.Limits[k8sapi.ResourceCPU] = newVal
}
if newVal, found := spec.NewCapacity[k8sapi.ResourceMemory]; found {
if newVal, found := spec.NewCapacity[k8sapi.ResourceMemory]; found && !newVal.IsZero() {
podSpec.Containers[spec.Index].Resources.Limits[k8sapi.ResourceMemory] = newVal
}
}
Expand Down

0 comments on commit 0c52a40

Please sign in to comment.