Skip to content

Commit ae525f5

Browse files
tzifudzitzifudzi
committed
Various code fixes for PR feedback
aws#443
1 parent 1dd0d2e commit ae525f5

File tree

7 files changed

+49
-71
lines changed

7 files changed

+49
-71
lines changed

‎pkg/config/loader.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,11 @@ func ParseWinIPTargetConfigs(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (wa
126126

127127
// If warm IP target config value is not found, or there is an error parsing it, the value will be set to zero
128128
if foundWarmIP {
129-
warmIPTargetInt, err := strconv.Atoi(warmIPTargetStr)
130129
if err != nil {
131130
log.Info("Could not parse warm ip target, defaulting to zero", "warm ip target", warmIPTargetStr)
132131
warmIPTarget = 0
133132
} else {
134-
warmIPTarget = warmIPTargetInt
133+
warmIPTarget, err = strconv.Atoi(warmIPTargetStr)
135134

136135
// Handle secondary IP mode scenario where WarmIPTarget is explicitly configured to zero
137136
// In such a case there must always be 1 warm IP to ensure that the warmpool is never empty
@@ -147,12 +146,11 @@ func ParseWinIPTargetConfigs(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (wa
147146

148147
// If min IP target config value is not found, or there is an error parsing it, the value will be set to zero
149148
if foundMinIP {
150-
minIPTargetInt, err := strconv.Atoi(minIPTargetStr)
151149
if err != nil {
152150
log.Info("Could not parse minimum ip target, defaulting to zero", "minimum ip target", minIPTargetStr)
153151
minIPTarget = 0
154152
} else {
155-
minIPTarget = minIPTargetInt
153+
minIPTarget, err = strconv.Atoi(minIPTargetStr)
156154
}
157155
} else {
158156
log.V(1).Info("could not find minimum ip target in ConfigMap, defaulting to zero")
@@ -161,11 +159,10 @@ func ParseWinIPTargetConfigs(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (wa
161159

162160
warmPrefixTarget = 0
163161
if isPDEnabled && foundWarmPrefix {
164-
warmPrefixTargetInt, err := strconv.Atoi(warmPrefixTargetStr)
165162
if err != nil {
166163
log.Info("Could not parse warm prefix target, defaulting to zero", "warm prefix target", warmPrefixTargetStr)
167164
} else {
168-
warmPrefixTarget = warmPrefixTargetInt
165+
warmPrefixTarget, err = strconv.Atoi(warmPrefixTargetStr)
169166
}
170167
}
171168

@@ -178,7 +175,7 @@ func ParseWinIPTargetConfigs(log logr.Logger, vpcCniConfigMap *v1.ConfigMap) (wa
178175
minIPTarget = IPv4DefaultWinMinIPTarget
179176
warmIPTarget = IPv4DefaultWinWarmIPTarget
180177
}
181-
log.V(1).Info(
178+
log.Info(
182179
"Encountered zero values for warm-ip-target, min-ip-target and warm-prefix-target in ConfigMap data, falling back to using default values since on demand IP allocation is not supported",
183180
"minIPTarget", minIPTarget,
184181
"warmIPTarget", warmIPTarget,

‎pkg/pool/pool.go‎

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -424,21 +424,6 @@ func (p *pool) ReconcilePool() *worker.WarmPoolJob {
424424
len(p.usedResources), "pending create", p.pendingCreate, "pending delete", p.pendingDelete,
425425
"cool down queue", len(p.coolDownQueue), "total resources", totalCreatedResources, "capacity", p.capacity)
426426

427-
p.log.V(1).Info(
428-
"Reconciling pool",
429-
"isPDPool", p.isPDPool,
430-
"reSyncRequired", p.reSyncRequired,
431-
"minIPTarget", p.warmPoolConfig.MinIPTarget,
432-
"warmIPTarget", p.warmPoolConfig.WarmIPTarget,
433-
"numWarmResources", numWarmResources,
434-
"used resouces", len(p.usedResources),
435-
"cool down queue", len(p.coolDownQueue),
436-
"total resources", totalCreatedResources,
437-
"pendingCreate", p.pendingCreate,
438-
"pendingDelete", p.pendingDelete,
439-
"capacity", p.capacity,
440-
)
441-
442427
if p.reSyncRequired {
443428
// If Pending operations are present then we can't re-sync as the upstream
444429
// and pool could change during re-sync
@@ -745,7 +730,6 @@ func (p *pool) calculateSecondaryIPDeviation() int {
745730

746731
// warm pool is in draining state, set targets to zero
747732
if p.warmPoolConfig.DesiredSize == 0 {
748-
p.log.V(1).Info("DesiredSize is zero, warmPool is in draining state")
749733
p.warmPoolConfig.WarmIPTarget = 0
750734
p.warmPoolConfig.MinIPTarget = 0
751735
p.warmPoolConfig.WarmPrefixTarget = 0
@@ -755,22 +739,14 @@ func (p *pool) calculateSecondaryIPDeviation() int {
755739
isWarmIPTargetInvalid := p.warmPoolConfig.WarmIPTarget < 0
756740
// Handle scenario where MinIPTarget is configured to negative integer which is invalid
757741
if isMinIPTargetInvalid {
758-
p.log.V(1).Info(
759-
"MinIPTarget value is invalid negative integer, setting MinIPTarget to default",
760-
"IPv4DefaultWinMinIPTarget", config.IPv4DefaultWinMinIPTarget,
761-
)
762742
p.warmPoolConfig.MinIPTarget = config.IPv4DefaultWinMinIPTarget
763743
}
764744
// Handle scenario where WarmIPTarget is configured to negative integer which is invalid
765745
if isWarmIPTargetInvalid {
766-
p.log.V(1).Info(
767-
"WarmIPTarget value is invalid negative integer, setting warmIPTarget to default",
768-
"IPv4DefaultWinWarmIPTarget", config.IPv4DefaultWinWarmIPTarget,
769-
)
770746
p.warmPoolConfig.WarmIPTarget = config.IPv4DefaultWinWarmIPTarget
771747
}
772748

773-
availableResources := numWarmResources + p.pendingCreate - p.pendingDelete
749+
availableResources := numWarmResources + p.pendingCreate
774750

775751
// Calculate how many IPs we're short of the warm target
776752
resourcesShort := max(p.warmPoolConfig.WarmIPTarget-availableResources, 0)
@@ -787,19 +763,6 @@ func (p *pool) calculateSecondaryIPDeviation() int {
787763
// The final deviation is the difference between short and over
788764
deviation := resourcesShort - resourcesOver
789765

790-
p.log.V(1).Info(
791-
"Finished calculating IP deviation for secondary IP pool",
792-
"minIPTarget", p.warmPoolConfig.MinIPTarget,
793-
"warmIPTarget", p.warmPoolConfig.WarmIPTarget,
794-
"numWarmResources", numWarmResources,
795-
"numUsedResources", numUsedResources,
796-
"numAssigned", numAssignedResources,
797-
"availableResources", availableResources,
798-
"resourcesShort", resourcesShort,
799-
"resourcesOver", resourcesOver,
800-
"deviationResult", deviation,
801-
)
802-
803766
return deviation
804767
}
805768

‎pkg/pool/utils.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License"). You may
4+
// not use this file except in compliance with the License. A copy of the
5+
// License is located at
6+
//
7+
// http://aws.amazon.com/apache2.0/
8+
//
9+
// or in the "license" file accompanying this file. This file is distributed
10+
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
11+
// express or implied. See the License for the specific language governing
12+
// permissions and limitations under the License.
13+
14+
package pool
15+
16+
import (
17+
"github.com/go-logr/logr"
18+
19+
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/api"
20+
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
21+
)
22+
23+
// GetWinWarmPoolConfig retrieves Windows warmpool configuration from ConfigMap, falls back to using default values on failure
24+
func GetWinWarmPoolConfig(log logr.Logger, w api.Wrapper, isPDEnabled bool) *config.WarmPoolConfig {
25+
var resourceConfig map[string]config.ResourceConfig
26+
vpcCniConfigMap, err := w.K8sAPI.GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace)
27+
if err == nil {
28+
resourceConfig = config.LoadResourceConfigFromConfigMap(log, vpcCniConfigMap)
29+
} else {
30+
log.Error(err, "failed to read from config map, will use default resource config")
31+
resourceConfig = config.LoadResourceConfig()
32+
}
33+
34+
if isPDEnabled {
35+
return resourceConfig[config.ResourceNameIPAddressFromPrefix].WarmPoolConfig
36+
} else {
37+
return resourceConfig[config.ResourceNameIPAddress].WarmPoolConfig
38+
}
39+
}

‎pkg/provider/provider_test.go‎ renamed to ‎pkg/pool/utils_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
// express or implied. See the License for the specific language governing
1212
// permissions and limitations under the License.
1313

14-
package provider
14+
package pool
1515

1616
import (
1717
"fmt"

‎pkg/provider/ip/provider.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func (p *ipv4Provider) InitResource(instance ec2.EC2Instance) error {
154154
nodeCapacity := getCapacity(instance.Type(), instance.Os())
155155

156156
isPDEnabled := p.conditions.IsWindowsPrefixDelegationEnabled()
157-
p.config = provider.GetWinWarmPoolConfig(p.log, p.apiWrapper, isPDEnabled)
157+
p.config = pool.GetWinWarmPoolConfig(p.log, p.apiWrapper, isPDEnabled)
158158

159159
// Set warm pool config to empty config if PD is enabled
160160
secondaryIPWPConfig := p.config
@@ -241,7 +241,7 @@ func (p *ipv4Provider) UpdateResourceCapacity(instance ec2.EC2Instance) error {
241241

242242
resourceProviderAndPool.isPrevPDEnabled = false
243243

244-
p.config = provider.GetWinWarmPoolConfig(p.log, p.apiWrapper, isCurrPDEnabled && isNitroInstance)
244+
p.config = pool.GetWinWarmPoolConfig(p.log, p.apiWrapper, isCurrPDEnabled && isNitroInstance)
245245

246246
// Set the secondary IP provider pool state to active
247247
job := resourceProviderAndPool.resourcePool.SetToActive(p.config)

‎pkg/provider/prefix/provider.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func (p *ipv4PrefixProvider) InitResource(instance ec2.EC2Instance) error {
156156
nodeCapacity := getCapacity(instance.Type(), instance.Os()) * pool.NumIPv4AddrPerPrefix
157157

158158
isPDEnabled := p.conditions.IsWindowsPrefixDelegationEnabled()
159-
p.config = provider.GetWinWarmPoolConfig(p.log, p.apiWrapper, isPDEnabled)
159+
p.config = pool.GetWinWarmPoolConfig(p.log, p.apiWrapper, isPDEnabled)
160160

161161
// Set warm pool config to empty if PD is not enabled
162162
prefixIPWPConfig := p.config
@@ -234,7 +234,7 @@ func (p *ipv4PrefixProvider) UpdateResourceCapacity(instance ec2.EC2Instance) er
234234

235235
resourceProviderAndPool.isPrevPDEnabled = true
236236

237-
warmPoolConfig := provider.GetWinWarmPoolConfig(p.log, p.apiWrapper, isCurrPDEnabled)
237+
warmPoolConfig := pool.GetWinWarmPoolConfig(p.log, p.apiWrapper, isCurrPDEnabled)
238238

239239
// Set the secondary IP provider pool state to active
240240
job := resourceProviderAndPool.resourcePool.SetToActive(warmPoolConfig)

‎pkg/provider/provider.go‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,10 @@
1414
package provider
1515

1616
import (
17-
"github.com/go-logr/logr"
1817
ctrl "sigs.k8s.io/controller-runtime"
1918
"sigs.k8s.io/controller-runtime/pkg/healthz"
2019

21-
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/api"
2220
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/aws/ec2"
23-
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/config"
2421
"github.com/aws/amazon-vpc-resource-controller-k8s/pkg/pool"
2522
)
2623

@@ -50,21 +47,3 @@ type ResourceProvider interface {
5047
IntrospectSummary() interface{}
5148
ReconcileNode(nodeName string) bool
5249
}
53-
54-
// GetWinWarmPoolConfig retrieves Windows warmpool configuration from ConfigMap, falls back to using default values on failure
55-
func GetWinWarmPoolConfig(log logr.Logger, w api.Wrapper, isPDEnabled bool) *config.WarmPoolConfig {
56-
var resourceConfig map[string]config.ResourceConfig
57-
vpcCniConfigMap, err := w.K8sAPI.GetConfigMap(config.VpcCniConfigMapName, config.KubeSystemNamespace)
58-
if err == nil {
59-
resourceConfig = config.LoadResourceConfigFromConfigMap(log, vpcCniConfigMap)
60-
} else {
61-
log.Error(err, "failed to read from config map, will use default resource config")
62-
resourceConfig = config.LoadResourceConfig()
63-
}
64-
65-
if isPDEnabled {
66-
return resourceConfig[config.ResourceNameIPAddressFromPrefix].WarmPoolConfig
67-
} else {
68-
return resourceConfig[config.ResourceNameIPAddress].WarmPoolConfig
69-
}
70-
}

0 commit comments

Comments
 (0)