Skip to content

Commit bdc54eb

Browse files
committed
Use compute v1 api to specify network tier
Drop the use of the alpha api for operations that are supported by compute v1 The switch/case logic was wrong because the user can set the default tier for a project: https://cloud.google.com/network-tiers/docs/using-network-service-tiers#setting_the_tier_for_all_resources_in_a_project The assumption that the default tier is always PREMIUM is wrong This patch uses the explicit network tier when possible, or it falls back to the project default. Signed-off-by: Saverio Proto <[email protected]>
1 parent 4e79344 commit bdc54eb

File tree

2 files changed

+38
-64
lines changed

2 files changed

+38
-64
lines changed

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external.go

Lines changed: 18 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ import (
3333
servicehelpers "k8s.io/cloud-provider/service/helpers"
3434
utilnet "k8s.io/utils/net"
3535

36-
computealpha "google.golang.org/api/compute/v0.alpha"
3736
compute "google.golang.org/api/compute/v1"
3837
"k8s.io/klog"
3938
)
@@ -932,30 +931,18 @@ func createForwardingRule(s CloudForwardingRuleService, name, serviceName, regio
932931
desc := makeServiceDescription(serviceName)
933932
ipProtocol := string(ports[0].Protocol)
934933

935-
switch netTier {
936-
case cloud.NetworkTierPremium:
937-
rule := &compute.ForwardingRule{
938-
Name: name,
939-
Description: desc,
940-
IPAddress: ipAddress,
941-
IPProtocol: ipProtocol,
942-
PortRange: portRange,
943-
Target: target,
944-
}
945-
err = s.CreateRegionForwardingRule(rule, region)
946-
default:
947-
rule := &computealpha.ForwardingRule{
948-
Name: name,
949-
Description: desc,
950-
IPAddress: ipAddress,
951-
IPProtocol: ipProtocol,
952-
PortRange: portRange,
953-
Target: target,
954-
NetworkTier: netTier.ToGCEValue(),
955-
}
956-
err = s.CreateAlphaRegionForwardingRule(rule, region)
934+
rule := &compute.ForwardingRule{
935+
Name: name,
936+
Description: desc,
937+
IPAddress: ipAddress,
938+
IPProtocol: ipProtocol,
939+
PortRange: portRange,
940+
Target: target,
941+
NetworkTier: netTier.ToGCEValue(),
957942
}
958943

944+
err = s.CreateRegionForwardingRule(rule, region)
945+
959946
if err != nil && !isHTTPErrorCode(err, http.StatusConflict) {
960947
return err
961948
}
@@ -1045,27 +1032,15 @@ func ensureStaticIP(s CloudAddressService, name, serviceName, region, existingIP
10451032
desc := makeServiceDescription(serviceName)
10461033

10471034
var creationErr error
1048-
switch netTier {
1049-
case cloud.NetworkTierPremium:
1050-
addressObj := &compute.Address{
1051-
Name: name,
1052-
Description: desc,
1053-
}
1054-
if existingIP != "" {
1055-
addressObj.Address = existingIP
1056-
}
1057-
creationErr = s.ReserveRegionAddress(addressObj, region)
1058-
default:
1059-
addressObj := &computealpha.Address{
1060-
Name: name,
1061-
Description: desc,
1062-
NetworkTier: netTier.ToGCEValue(),
1063-
}
1064-
if existingIP != "" {
1065-
addressObj.Address = existingIP
1066-
}
1067-
creationErr = s.ReserveAlphaRegionAddress(addressObj, region)
1035+
addressObj := &compute.Address{
1036+
Name: name,
1037+
Description: desc,
1038+
NetworkTier: netTier.ToGCEValue(),
1039+
}
1040+
if existingIP != "" {
1041+
addressObj.Address = existingIP
10681042
}
1043+
creationErr = s.ReserveRegionAddress(addressObj, region)
10691044

10701045
if creationErr != nil {
10711046
// GCE returns StatusConflict if the name conflicts; it returns

staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_external_test.go

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626

2727
"github.com/stretchr/testify/assert"
2828
"github.com/stretchr/testify/require"
29-
computealpha "google.golang.org/api/compute/v0.alpha"
3029
compute "google.golang.org/api/compute/v1"
3130

3231
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
@@ -103,9 +102,9 @@ func TestEnsureStaticIPWithTier(t *testing.T) {
103102
assert.NotEqual(t, ip, "")
104103
// Get the Address from the fake address service and verify that the tier
105104
// is set correctly.
106-
alphaAddr, err := s.GetAlphaRegionAddress(tc.name, s.region)
105+
Addr, err := s.GetRegionAddress(tc.name, s.region)
107106
require.NoError(t, err)
108-
assert.Equal(t, tc.expected, alphaAddr.NetworkTier)
107+
assert.Equal(t, tc.expected, Addr.NetworkTier)
109108
})
110109
}
111110
}
@@ -119,14 +118,14 @@ func TestVerifyRequestedIP(t *testing.T) {
119118
requestedIP string
120119
fwdRuleIP string
121120
netTier cloud.NetworkTier
122-
addrList []*computealpha.Address
121+
addrList []*compute.Address
123122
expectErr bool
124123
expectUserOwned bool
125124
}{
126125
"requested IP exists": {
127126
requestedIP: "1.1.1.1",
128127
netTier: cloud.NetworkTierPremium,
129-
addrList: []*computealpha.Address{{Name: "foo", Address: "1.1.1.1", NetworkTier: "PREMIUM"}},
128+
addrList: []*compute.Address{{Name: "foo", Address: "1.1.1.1", NetworkTier: "PREMIUM"}},
130129
expectErr: false,
131130
expectUserOwned: true,
132131
},
@@ -149,7 +148,7 @@ func TestVerifyRequestedIP(t *testing.T) {
149148
"requested IP exists, but network tier does not match": {
150149
requestedIP: "1.1.1.1",
151150
netTier: cloud.NetworkTierStandard,
152-
addrList: []*computealpha.Address{{Name: "foo", Address: "1.1.1.1", NetworkTier: "PREMIUM"}},
151+
addrList: []*compute.Address{{Name: "foo", Address: "1.1.1.1", NetworkTier: "PREMIUM"}},
153152
expectErr: true,
154153
},
155154
} {
@@ -158,7 +157,7 @@ func TestVerifyRequestedIP(t *testing.T) {
158157
require.NoError(t, err)
159158

160159
for _, addr := range tc.addrList {
161-
s.ReserveAlphaRegionAddress(addr, s.region)
160+
s.ReserveRegionAddress(addr, s.region)
162161
}
163162
isUserOwnedIP, err := verifyUserRequestedIP(s, s.region, tc.requestedIP, tc.fwdRuleIP, lbRef, tc.netTier)
164163
assert.Equal(t, tc.expectErr, err != nil, fmt.Sprintf("err: %v", err))
@@ -180,11 +179,11 @@ func TestCreateForwardingRuleWithTier(t *testing.T) {
180179

181180
for desc, tc := range map[string]struct {
182181
netTier cloud.NetworkTier
183-
expectedRule *computealpha.ForwardingRule
182+
expectedRule *compute.ForwardingRule
184183
}{
185184
"Premium tier": {
186185
netTier: cloud.NetworkTierPremium,
187-
expectedRule: &computealpha.ForwardingRule{
186+
expectedRule: &compute.ForwardingRule{
188187
Name: "lb-1",
189188
Description: `{"kubernetes.io/service-name":"foo-svc"}`,
190189
IPAddress: "1.1.1.1",
@@ -197,15 +196,15 @@ func TestCreateForwardingRuleWithTier(t *testing.T) {
197196
},
198197
"Standard tier": {
199198
netTier: cloud.NetworkTierStandard,
200-
expectedRule: &computealpha.ForwardingRule{
199+
expectedRule: &compute.ForwardingRule{
201200
Name: "lb-2",
202201
Description: `{"kubernetes.io/service-name":"foo-svc"}`,
203202
IPAddress: "2.2.2.2",
204203
IPProtocol: "TCP",
205204
PortRange: "123-123",
206205
Target: target,
207206
NetworkTier: "STANDARD",
208-
SelfLink: fmt.Sprintf(baseLinkURL, "alpha", vals.ProjectID, vals.Region, "lb-2"),
207+
SelfLink: fmt.Sprintf(baseLinkURL, "v1", vals.ProjectID, vals.Region, "lb-2"),
209208
},
210209
},
211210
} {
@@ -219,9 +218,9 @@ func TestCreateForwardingRuleWithTier(t *testing.T) {
219218
err = createForwardingRule(s, lbName, serviceName, s.region, ipAddr, target, ports, tc.netTier)
220219
assert.NoError(t, err)
221220

222-
alphaRule, err := s.GetAlphaRegionForwardingRule(lbName, s.region)
221+
Rule, err := s.GetRegionForwardingRule(lbName, s.region)
223222
assert.NoError(t, err)
224-
assert.Equal(t, tc.expectedRule, alphaRule)
223+
assert.Equal(t, tc.expectedRule, Rule)
225224
})
226225
}
227226
}
@@ -240,35 +239,35 @@ func TestDeleteAddressWithWrongTier(t *testing.T) {
240239
for desc, tc := range map[string]struct {
241240
addrName string
242241
netTier cloud.NetworkTier
243-
addrList []*computealpha.Address
242+
addrList []*compute.Address
244243
expectDelete bool
245244
}{
246245
"Network tiers (premium) match; do nothing": {
247246
addrName: "foo1",
248247
netTier: cloud.NetworkTierPremium,
249-
addrList: []*computealpha.Address{{Name: "foo1", Address: "1.1.1.1", NetworkTier: "PREMIUM"}},
248+
addrList: []*compute.Address{{Name: "foo1", Address: "1.1.1.1", NetworkTier: "PREMIUM"}},
250249
},
251250
"Network tiers (standard) match; do nothing": {
252251
addrName: "foo2",
253252
netTier: cloud.NetworkTierStandard,
254-
addrList: []*computealpha.Address{{Name: "foo2", Address: "1.1.1.2", NetworkTier: "STANDARD"}},
253+
addrList: []*compute.Address{{Name: "foo2", Address: "1.1.1.2", NetworkTier: "STANDARD"}},
255254
},
256255
"Wrong network tier (standard); delete address": {
257256
addrName: "foo3",
258257
netTier: cloud.NetworkTierPremium,
259-
addrList: []*computealpha.Address{{Name: "foo3", Address: "1.1.1.3", NetworkTier: "STANDARD"}},
258+
addrList: []*compute.Address{{Name: "foo3", Address: "1.1.1.3", NetworkTier: "STANDARD"}},
260259
expectDelete: true,
261260
},
262261
"Wrong network tier (premium); delete address": {
263262
addrName: "foo4",
264263
netTier: cloud.NetworkTierStandard,
265-
addrList: []*computealpha.Address{{Name: "foo4", Address: "1.1.1.4", NetworkTier: "PREMIUM"}},
264+
addrList: []*compute.Address{{Name: "foo4", Address: "1.1.1.4", NetworkTier: "PREMIUM"}},
266265
expectDelete: true,
267266
},
268267
} {
269268
t.Run(desc, func(t *testing.T) {
270269
for _, addr := range tc.addrList {
271-
s.ReserveAlphaRegionAddress(addr, s.region)
270+
s.ReserveRegionAddress(addr, s.region)
272271
}
273272

274273
// Sanity check to ensure we inject the right address.
@@ -428,13 +427,13 @@ func TestLoadBalancerWrongTierResourceDeletion(t *testing.T) {
428427
)
429428
require.NoError(t, err)
430429

431-
addressObj := &computealpha.Address{
430+
addressObj := &compute.Address{
432431
Name: lbName,
433432
Description: serviceName.String(),
434433
NetworkTier: cloud.NetworkTierStandard.ToGCEValue(),
435434
}
436435

437-
err = gce.ReserveAlphaRegionAddress(addressObj, gce.region)
436+
err = gce.ReserveRegionAddress(addressObj, gce.region)
438437
require.NoError(t, err)
439438

440439
_, err = createExternalLoadBalancer(gce, svc, []string{"test-node-1"}, vals.ClusterName, vals.ClusterID, vals.ZoneName)

0 commit comments

Comments
 (0)