Skip to content

Commit 715be15

Browse files
kubernetes-prow[bot]kubernetes-prow[bot]
authored
Merge pull request #3143 from zuzqqa/split-pkg-utils
Move L4-specific error types and helpers to pkg/l4/utils
2 parents f3df263 + d98135f commit 715be15

21 files changed

Lines changed: 426 additions & 460 deletions

‎pkg/l4/address/manager.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,10 @@ import (
2323

2424
compute "google.golang.org/api/compute/v1"
2525
"k8s.io/cloud-provider-gcp/providers/gce"
26-
"k8s.io/ingress-gce/pkg/utils"
2726

2827
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
28+
l4utils "k8s.io/ingress-gce/pkg/l4/utils"
29+
"k8s.io/ingress-gce/pkg/utils"
2930
"k8s.io/klog/v2"
3031
)
3132

@@ -240,7 +241,7 @@ func (m *Manager) ensureAddressReservation() (string, IPAddressType, error) {
240241

241242
m.frLogger.V(2).Info("Unable to reserve address (could be expected). Checking for conflict", "err", reserveErr)
242243

243-
if utils.IsNetworkTierMismatchGCEError(reserveErr) {
244+
if l4utils.IsNetworkTierMismatchGCEError(reserveErr) {
244245
receivedNetworkTier := cloud.NetworkTierPremium
245246
if receivedNetworkTier == m.networkTier {
246247
// We don't have information of current ephemeral IP address Network Tier since
@@ -249,7 +250,7 @@ func (m *Manager) ensureAddressReservation() (string, IPAddressType, error) {
249250
receivedNetworkTier = cloud.NetworkTierStandard
250251
}
251252
resource := fmt.Sprintf("Reserved static IP (%v)", m.name)
252-
networkTierError := utils.NewNetworkTierErr(resource, string(m.networkTier), string(receivedNetworkTier))
253+
networkTierError := l4utils.NewNetworkTierErr(resource, string(m.networkTier), string(receivedNetworkTier))
253254
return "", IPAddrUndefined, networkTierError
254255
}
255256

@@ -274,7 +275,7 @@ func (m *Manager) ensureAddressReservation() (string, IPAddressType, error) {
274275
addr, err := m.svc.GetRegionAddressByIP(m.region, m.targetIP)
275276
if err != nil {
276277
if utils.IsHTTPErrorCode(err, http.StatusNotFound) {
277-
return "", IPAddrUndefined, utils.NewIPConfigurationError(m.targetIP, "address not found. Check if the IP is valid and is reserved in your VPC.")
278+
return "", IPAddrUndefined, l4utils.NewIPConfigurationError(m.targetIP, "address not found. Check if the IP is valid and is reserved in your VPC.")
278279
}
279280
return "", IPAddrUndefined, fmt.Errorf("failed to get address by IP %q after reservation attempt, err: %q, reservation err: %q", m.targetIP, err, reserveErr)
280281
}
@@ -302,10 +303,10 @@ func (m *Manager) validateAddress(addr *compute.Address) error {
302303
return fmt.Errorf("IP mismatch, expected %q, actual: %q", m.targetIP, addr.Address)
303304
}
304305
if addr.AddressType != string(m.addressType) {
305-
return utils.NewIPConfigurationError(m.targetIP, fmt.Sprintf("address type mismatch, expected %q, actual: %q", m.addressType, addr.AddressType))
306+
return l4utils.NewIPConfigurationError(m.targetIP, fmt.Sprintf("address type mismatch, expected %q, actual: %q", m.addressType, addr.AddressType))
306307
}
307308
if addr.NetworkTier != m.networkTier.ToGCEValue() {
308-
return utils.NewNetworkTierErr(fmt.Sprintf("Static IP (%v)", m.name), m.networkTier.ToGCEValue(), addr.NetworkTier)
309+
return l4utils.NewNetworkTierErr(fmt.Sprintf("Static IP (%v)", m.name), m.networkTier.ToGCEValue(), addr.NetworkTier)
309310
}
310311
return nil
311312
}
@@ -342,7 +343,7 @@ func (m *Manager) TearDownAddressIPIfNetworkTierMismatch() error {
342343
}
343344
if addr != nil && addr.NetworkTier != m.networkTier.ToGCEValue() {
344345
if !m.isManagedAddress(addr) {
345-
return utils.NewNetworkTierErr(fmt.Sprintf("User specific address IP (%v)", m.name), string(m.networkTier), addr.NetworkTier)
346+
return l4utils.NewNetworkTierErr(fmt.Sprintf("User specific address IP (%v)", m.name), string(m.networkTier), addr.NetworkTier)
346347
}
347348
m.frLogger.V(3).Info("Deleting IP address because it has a wrong network tier", "ip", m.targetIP)
348349
if err := m.svc.DeleteRegionAddress(addr.Name, m.targetIP); err != nil {

‎pkg/l4/address/manager_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
compute "google.golang.org/api/compute/v1"
3434

3535
"k8s.io/cloud-provider-gcp/providers/gce"
36+
l4utils "k8s.io/ingress-gce/pkg/l4/utils"
3637
)
3738

3839
const (
@@ -195,7 +196,7 @@ func TestAddressManagerNonExisting(t *testing.T) {
195196
svc.Compute().(*cloud.MockGCE).MockAddresses.InsertHook = test.InsertAddressNotAllocatedToProjectErrorHook
196197
_, _, err = mgr.HoldAddress()
197198
require.Error(t, err)
198-
assert.True(t, utils.IsIPConfigurationError(err))
199+
assert.True(t, l4utils.IsIPConfigurationError(err))
199200
}
200201

201202
// TestAddressManagerWrongTypeReserved tests the case where the address was reserved by the user but it is of the wrong type.
@@ -214,7 +215,7 @@ func TestAddressManagerWrongTypeReserved(t *testing.T) {
214215

215216
_, _, err = mgr.HoldAddress()
216217
require.Error(t, err)
217-
assert.True(t, utils.IsIPConfigurationError(err))
218+
assert.True(t, l4utils.IsIPConfigurationError(err))
218219
}
219220

220221
// TestAddressManagerExternallyOwnedWrongNetworkTier tests the case where the address exists but isn't
@@ -230,8 +231,8 @@ func TestAddressManagerExternallyOwnedWrongNetworkTier(t *testing.T) {
230231
mgr := address.NewManager(svc, testSvcName, vals.Region, testSubnet, testLBName, "", targetIP, cloud.SchemeInternal, cloud.NetworkTierPremium, address.IPv4Version, klog.TODO())
231232
svc.Compute().(*cloud.MockGCE).MockAddresses.InsertHook = test.InsertAddressNetworkErrorHook
232233
_, _, err = mgr.HoldAddress()
233-
if err == nil || !utils.IsNetworkTierError(err) {
234-
t.Fatalf("mgr.HoldAddress() = %v, utils.IsNetworkTierError(err) = %t, want %t", err, utils.IsNetworkTierError(err), true)
234+
if err == nil || !l4utils.IsNetworkTierError(err) {
235+
t.Fatalf("mgr.HoldAddress() = %v, l4utils.IsNetworkTierError(err) = %t, want %t", err, l4utils.IsNetworkTierError(err), true)
235236
}
236237
}
237238

‎pkg/l4/controllers/l4controller.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"k8s.io/ingress-gce/pkg/l4/backends"
4242
"k8s.io/ingress-gce/pkg/l4/forwardingrules"
4343
"k8s.io/ingress-gce/pkg/l4/metrics"
44+
l4utils "k8s.io/ingress-gce/pkg/l4/utils"
4445
activecontrollermetrics "k8s.io/ingress-gce/pkg/metrics/activecontroller"
4546
negmetrics "k8s.io/ingress-gce/pkg/neg/metrics"
4647
negtypes "k8s.io/ingress-gce/pkg/neg/types"
@@ -707,7 +708,7 @@ func (l4c *L4Controller) publishMetrics(result *resources.L4ILBSyncResult, names
707708
isWeightedLB := result.MetricsState.WeightedLBPodsPerNode
708709
isZonalAffinityLB := result.MetricsState.ZonalAffinity
709710
isLoggingControlEnabled := result.MetricsState.LoggingControlEnabled
710-
metrics.PublishILBSyncMetrics(result.Error == nil, result.SyncType, result.GCEResourceInError, utils.GetErrorType(result.Error), result.StartTime, isResync, isWeightedLB, result.MetricsState.Protocol, isZonalAffinityLB, isLoggingControlEnabled)
711+
metrics.PublishILBSyncMetrics(result.Error == nil, result.SyncType, result.GCEResourceInError, l4utils.GetErrorType(result.Error), result.StartTime, isResync, isWeightedLB, result.MetricsState.Protocol, isZonalAffinityLB, isLoggingControlEnabled)
711712
if l4c.enableDualStack {
712713
svcLogger.V(2).Info("Internal L4 DualStack Loadbalancer for Service ensured, updating its state in metrics cache", "serviceState", result.MetricsState)
713714
metrics.PublishL4ILBDualStackSyncLatency(result.Error == nil, result.SyncType, result.MetricsState.IPFamilies, result.StartTime, isResync)
@@ -727,7 +728,7 @@ func (l4c *L4Controller) publishMetrics(result *resources.L4ILBSyncResult, names
727728
isWeightedLB := result.MetricsState.WeightedLBPodsPerNode
728729
isZonalAffinityLB := result.MetricsState.ZonalAffinity
729730
isLoggingControlEnabled := result.MetricsState.LoggingControlEnabled
730-
metrics.PublishILBSyncMetrics(result.Error == nil, result.SyncType, result.GCEResourceInError, utils.GetErrorType(result.Error), result.StartTime, false, isWeightedLB, result.MetricsState.Protocol, isZonalAffinityLB, isLoggingControlEnabled)
731+
metrics.PublishILBSyncMetrics(result.Error == nil, result.SyncType, result.GCEResourceInError, l4utils.GetErrorType(result.Error), result.StartTime, false, isWeightedLB, result.MetricsState.Protocol, isZonalAffinityLB, isLoggingControlEnabled)
731732
if l4c.enableDualStack {
732733
metrics.PublishL4ILBDualStackSyncLatency(result.Error == nil, result.SyncType, result.MetricsState.IPFamilies, result.StartTime, false)
733734
}

‎pkg/l4/controllers/l4netlbcontroller.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242
"k8s.io/ingress-gce/pkg/l4/backends"
4343
"k8s.io/ingress-gce/pkg/l4/forwardingrules"
4444
"k8s.io/ingress-gce/pkg/l4/metrics"
45+
l4utils "k8s.io/ingress-gce/pkg/l4/utils"
4546
activecontrollermetrics "k8s.io/ingress-gce/pkg/metrics/activecontroller"
4647
negmetrics "k8s.io/ingress-gce/pkg/neg/metrics"
4748
negtypes "k8s.io/ingress-gce/pkg/neg/types"
@@ -1032,5 +1033,5 @@ func (lc *L4NetLBController) publishSyncMetrics(result *resources.L4NetLBSyncRes
10321033
backendType := result.MetricsState.BackendType
10331034
isLoggingControlEnabled := result.MetricsState.LoggingControlEnabled
10341035

1035-
metrics.PublishNetLBSyncMetrics(result.Error == nil, result.SyncType, result.GCEResourceInError, utils.GetErrorType(result.Error), result.StartTime, isResync, isWeightedLB, result.MetricsState.Protocol, backendType, isLoggingControlEnabled)
1036+
metrics.PublishNetLBSyncMetrics(result.Error == nil, result.SyncType, result.GCEResourceInError, l4utils.GetErrorType(result.Error), result.StartTime, isResync, isWeightedLB, result.MetricsState.Protocol, backendType, isLoggingControlEnabled)
10361037
}

‎pkg/l4/controllers/l4netlbcontroller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1660,7 +1660,7 @@ func TestControllerUserIPWithStandardNetworkTier(t *testing.T) {
16601660
key, _ := common.KeyFunc(svc)
16611661
addUsersStaticAddress(lc, cloud.NetworkTierStandard)
16621662
// Sync should return error that Network Tier mismatch because we cannot tear User Managed Address.
1663-
if err := lc.sync(key, klog.TODO()); !utils.IsNetworkTierError(err) {
1663+
if err := lc.sync(key, klog.TODO()); !l4utils.IsNetworkTierError(err) {
16641664
t.Errorf("Expected error when trying to ensure service with wrong Network Tier, err: %v", err)
16651665
}
16661666
svc.Annotations[annotations.NetworkTierAnnotationKey] = string(cloud.NetworkTierStandard)

‎pkg/l4/forwardingrules/netlb_mixed_manager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ func (m *MixedManagerNetLB) ensure(existing *composite.ForwardingRule, backendSe
193193
// Can't update
194194
if networkMismatch := existing.NetworkTier != wanted.NetworkTier; networkMismatch {
195195
resource := fmt.Sprintf("Forwarding rule (%v)", name)
196-
networkTierMismatchErr := utils.NewNetworkTierErr(resource, wanted.NetworkTier, wanted.NetworkTier)
196+
networkTierMismatchErr := l4utils.NewNetworkTierErr(resource, wanted.NetworkTier, wanted.NetworkTier)
197197
return nil, l4utils.ResourceUpdate, networkTierMismatchErr
198198
}
199199

‎pkg/l4/resources/forwarding_rules.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,10 @@ func (l4 *L4) createFwdRule(newFr *composite.ForwardingRule, frLogger klog.Logge
166166
frLogger.V(2).Info("ensureIPv4ForwardingRule: Creating/Recreating forwarding rule")
167167
if err := l4.forwardingRules.Create(newFr); err != nil {
168168
if isAddressAlreadyInUseError(err) {
169-
return utils.NewIPConfigurationError(newFr.IPAddress, err.Error())
169+
return l4utils.NewIPConfigurationError(newFr.IPAddress, err.Error())
170170
}
171171
if isConflictingPortsError(err) {
172-
return utils.NewConflictingPortsConfigurationError(newFr.PortRange, err.Error())
172+
return l4utils.NewConflictingPortsConfigurationError(newFr.PortRange, err.Error())
173173
}
174174
return err
175175
}
@@ -255,7 +255,7 @@ func (l4netlb *L4NetLB) ensureIPv4ForwardingRule(bsLink string) (*composite.Forw
255255
if existingFwdRule != nil {
256256
if existingFwdRule.NetworkTier != newFwdRule.NetworkTier {
257257
resource := fmt.Sprintf("Forwarding rule (%v)", frName)
258-
networkTierMismatchError := utils.NewNetworkTierErr(resource, existingFwdRule.NetworkTier, newFwdRule.NetworkTier)
258+
networkTierMismatchError := l4utils.NewNetworkTierErr(resource, existingFwdRule.NetworkTier, newFwdRule.NetworkTier)
259259
return nil, address.IPAddrUndefined, l4utils.ResourceUpdate, networkTierMismatchError
260260
}
261261
equal, err := forwardingrules.EqualIPv4(existingFwdRule, newFwdRule)
@@ -316,10 +316,10 @@ func (l4netlb *L4NetLB) createFwdRule(newFr *composite.ForwardingRule, frLogger
316316
frLogger.V(2).Info("ensureIPv4ForwardingRule: Creating/Recreating forwarding rule")
317317
if err := l4netlb.forwardingRules.Create(newFr); err != nil {
318318
if isAddressAlreadyInUseError(err) {
319-
return utils.NewIPConfigurationError(newFr.IPAddress, addressAlreadyInUseMessageExternal)
319+
return l4utils.NewIPConfigurationError(newFr.IPAddress, addressAlreadyInUseMessageExternal)
320320
}
321321
if isConflictingPortsError(err) {
322-
return utils.NewConflictingPortsConfigurationError(newFr.PortRange, err.Error())
322+
return l4utils.NewConflictingPortsConfigurationError(newFr.PortRange, err.Error())
323323
}
324324
return err
325325
}

‎pkg/l4/resources/forwarding_rules_ipv6.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func (l4netlb *L4NetLB) ensureIPv6ForwardingRule(bsLink string) (*composite.Forw
207207
// IPv6 address is not supported for External Regional Network Load Balancing with Standard network tier.
208208
if netTier == cloud.NetworkTierStandard {
209209
resourceErr := "IPv6 External Load Balancer"
210-
return nil, l4utils.ResourceResync, utils.NewUnsupportedNetworkTierErr(resourceErr, string(cloud.NetworkTierStandard))
210+
return nil, l4utils.ResourceResync, l4utils.NewUnsupportedNetworkTierErr(resourceErr, string(cloud.NetworkTierStandard))
211211
}
212212

213213
// Only for IPv6, address reservation is not supported on Standard Tier
@@ -263,7 +263,7 @@ func (l4netlb *L4NetLB) ensureIPv6ForwardingRule(bsLink string) (*composite.Forw
263263
if existingIPv6FwdRule != nil {
264264
if existingIPv6FwdRule.NetworkTier != expectedIPv6FwdRule.NetworkTier {
265265
resource := fmt.Sprintf("Forwarding rule (%v)", existingIPv6FwdRule.Name)
266-
networkTierMismatchError := utils.NewNetworkTierErr(resource, existingIPv6FwdRule.NetworkTier, expectedIPv6FwdRule.NetworkTier)
266+
networkTierMismatchError := l4utils.NewNetworkTierErr(resource, existingIPv6FwdRule.NetworkTier, expectedIPv6FwdRule.NetworkTier)
267267
return nil, l4utils.ResourceResync, networkTierMismatchError
268268
}
269269

‎pkg/l4/resources/forwarding_rules_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@ func TestL4CreateExternalForwardingRuleUserErrors(t *testing.T) {
3939
{
4040
desc: "Address already in-use",
4141
insertError: &googleapi.Error{Code: http.StatusBadRequest, Message: "Invalid value for field 'resource.IPAddress': '1.1.1.1'. Specified IP address is in-use and would result in a conflict., invalid"},
42-
expectedUserErrorFunc: utils.IsIPConfigurationError,
42+
expectedUserErrorFunc: l4utils.IsIPConfigurationError,
4343
},
4444
{
4545
desc: "Conflicting port range",
4646
insertError: &googleapi.Error{Code: http.StatusBadRequest, Message: "Invalid resource usage: 'Forwarding rule's port range is conflicting with forwarding rule: abcd'."},
47-
expectedUserErrorFunc: utils.IsConflictingPortsConfigurationError,
47+
expectedUserErrorFunc: l4utils.IsConflictingPortsConfigurationError,
4848
},
4949
}
5050
for _, tc := range testCases {
@@ -365,12 +365,12 @@ func TestL4EnsureIPv4ForwardingRuleAddressAlreadyInUse(t *testing.T) {
365365
{
366366
desc: "Address already in-use",
367367
insertError: &googleapi.Error{Code: http.StatusConflict, Message: "IP_IN_USE_BY_ANOTHER_RESOURCE - IP '10.107.116.14' is already being used by another resource."},
368-
expectedUserErrorFunc: utils.IsIPConfigurationError,
368+
expectedUserErrorFunc: l4utils.IsIPConfigurationError,
369369
},
370370
{
371371
desc: "Conflicting port range",
372372
insertError: &googleapi.Error{Code: http.StatusBadRequest, Message: "Invalid resource usage: 'Forwarding rule's port range is conflicting with forwarding rule: abcd'."},
373-
expectedUserErrorFunc: utils.IsConflictingPortsConfigurationError,
373+
expectedUserErrorFunc: l4utils.IsConflictingPortsConfigurationError,
374374
},
375375
}
376376

‎pkg/l4/resources/l4.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -585,10 +585,10 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service
585585
result.Conditions = append(result.Conditions, l4lbconfig.NewConditionLoggingError(err))
586586
}
587587

588-
if utils.IsUnsupportedFeatureError(err, string(backends.LocalityLbPolicyRendezvous)) {
588+
if l4utils.IsUnsupportedFeatureError(err, string(backends.LocalityLbPolicyRendezvous)) {
589589
result.GCEResourceInError = annotations.BackendServiceResource
590590
l4.recorder.Eventf(l4.Service, corev1.EventTypeWarning, "AllowlistingRequired", WeightedLBPodsPerNodeAllowlistMessage)
591-
result.Error = utils.NewUserError(err)
591+
result.Error = l4utils.NewUserError(err)
592592
} else {
593593
result.GCEResourceInError = annotations.BackendServiceResource
594594
result.Error = err
@@ -799,7 +799,7 @@ func (l4 *L4) getSubnetworkURLByName(subnetName string) (string, error) {
799799
subnetwork, err := l4.cloud.GetSubnetwork(l4.cloud.Region(), subnetName)
800800
if err != nil {
801801
if utils.IsNotFoundError(err) {
802-
return "", utils.NewInvalidSubnetConfigurationError(l4.cloud.ProjectID(), subnetName)
802+
return "", l4utils.NewInvalidSubnetConfigurationError(l4.cloud.ProjectID(), subnetName)
803803
}
804804
return "", err
805805
}

0 commit comments

Comments
 (0)