Skip to content

Commit

Permalink
Avoid using min and max to name vars
Browse files Browse the repository at this point in the history
  • Loading branch information
thbkrkr committed Nov 18, 2024
1 parent 724c90b commit 0375808
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 52 deletions.
1 change: 0 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ linters-settings:
disabled: false
- name: redefines-builtin-id
disabled: false
exclude: ["min", "max"]
- name: superfluous-else
disabled: false
- name: time-naming
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (ctx *Context) scaleHorizontally(
var nodeCount int32
for _, recommender := range ctx.Recommenders {
if recommender.HasResourceRecommendation() {
nodeCount = max(nodeCount, recommender.NodeCount(nodeCapacity))
nodeCount = max32(nodeCount, recommender.NodeCount(nodeCapacity))
}
}

Expand All @@ -151,7 +151,7 @@ func (ctx *Context) scaleHorizontally(
return nodeSetsResources
}

func max(a, b int32) int32 {
func max32(a, b int32) int32 {
if a > b {
return a
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,49 +524,49 @@ func NewAutoscalingSpecBuilder(name string) *AutoscalingSpecBuilder {
return &AutoscalingSpecBuilder{name: name}
}

func (asb *AutoscalingSpecBuilder) WithNodeCounts(min, max int) *AutoscalingSpecBuilder {
asb.nodeCountMin = int32(min)
asb.nodeCountMax = int32(max)
func (asb *AutoscalingSpecBuilder) WithNodeCounts(minCount, maxCount int) *AutoscalingSpecBuilder {
asb.nodeCountMin = int32(minCount)
asb.nodeCountMax = int32(maxCount)
return asb
}

func (asb *AutoscalingSpecBuilder) WithMemory(min, max string) *AutoscalingSpecBuilder {
func (asb *AutoscalingSpecBuilder) WithMemory(minMem, maxMem string) *AutoscalingSpecBuilder {
asb.memory = &v1alpha1.QuantityRange{
Min: resource.MustParse(min),
Max: resource.MustParse(max),
Min: resource.MustParse(minMem),
Max: resource.MustParse(maxMem),
}
return asb
}

func (asb *AutoscalingSpecBuilder) WithMemoryAndRatio(min, max string, ratio resource.Quantity) *AutoscalingSpecBuilder {
func (asb *AutoscalingSpecBuilder) WithMemoryAndRatio(minMem, maxMem string, ratio resource.Quantity) *AutoscalingSpecBuilder {
asb.memory = &v1alpha1.QuantityRange{
Min: resource.MustParse(min),
Max: resource.MustParse(max),
Min: resource.MustParse(minMem),
Max: resource.MustParse(maxMem),
RequestsToLimitsRatio: &ratio,
}
return asb
}

func (asb *AutoscalingSpecBuilder) WithStorage(min, max string) *AutoscalingSpecBuilder {
func (asb *AutoscalingSpecBuilder) WithStorage(minStorage, maxStorage string) *AutoscalingSpecBuilder {
asb.storage = &v1alpha1.QuantityRange{
Min: resource.MustParse(min),
Max: resource.MustParse(max),
Min: resource.MustParse(minStorage),
Max: resource.MustParse(maxStorage),
}
return asb
}

func (asb *AutoscalingSpecBuilder) WithCPU(min, max string) *AutoscalingSpecBuilder {
func (asb *AutoscalingSpecBuilder) WithCPU(minCPU, maxCPU string) *AutoscalingSpecBuilder {
asb.cpu = &v1alpha1.QuantityRange{
Min: resource.MustParse(min),
Max: resource.MustParse(max),
Min: resource.MustParse(minCPU),
Max: resource.MustParse(maxCPU),
}
return asb
}

func (asb *AutoscalingSpecBuilder) WithCPUAndRatio(min, max string, ratio resource.Quantity) *AutoscalingSpecBuilder {
func (asb *AutoscalingSpecBuilder) WithCPUAndRatio(minCPU, maxCPU string, ratio resource.Quantity) *AutoscalingSpecBuilder {
asb.cpu = &v1alpha1.QuantityRange{
Min: resource.MustParse(min),
Max: resource.MustParse(max),
Min: resource.MustParse(minCPU),
Max: resource.MustParse(maxCPU),
RequestsToLimitsRatio: &ratio,
}
return asb
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ func (m *cpu) ManagedResource() corev1.ResourceName {
}

func (m *cpu) NodeResourceQuantity() resource.Quantity {
max := m.autoscalingSpec.CPURange.Max.MilliValue()
maxCPU := m.autoscalingSpec.CPURange.Max.MilliValue()
// Surface the situation where CPU is exhausted.
if m.requiredNodeCPUCapacity.MilliValue() > max {
if m.requiredNodeCPUCapacity.MilliValue() > maxCPU {
// Elasticsearch requested more CPU per node than allowed by the user
err := fmt.Errorf("CPU required per node is greater than the maximum allowed")
m.log.Error(
Expand Down Expand Up @@ -71,8 +71,8 @@ func (m *cpu) NodeResourceQuantity() resource.Quantity {

// Resource has been rounded up or scaled up to meet the tier requirements. We need to check that those operations
// do not result in a resource quantity which is greater than the max. limit set by the user.
if nodeResource > max {
nodeResource = max
if nodeResource > maxCPU {
nodeResource = maxCPU
}
quantity := resource.NewMilliQuantity(nodeResource, resource.DecimalSI)
return *quantity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,24 @@ func getResourceValue(
totalRequired *client.AutoscalingCapacity, // tier required capacity as returned by the Elasticsearch API, considered as optional
quantityRange v1alpha1.QuantityRange, // as expressed by the user
) resource.Quantity {
max := quantityRange.Max.Value()
maxCapacity := quantityRange.Max.Value()
// Surface the situation where a resource is exhausted.
if nodeRequired.Value() > max {
if nodeRequired.Value() > maxCapacity {
// Elasticsearch requested more capacity per node than allowed by the user
err := fmt.Errorf("%s required per node is greater than the maximum one", resourceType)
log.Error(
err, err.Error(),
"scope", "node",
"policy", autoscalingSpec.Name,
"required_"+resourceType, nodeRequired,
"max_allowed_"+resourceType, max,
"max_allowed_"+resourceType, maxCapacity,
)
// Also update the autoscaling status accordingly
statusBuilder.
ForPolicy(autoscalingSpec.Name).
RecordEvent(
v1alpha1.VerticalScalingLimitReached,
fmt.Sprintf("%s required per node, %d, is greater than the maximum allowed: %d", resourceType, nodeRequired.Value(), max),
fmt.Sprintf("%s required per node, %d, is greater than the maximum allowed: %d", resourceType, nodeRequired.Value(), maxCapacity),
)
}

Expand Down Expand Up @@ -118,8 +118,8 @@ func getResourceValue(

// Resource has been rounded up or scaled up to meet the tier requirements. We need to check that those operations
// do not result in a resource quantity which is greater than the max. limit set by the user.
if nodeResource > max {
nodeResource = max
if nodeResource > maxCapacity {
nodeResource = maxCapacity
}

return v1alpha1.ResourceToQuantity(nodeResource)
Expand Down Expand Up @@ -192,11 +192,11 @@ func maxResource(a, b resource.Quantity) resource.Quantity {
}

func max64(x int64, others ...int64) int64 {
max := x
maximum := x
for _, other := range others {
if other > max {
max = other
if other > maximum {
maximum = other
}
}
return max
return maximum
}
22 changes: 11 additions & 11 deletions pkg/controller/common/version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ func (mmv MinMaxVersion) WithinRange(v Version) error {
return nil
}

func (mmv MinMaxVersion) WithMin(min Version) MinMaxVersion {
if min.GT(mmv.Min) {
func (mmv MinMaxVersion) WithMin(minVersion Version) MinMaxVersion {
if minVersion.GT(mmv.Min) {
return MinMaxVersion{
Min: min,
Min: minVersion,
Max: mmv.Max,
}
}
Expand Down Expand Up @@ -100,36 +100,36 @@ func WithoutPre(v Version) Version {

// MinInPods returns the lowest version parsed from labels in the given Pods.
func MinInPods(pods []corev1.Pod, labelName string) (*Version, error) {
var min *Version
var minVersion *Version
for _, p := range pods {
v, err := FromLabels(p.Labels, labelName)
if err != nil {
return nil, err
}

if min == nil || v.LT(*min) {
min = &v
if minVersion == nil || v.LT(*minVersion) {
minVersion = &v
}
}

return min, nil
return minVersion, nil
}

// MinInStatefulSets returns the lowest version parsed from labels in the given StatefulSets template.
func MinInStatefulSets(ssets []appsv1.StatefulSet, labelName string) (*Version, error) {
var min *Version
var minVersion *Version
for _, s := range ssets {
v, err := FromLabels(s.Spec.Template.Labels, labelName)
if err != nil {
return nil, err
}

if min == nil || v.LT(*min) {
min = &v
if minVersion == nil || v.LT(*minVersion) {
minVersion = &v
}
}

return min, nil
return minVersion, nil
}

func FromLabels(labels map[string]string, labelName string) (Version, error) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/elasticsearch/version/supported_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ func SupportedVersions(v version.Version) *version.MinMaxVersion {
return supportedVersionsWithMinimum(v, version.GlobalMinStackVersion)
}

func supportedVersionsWithMinimum(v version.Version, min version.Version) *version.MinMaxVersion {
if min.GT(v) {
func supportedVersionsWithMinimum(v version.Version, minVersion version.Version) *version.MinMaxVersion {
if minVersion.GT(v) {
return nil
}
return technicallySupportedVersions(v)
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/test/checks.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ func (e ExpectedSecret) MatchesActualSecret(k *K8sClient, namespace string) erro
}

// have the expected keys
min := len(e.Keys)
max := min + len(e.OptionalKeys)
if len(s.Data) < min || max < len(s.Data) {
return fmt.Errorf("expected between %d and %d keys in %s, got %d", min, max, e.Name, len(s.Data))
minExpectedKeys := len(e.Keys)
maxExpectedKeys := minExpectedKeys + len(e.OptionalKeys)
if len(s.Data) < minExpectedKeys || maxExpectedKeys < len(s.Data) {
return fmt.Errorf("expected between %d and %d keys in %s, got %d", minExpectedKeys, maxExpectedKeys, e.Name, len(s.Data))
}
for _, k := range e.Keys {
if _, exists := s.Data[k]; !exists {
Expand Down

0 comments on commit 0375808

Please sign in to comment.