Skip to content

Commit dc4cd63

Browse files
MaIT-HAMaIT-HA
committed
fix some code logic and add tests and integration tests
1 parent 5f5caa6 commit dc4cd63

14 files changed

+395
-118
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ SHELL = /usr/bin/env bash -o pipefail
2525
GEN_DIR := gen/mock_interfaces
2626
NETBOX_MOCKS_OUTPUT_FILE := netbox_mocks.go
2727
INTERFACE_DEFITIONS_DIR := pkg/netbox/interfaces/netbox.go
28-
GINKGO=ginkgo
28+
GINKGO=$(GOBIN)/ginkgo
2929

3030
GOFILES = $(shell find . -name \*.go ! -name 'zz_generated.*')
3131

‎internal/controller/expected_netboxmock_calls_test.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,22 @@ func mockIpAddressListWithHashFilterMismatch(ipamMock *mock_interfaces.MockIpamI
110110
}).MinTimes(1)
111111
}
112112

113+
func mockIpAddressListWithNoChange(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
114+
ipamMock.EXPECT().IpamIPAddressesList(gomock.Any(), gomock.Any()).
115+
DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamIPAddressesListOK, error) {
116+
got := params.(*ipam.IpamIPAddressesListParams)
117+
diff := deep.Equal(got, ExpectedIpAddressListParamsWithIpAddressData)
118+
// skip check for the 3rd input parameter as it is a method, method is a non comparable type
119+
if len(diff) > 0 {
120+
err := fmt.Errorf("netboxmock: unexpected call to ipam.IpamIPAddressesList (no change), diff to expected params diff: %+v", diff)
121+
catchUnexpectedParams <- err
122+
return &ipam.IpamIPAddressesListOK{Payload: nil}, err
123+
}
124+
fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesList (no change) was called with expected input,\n")
125+
return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithNoChange()}, nil
126+
}).MinTimes(1)
127+
}
128+
113129
func mockPrefixesList(ipamAPIMock *mock_interfaces.MockIpamAPI, catchUnexpectedParams chan error) {
114130
ipamAPIMock.EXPECT().IpamPrefixesList(gomock.Any()).
115131
DoAndReturn(func(ctx interface{}) *mock_interfaces.MockIpamPrefixesListRequest {

‎internal/controller/ipaddress_controller.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,18 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
205205
ll.UnlockWithRetry(ctx)
206206
}
207207

208-
// 4. if there is no change then no need to update
209-
if netboxIpAddressModel == nil {
208+
// 4. if no change in custom fields and status, skip update
209+
currentCustomFields, err := generateManagedCustomFieldsAnnotation(o.Spec.CustomFields)
210+
if err != nil {
211+
return ctrl.Result{}, err
212+
}
213+
readyCondition := apismeta.FindStatusCondition(o.Status.Conditions, "Ready")
214+
215+
if o.Status.IpAddressId == int64(netboxIpAddressModel.ID) &&
216+
readyCondition != nil &&
217+
readyCondition.Status == "True" &&
218+
readyCondition.ObservedGeneration == o.Generation &&
219+
annotations[IPManagedCustomFieldsAnnotationName] == currentCustomFields {
210220
return ctrl.Result{}, nil
211221
}
212222

‎internal/controller/ipaddress_controller_test.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,5 +172,14 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
172172
mockTenancyTenancyTenantsList,
173173
},
174174
true, false, nil),
175+
Entry("Create IpAddress CR, no change needed in NetBox",
176+
defaultIpAddressCR(true),
177+
[]func(*mock_interfaces.MockIpamInterface, chan error){
178+
mockIpAddressListWithNoChange,
179+
},
180+
[]func(*mock_interfaces.MockTenancyInterface, chan error){
181+
mockTenancyTenancyTenantsList,
182+
},
183+
false, true, ExpectedIpAddressStatus),
175184
)
176185
})

‎internal/controller/iprange_controller.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,18 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
187187
ll.UnlockWithRetry(ctx)
188188
}
189189

190-
// 4. if there is no change then no need to update
191-
if netboxIpRangeModel == nil {
190+
// 4. if no change in custom fields and status, skip update
191+
currentCustomFields, err := generateManagedCustomFieldsAnnotation(o.Spec.CustomFields)
192+
if err != nil {
193+
return ctrl.Result{}, err
194+
}
195+
readyCondition := apismeta.FindStatusCondition(o.Status.Conditions, "Ready")
196+
197+
if o.Status.IpRangeId == int64(netboxIpRangeModel.GetId()) &&
198+
readyCondition != nil &&
199+
readyCondition.Status == "True" &&
200+
readyCondition.ObservedGeneration == o.Generation &&
201+
annotations[IPRManagedCustomFieldsAnnotationName] == currentCustomFields {
192202
return ctrl.Result{}, nil
193203
}
194204

‎internal/controller/netbox_testdata_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,26 @@ func mockedResponseIPAddressListWithHash(customFields map[string]interface{}) *i
200200
}
201201
}
202202

203+
func mockedResponseIPAddressListWithNoChange() *ipam.IpamIPAddressesListOKBody {
204+
return &ipam.IpamIPAddressesListOKBody{
205+
Results: []*netboxModels.IPAddress{
206+
{
207+
ID: mockedResponseIPAddress().ID,
208+
Address: mockedResponseIPAddress().Address,
209+
Comments: comments + warningComment,
210+
Description: nsn + description + warningComment,
211+
Display: mockedResponseIPAddress().Display,
212+
Tenant: mockedResponseIPAddress().Tenant,
213+
Status: &netboxModels.IPAddressStatus{
214+
Label: &netboxLabel,
215+
Value: &value,
216+
},
217+
CustomFields: customFieldsCR,
218+
},
219+
},
220+
}
221+
}
222+
203223
func mockedResponseIPAddressList() *ipam.IpamIPAddressesListOKBody {
204224
return &ipam.IpamIPAddressesListOKBody{
205225
Results: []*netboxModels.IPAddress{

‎internal/controller/prefix_controller.go‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,14 +224,25 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
224224
ll.UnlockWithRetry(ctx)
225225
}
226226

227-
// 4. if there is no change then no need to update
228-
if netboxPrefixModel == nil {
227+
// 4. if k8s status and annotation already reflect the current state, skip patching
228+
// 4. if no change in custom fields and status, skip update
229+
currentCustomFields, err := generateManagedCustomFieldsAnnotation(o.Spec.CustomFields)
230+
if err != nil {
231+
return ctrl.Result{}, err
232+
}
233+
readyCondition := apismeta.FindStatusCondition(o.Status.Conditions, "Ready")
234+
235+
if o.Status.PrefixId == int64(netboxPrefixModel.GetId()) &&
236+
readyCondition != nil &&
237+
readyCondition.Status == "True" &&
238+
readyCondition.ObservedGeneration == o.Generation &&
239+
annotations[PXManagedCustomFieldsAnnotationName] == currentCustomFields {
229240
return ctrl.Result{}, nil
230241
}
231242

232243
// 4.1 update status fields
233-
o.Status.PrefixId = int64(netboxPrefixModel.Id)
234-
o.Status.PrefixUrl = config.GetBaseUrl() + "/ipam/prefixes/" + strconv.FormatInt(int64(netboxPrefixModel.Id), 10)
244+
o.Status.PrefixId = int64(netboxPrefixModel.GetId())
245+
o.Status.PrefixUrl = config.GetBaseUrl() + "/ipam/prefixes/" + strconv.FormatInt(int64(netboxPrefixModel.GetId()), 10)
235246
err = r.Client.Status().Update(ctx, o)
236247
if err != nil {
237248
return ctrl.Result{}, err

‎pkg/netbox/api/ip_address.go‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,27 +64,25 @@ func (c *NetboxCompositeClient) ReserveOrUpdateIpAddress(ipAddress *models.IPAdd
6464
needsUpdate := utils.NeedsUpdate(
6565
ipToUpdate,
6666
desiredIPAddress,
67-
func(c *netboxModels.IPAddress, d *netboxModels.WritableIPAddress) bool {
68-
return c.Description != d.Description
67+
func(current *netboxModels.IPAddress, desired *netboxModels.WritableIPAddress) bool {
68+
return current.Description != desired.Description
6969
},
70-
func(c *netboxModels.IPAddress, d *netboxModels.WritableIPAddress) bool {
71-
return c.Comments != d.Comments
70+
func(current *netboxModels.IPAddress, desired *netboxModels.WritableIPAddress) bool {
71+
return current.Comments != desired.Comments
7272
},
73-
func(c *netboxModels.IPAddress, d *netboxModels.WritableIPAddress) bool {
74-
return *c.Status.Value != d.Status
73+
func(current *netboxModels.IPAddress, desired *netboxModels.WritableIPAddress) bool {
74+
return *current.Status.Value != desired.Status
75+
},
76+
func(current *netboxModels.IPAddress, desired *netboxModels.WritableIPAddress) bool {
77+
return utils.CompareCustomFields(
78+
utils.NormalizeCustomFields(current.CustomFields),
79+
utils.NormalizeCustomFields(desired.CustomFields),
80+
)
7581
},
76-
utils.CheckCustomFields(
77-
func(c *netboxModels.IPAddress) map[string]interface{} {
78-
return utils.NormalizeCustomFields(c.CustomFields)
79-
},
80-
func(d *netboxModels.WritableIPAddress) map[string]interface{} {
81-
return utils.NormalizeCustomFields(d.CustomFields)
82-
},
83-
),
8482
)
8583

8684
if !needsUpdate {
87-
return nil, nil
85+
return ipToUpdate, nil
8886
}
8987

9088
// if the desired ip address has a restoration hash

‎pkg/netbox/api/ip_address_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,54 @@ func TestIPAddress(t *testing.T) {
359359
AssertNil(t, err)
360360
})
361361

362+
t.Run("Check ReserveOrUpdate with no change needed", func(t *testing.T) {
363+
inputList := ipam.NewIpamIPAddressesListParams().WithAddress(&ipAddress)
364+
365+
description := TruncateDescription(Description)
366+
comments := Comments + warningComment
367+
outputList := &ipam.IpamIPAddressesListOK{
368+
Payload: &ipam.IpamIPAddressesListOKBody{
369+
Results: []*netboxModels.IPAddress{
370+
{
371+
ID: expectedIPAddress().ID,
372+
Address: expectedIPAddress().Address,
373+
Description: description,
374+
Comments: comments,
375+
Status: &netboxModels.IPAddressStatus{
376+
Label: &Label,
377+
Value: &Value,
378+
},
379+
CustomFields: map[string]interface{}{},
380+
},
381+
},
382+
},
383+
}
384+
385+
mockIPAddress.EXPECT().IpamIPAddressesList(inputList, nil).Return(outputList, nil).AnyTimes()
386+
387+
clientV3 := &NetboxClientV3{
388+
Ipam: mockIPAddress,
389+
}
390+
compositeClient := &NetboxCompositeClient{
391+
clientV3: clientV3,
392+
}
393+
394+
result, err := compositeClient.ReserveOrUpdateIpAddress(&models.IPAddress{
395+
IpAddress: ipAddress,
396+
Metadata: &models.NetboxMetadata{
397+
Description: Description,
398+
Comments: Comments,
399+
},
400+
})
401+
AssertNil(t, err)
402+
assert.NotNil(t, result)
403+
assert.Equal(t, expectedIPAddress().ID, result.ID)
404+
assert.Equal(t, expectedIPAddress().Address, result.Address)
405+
assert.Equal(t, expectedIPAddress().Description+warningComment, result.Description)
406+
assert.Equal(t, expectedIPAddress().Comments+warningComment, result.Comments)
407+
assert.Equal(t, *expectedIPAddress().Status, *result.Status)
408+
})
409+
362410
t.Run("Check ReserveOrUpdate with hash mismatch", func(t *testing.T) {
363411
inputList := ipam.NewIpamIPAddressesListParams().WithAddress(&ipAddress)
364412
outputList := &ipam.IpamIPAddressesListOK{

‎pkg/netbox/api/ip_range.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,22 +70,24 @@ func (c *NetboxCompositeClient) ReserveOrUpdateIpRange(ctx context.Context, ipRa
7070
ipRangeToUpdate,
7171
desiredIpRange,
7272
func(current *v4client.IPRange, desired *v4client.WritableIPRangeRequest) bool {
73-
return *current.Description != *desired.Description
73+
return current.GetDescription() != desired.GetDescription()
7474
},
7575
func(current *v4client.IPRange, desired *v4client.WritableIPRangeRequest) bool {
76-
return *current.Comments != *desired.Comments
76+
return current.GetComments() != desired.GetComments()
7777
},
7878
func(current *v4client.IPRange, desired *v4client.WritableIPRangeRequest) bool {
79-
return string(*current.Status.Value) != string(*desired.Status)
79+
return string(current.Status.GetValue()) != string(desired.GetStatus())
80+
},
81+
func(current *v4client.IPRange, desired *v4client.WritableIPRangeRequest) bool {
82+
return utils.CompareCustomFields(
83+
current.CustomFields,
84+
desired.CustomFields,
85+
)
8086
},
81-
utils.CheckCustomFields(
82-
func(c *v4client.IPRange) map[string]interface{} { return c.CustomFields },
83-
func(d *v4client.WritableIPRangeRequest) map[string]interface{} { return d.CustomFields },
84-
),
8587
)
8688

8789
if !needsUpdate {
88-
return nil, nil
90+
return ipRangeToUpdate, nil
8991
}
9092

9193
// if the desired ip address has a restoration hash

0 commit comments

Comments
 (0)