Skip to content

Commit b2514b1

Browse files
author
Cecile Robert-Michon
committed
🐛 don't update network interface if it already exists
1 parent 6f0baa6 commit b2514b1

File tree

2 files changed

+111
-59
lines changed

2 files changed

+111
-59
lines changed

‎cloud/services/networkinterfaces/networkinterfaces.go‎

Lines changed: 66 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package networkinterfaces
1818

1919
import (
2020
"context"
21-
2221
"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network"
2322
"github.com/Azure/go-autorest/autorest/to"
2423
"github.com/pkg/errors"
@@ -30,78 +29,87 @@ import (
3029
func (s *Service) Reconcile(ctx context.Context) error {
3130
for _, nicSpec := range s.Scope.NICSpecs() {
3231

33-
nicConfig := &network.InterfaceIPConfigurationPropertiesFormat{}
34-
35-
nicConfig.Subnet = &network.Subnet{ID: to.StringPtr(azure.SubnetID(s.Scope.SubscriptionID(), nicSpec.VNetResourceGroup, nicSpec.VNetName, nicSpec.SubnetName))}
36-
37-
nicConfig.PrivateIPAllocationMethod = network.Dynamic
38-
if nicSpec.StaticIPAddress != "" {
39-
nicConfig.PrivateIPAllocationMethod = network.Static
40-
nicConfig.PrivateIPAddress = to.StringPtr(nicSpec.StaticIPAddress)
41-
}
32+
_, err := s.Client.Get(ctx, s.Scope.ResourceGroup(), nicSpec.Name)
33+
switch {
34+
case err != nil && !azure.ResourceNotFound(err):
35+
return errors.Wrapf(err, "failed to fetch network interface %s", nicSpec.Name)
36+
case err == nil:
37+
// network interface already exists, do nothing
38+
continue
39+
default:
40+
nicConfig := &network.InterfaceIPConfigurationPropertiesFormat{}
41+
42+
nicConfig.Subnet = &network.Subnet{ID: to.StringPtr(azure.SubnetID(s.Scope.SubscriptionID(), nicSpec.VNetResourceGroup, nicSpec.VNetName, nicSpec.SubnetName))}
43+
44+
nicConfig.PrivateIPAllocationMethod = network.Dynamic
45+
if nicSpec.StaticIPAddress != "" {
46+
nicConfig.PrivateIPAllocationMethod = network.Static
47+
nicConfig.PrivateIPAddress = to.StringPtr(nicSpec.StaticIPAddress)
48+
}
4249

43-
backendAddressPools := []network.BackendAddressPool{}
44-
if nicSpec.PublicLBName != "" {
45-
if nicSpec.PublicLBAddressPoolName != "" {
50+
backendAddressPools := []network.BackendAddressPool{}
51+
if nicSpec.PublicLBName != "" {
52+
if nicSpec.PublicLBAddressPoolName != "" {
53+
backendAddressPools = append(backendAddressPools,
54+
network.BackendAddressPool{
55+
ID: to.StringPtr(azure.AddressPoolID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicSpec.PublicLBName, nicSpec.PublicLBAddressPoolName)),
56+
})
57+
}
58+
if nicSpec.PublicLBNATRuleName != "" {
59+
nicConfig.LoadBalancerInboundNatRules = &[]network.InboundNatRule{
60+
{
61+
ID: to.StringPtr(azure.NATRuleID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicSpec.PublicLBName, nicSpec.PublicLBNATRuleName)),
62+
},
63+
}
64+
}
65+
}
66+
if nicSpec.InternalLBName != "" && nicSpec.InternalLBAddressPoolName != "" {
67+
// only control planes have an attached internal LB
4668
backendAddressPools = append(backendAddressPools,
4769
network.BackendAddressPool{
48-
ID: to.StringPtr(azure.AddressPoolID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicSpec.PublicLBName, nicSpec.PublicLBAddressPoolName)),
70+
ID: to.StringPtr(azure.AddressPoolID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicSpec.InternalLBName, nicSpec.InternalLBAddressPoolName)),
4971
})
5072
}
51-
if nicSpec.PublicLBNATRuleName != "" {
52-
nicConfig.LoadBalancerInboundNatRules = &[]network.InboundNatRule{
53-
{
54-
ID: to.StringPtr(azure.NATRuleID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicSpec.PublicLBName, nicSpec.PublicLBNATRuleName)),
55-
},
73+
nicConfig.LoadBalancerBackendAddressPools = &backendAddressPools
74+
75+
if nicSpec.PublicIPName != "" {
76+
nicConfig.PublicIPAddress = &network.PublicIPAddress{
77+
ID: to.StringPtr(azure.PublicIPID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicSpec.PublicIPName)),
5678
}
5779
}
58-
}
59-
if nicSpec.InternalLBName != "" && nicSpec.InternalLBAddressPoolName != "" {
60-
// only control planes have an attached internal LB
61-
backendAddressPools = append(backendAddressPools,
62-
network.BackendAddressPool{
63-
ID: to.StringPtr(azure.AddressPoolID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicSpec.InternalLBName, nicSpec.InternalLBAddressPoolName)),
64-
})
65-
}
66-
nicConfig.LoadBalancerBackendAddressPools = &backendAddressPools
6780

68-
if nicSpec.PublicIPName != "" {
69-
nicConfig.PublicIPAddress = &network.PublicIPAddress{
70-
ID: to.StringPtr(azure.PublicIPID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), nicSpec.PublicIPName)),
71-
}
72-
}
81+
if nicSpec.AcceleratedNetworking == nil {
82+
// set accelerated networking to the capability of the VMSize
83+
sku, err := s.ResourceSKUCache.Get(ctx, nicSpec.VMSize, resourceskus.VirtualMachines)
84+
if err != nil {
85+
return errors.Wrapf(err, "failed to get find vm sku %s in compute api", nicSpec.VMSize)
86+
}
7387

74-
if nicSpec.AcceleratedNetworking == nil {
75-
// set accelerated networking to the capability of the VMSize
76-
sku, err := s.ResourceSKUCache.Get(ctx, nicSpec.VMSize, resourceskus.VirtualMachines)
77-
if err != nil {
78-
return errors.Wrapf(err, "failed to get find vm sku %s in compute api", nicSpec.VMSize)
88+
accelNet := sku.HasCapability(resourceskus.AcceleratedNetworking)
89+
nicSpec.AcceleratedNetworking = &accelNet
7990
}
8091

81-
accelNet := sku.HasCapability(resourceskus.AcceleratedNetworking)
82-
nicSpec.AcceleratedNetworking = &accelNet
83-
}
84-
85-
err := s.Client.CreateOrUpdate(ctx,
86-
s.Scope.ResourceGroup(),
87-
nicSpec.Name,
88-
network.Interface{
89-
Location: to.StringPtr(s.Scope.Location()),
90-
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
91-
IPConfigurations: &[]network.InterfaceIPConfiguration{
92-
{
93-
Name: to.StringPtr("pipConfig"),
94-
InterfaceIPConfigurationPropertiesFormat: nicConfig,
92+
err = s.Client.CreateOrUpdate(ctx,
93+
s.Scope.ResourceGroup(),
94+
nicSpec.Name,
95+
network.Interface{
96+
Location: to.StringPtr(s.Scope.Location()),
97+
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
98+
IPConfigurations: &[]network.InterfaceIPConfiguration{
99+
{
100+
Name: to.StringPtr("pipConfig"),
101+
InterfaceIPConfigurationPropertiesFormat: nicConfig,
102+
},
95103
},
104+
EnableAcceleratedNetworking: nicSpec.AcceleratedNetworking,
96105
},
97-
EnableAcceleratedNetworking: nicSpec.AcceleratedNetworking,
98-
},
99-
})
106+
})
100107

101-
if err != nil {
102-
return errors.Wrapf(err, "failed to create network interface %s in resource group %s", nicSpec.Name, s.Scope.ResourceGroup())
108+
if err != nil {
109+
return errors.Wrapf(err, "failed to create network interface %s in resource group %s", nicSpec.Name, s.Scope.ResourceGroup())
110+
}
111+
s.Scope.V(2).Info("successfully created network interface", "network interface", nicSpec.Name)
103112
}
104-
s.Scope.V(2).Info("successfully created network interface", "network interface", nicSpec.Name)
105113
}
106114
return nil
107115
}

‎cloud/services/networkinterfaces/networkinterfaces_test.go‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,36 @@ func TestReconcileNetworkInterface(t *testing.T) {
4343
expectedError string
4444
expect func(s *mock_networkinterfaces.MockNICScopeMockRecorder, m *mock_networkinterfaces.MockClientMockRecorder)
4545
}{
46+
{
47+
name: "network interface already exists",
48+
expectedError: "",
49+
expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, m *mock_networkinterfaces.MockClientMockRecorder) {
50+
s.NICSpecs().Return([]azure.NICSpec{
51+
{
52+
Name: "nic-1",
53+
MachineName: "azure-test1",
54+
SubnetName: "my-subnet",
55+
VNetName: "my-vnet",
56+
VNetResourceGroup: "my-rg",
57+
VMSize: "Standard_D2v2",
58+
},
59+
{
60+
Name: "nic-2",
61+
MachineName: "azure-test1",
62+
SubnetName: "my-subnet",
63+
VNetName: "my-vnet",
64+
VNetResourceGroup: "my-rg",
65+
VMSize: "Standard_D2v2",
66+
},
67+
})
68+
s.SubscriptionID().AnyTimes().Return("123")
69+
s.ResourceGroup().AnyTimes().Return("my-rg")
70+
s.Location().AnyTimes().Return("fake-location")
71+
gomock.InOrder(
72+
m.Get(context.TODO(), "my-rg", "nic-1"),
73+
m.Get(context.TODO(), "my-rg", "nic-2"))
74+
},
75+
},
4676
{
4777
name: "node network interface create fails",
4878
expectedError: "failed to create network interface my-net-interface in resource group my-rg: #: Internal Server Error: StatusCode=500",
@@ -63,6 +93,8 @@ func TestReconcileNetworkInterface(t *testing.T) {
6393
s.ResourceGroup().AnyTimes().Return("my-rg")
6494
s.Location().AnyTimes().Return("fake-location")
6595
gomock.InOrder(
96+
m.Get(context.TODO(), "my-rg", "my-net-interface").
97+
Return(network.Interface{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")),
6698
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomock.AssignableToTypeOf(network.Interface{})).
6799
Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")))
68100
},
@@ -90,6 +122,8 @@ func TestReconcileNetworkInterface(t *testing.T) {
90122
s.ResourceGroup().AnyTimes().Return("my-rg")
91123
s.Location().AnyTimes().Return("fake-location")
92124
s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New())
125+
m.Get(context.TODO(), "my-rg", "my-net-interface").
126+
Return(network.Interface{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
93127
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
94128
Location: to.StringPtr("fake-location"),
95129
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
@@ -131,6 +165,8 @@ func TestReconcileNetworkInterface(t *testing.T) {
131165
s.Location().AnyTimes().Return("fake-location")
132166
s.V(gomock.AssignableToTypeOf(3)).AnyTimes().Return(klogr.New())
133167
gomock.InOrder(
168+
m.Get(context.TODO(), "my-rg", "my-net-interface").
169+
Return(network.Interface{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")),
134170
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
135171
Location: to.StringPtr("fake-location"),
136172
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
@@ -173,6 +209,8 @@ func TestReconcileNetworkInterface(t *testing.T) {
173209
s.ResourceGroup().AnyTimes().Return("my-rg")
174210
s.Location().AnyTimes().Return("fake-location")
175211
s.V(gomock.AssignableToTypeOf(3)).AnyTimes().Return(klogr.New())
212+
m.Get(context.TODO(), "my-rg", "my-net-interface").
213+
Return(network.Interface{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
176214
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
177215
Location: to.StringPtr("fake-location"),
178216
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
@@ -214,6 +252,8 @@ func TestReconcileNetworkInterface(t *testing.T) {
214252
s.ResourceGroup().AnyTimes().Return("my-rg")
215253
s.Location().AnyTimes().Return("fake-location")
216254
s.V(gomock.AssignableToTypeOf(3)).AnyTimes().Return(klogr.New())
255+
m.Get(context.TODO(), "my-rg", "my-public-net-interface").
256+
Return(network.Interface{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
217257
m.CreateOrUpdate(context.TODO(), "my-rg", "my-public-net-interface", gomock.AssignableToTypeOf(network.Interface{}))
218258
},
219259
},
@@ -237,6 +277,8 @@ func TestReconcileNetworkInterface(t *testing.T) {
237277
s.ResourceGroup().AnyTimes().Return("my-rg")
238278
s.Location().AnyTimes().Return("fake-location")
239279
s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New())
280+
m.Get(context.TODO(), "my-rg", "my-net-interface").
281+
Return(network.Interface{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
240282
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
241283
Location: to.StringPtr("fake-location"),
242284
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
@@ -277,6 +319,8 @@ func TestReconcileNetworkInterface(t *testing.T) {
277319
s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New())
278320
s.Location().AnyTimes().Return("fake-location")
279321
s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New())
322+
m.Get(context.TODO(), "my-rg", "my-net-interface").
323+
Return(network.Interface{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
280324
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
281325
Location: to.StringPtr("fake-location"),
282326
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
@@ -301,7 +345,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
301345
tc := tc
302346
t.Run(tc.name, func(t *testing.T) {
303347
g := NewWithT(t)
304-
//t.Parallel()
348+
t.Parallel()
305349
mockCtrl := gomock.NewController(t)
306350
defer mockCtrl.Finish()
307351
scopeMock := mock_networkinterfaces.NewMockNICScope(mockCtrl)

0 commit comments

Comments
 (0)