Skip to content

Commit 69ccfae

Browse files
committed
update status if hash mismatch but id not nil
1 parent 744cd8b commit 69ccfae

15 files changed

+104
-63
lines changed

api/v1/iprangeclaim_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ var ConditionIpRangeAssignedFalse = metav1.Condition{
181181
Message: "Failed to fetch new IP Range from NetBox",
182182
}
183183

184-
var ConditionIpRangeAssignedFalseSizeMissmatch = metav1.Condition{
184+
var ConditionIpRangeAssignedFalseSizeMismatch = metav1.Condition{
185185
Type: "IPRangeAssigned",
186186
Status: "False",
187187
Reason: "IPRangeCRNotCreated",

internal/controller/expected_netboxmock_calls_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func mockIpAddressListWithHashFilter(ipamMock *mock_interfaces.MockIpamInterface
9292
}).MinTimes(1)
9393
}
9494

95-
func mockIpAddressListWithHashFilterMissmatch(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
95+
func mockIpAddressListWithHashFilterMismatch(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
9696
ipamMock.EXPECT().IpamIPAddressesList(gomock.Any(), gomock.Any(), gomock.Any()).
9797
DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamIPAddressesListOK, error) {
9898
got := params.(*ipam.IpamIPAddressesListParams)
@@ -104,7 +104,7 @@ func mockIpAddressListWithHashFilterMissmatch(ipamMock *mock_interfaces.MockIpam
104104
return &ipam.IpamIPAddressesListOK{Payload: nil}, err
105105
}
106106
fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesList (empty reslut) was called with expected input,\n")
107-
return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithHash(customFieldsWithHashMissmatch)}, nil
107+
return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithHash(customFieldsWithHashMismatch)}, nil
108108
}).MinTimes(1)
109109
}
110110

internal/controller/ipaddress_controller.go

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -170,25 +170,38 @@ func (r *IpAddressReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
170170

171171
netboxIpAddressModel, err := r.NetboxClient.ReserveOrUpdateIpAddress(ipAddressModel)
172172
if err != nil {
173-
if errors.Is(err, api.ErrRestorationHashMissmatch) && o.Status.IpAddressId == 0 {
174-
// if there is a restoration has missmatch and the IpAddressId status field is not set,
175-
// delete the ip address so it can be recreated by the ip address claim controller
176-
logger.Info("restoration hash missmatch, deleting ip address custom resource", "ipaddress", o.Spec.IpAddress)
177-
err = r.Client.Delete(ctx, o)
178-
if err != nil {
179-
updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
180-
corev1.EventTypeWarning, err.Error())
173+
if errors.Is(err, api.ErrRestorationHashMismatch) {
174+
if o.Status.IpAddressId == 0 {
175+
// if there is a restoration hash mismatch and the IpAddressId status field is not set,
176+
// delete the ip address so it can be recreated by the ip address claim controller
177+
// this will only affect resources that are created by a claim controller (and have a restoration hash custom field
178+
logger.Info("restoration hash mismatch, deleting ip address custom resource", "ipaddress", o.Spec.IpAddress)
179+
err = r.Client.Delete(ctx, o)
180+
if err != nil {
181+
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
182+
corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
183+
return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
184+
"after deletion of ip address cr failed: %w", updateStatusErr, err)
185+
}
186+
return ctrl.Result{Requeue: true}, nil
187+
}
188+
return ctrl.Result{}, nil
189+
}
190+
} else {
191+
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
192+
corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
181193
return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
182-
"after deletion of ip address cr failed: %w", updateStatusErr, err)
194+
"after reservation of ip in netbox failed: %w", updateStatusErr, err)
183195
}
184-
return ctrl.Result{}, nil
196+
return ctrl.Result{Requeue: true}, nil
185197
}
198+
186199
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
187-
corev1.EventTypeWarning, o.Spec.IpAddress); updateStatusErr != nil {
200+
corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
188201
return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
189202
"after reservation of ip in netbox failed: %w", updateStatusErr, err)
190203
}
191-
return ctrl.Result{}, nil
204+
return ctrl.Result{Requeue: true}, nil
192205
}
193206

194207
// 3. unlock lease of parent prefix

internal/controller/ipaddress_controller_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
5252
cr *netboxv1.IpAddress, // our CR as typed object
5353
IpamMocksIpAddress []func(*mock_interfaces.MockIpamInterface, chan error),
5454
TenancyMocks []func(*mock_interfaces.MockTenancyInterface, chan error),
55-
restorationHashMissmatch bool, // To check for deletion if restoration hash does not match
55+
restorationHashMismatch bool, // To check for deletion if restoration hash does not match
5656
expectedConditionReady bool, // Expected state of the ConditionReady condition
5757
expectedCRStatus netboxv1.IpAddressStatus, // Expected status of the CR
5858
) {
@@ -85,7 +85,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
8585

8686
createdCR := &netboxv1.IpAddress{}
8787

88-
if restorationHashMissmatch {
88+
if restorationHashMismatch {
8989
Eventually(func() bool {
9090
err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
9191
return apierrors.IsNotFound(err)
@@ -163,10 +163,10 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
163163
mockTenancyTenancyTenantsList,
164164
},
165165
false, false, ExpectedIpAddressFailedStatus),
166-
Entry("Create IpAddress CR, restoration hash missmatch",
166+
Entry("Create IpAddress CR, restoration hash mismatch",
167167
defaultIpAddressCreatedByClaim(true),
168168
[]func(*mock_interfaces.MockIpamInterface, chan error){
169-
mockIpAddressListWithHashFilterMissmatch,
169+
mockIpAddressListWithHashFilterMismatch,
170170
},
171171
[]func(*mock_interfaces.MockTenancyInterface, chan error){
172172
mockTenancyTenancyTenantsList,

internal/controller/iprange_controller.go

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -143,27 +143,37 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
143143

144144
netboxIpRangeModel, err := r.NetboxClient.ReserveOrUpdateIpRange(ipRangeModel)
145145
if err != nil {
146-
if errors.Is(err, api.ErrRestorationHashMissmatch) && o.Status.IpRangeId == 0 {
147-
// if there is a restoration has missmatch and the IpRangeId status field is not set,
148-
// delete the ip range so it can be recreated by the ip range claim controller
149-
logger.Info("restoration hash missmatch, deleting ip range custom resource", "ip-range-start", o.Spec.StartAddress, "ip-range-end", o.Spec.EndAddress)
150-
err = r.Client.Delete(ctx, o)
151-
if err != nil {
152-
if err := r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
146+
if errors.Is(err, api.ErrRestorationHashMismatch) {
147+
if o.Status.IpRangeId == 0 {
148+
// if there is a restoration hash mismatch and the IpRangeId status field is not set,
149+
// delete the ip range so it can be recreated by the ip range claim controller
150+
// this will only affect resources that are created by a claim controller (and have a restoration hash custom field
151+
logger.Info("restoration hash mismatch, deleting ip range custom resource", "ip-range-start", o.Spec.StartAddress, "ip-range-end", o.Spec.EndAddress)
152+
err = r.Client.Delete(ctx, o)
153+
if err != nil {
154+
if err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
155+
corev1.EventTypeWarning, "", err); err != nil {
156+
return ctrl.Result{}, err
157+
}
158+
return ctrl.Result{Requeue: true}, nil
159+
}
160+
return ctrl.Result{}, nil
161+
} else {
162+
if err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
153163
corev1.EventTypeWarning, "", err); err != nil {
154164
return ctrl.Result{}, err
155165
}
166+
return ctrl.Result{Requeue: true}, nil
156167
}
157-
return ctrl.Result{}, nil
158168
}
159169

160-
if loggingErr := r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
161-
corev1.EventTypeWarning, fmt.Sprintf("%s-%s ", o.Spec.StartAddress, o.Spec.EndAddress), err); loggingErr != nil {
162-
return ctrl.Result{}, fmt.Errorf("logging error: %w. Original error from ReserveOrUpdateIpRange: %w", loggingErr, err)
170+
if err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
171+
corev1.EventTypeWarning, fmt.Sprintf("%s-%s ", o.Spec.StartAddress, o.Spec.EndAddress), err); err != nil {
172+
return ctrl.Result{}, err
163173
}
164174

165175
// The decision to not return the error message (just logging it) is to not trigger printing the stacktrace on api errors
166-
return ctrl.Result{}, nil
176+
return ctrl.Result{Requeue: true}, nil
167177
}
168178

169179
// 3. unlock lease of parent prefix

internal/controller/iprangeclaim_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ func (r *IpRangeClaimReconciler) restoreOrAssignIpRangeAndSetCondition(ctx conte
322322
availableIpRanges, err := r.NetboxClient.GetAvailableIpAddressesByIpRange(ipRangeModel.Id)
323323
if len(availableIpRanges.Payload) != o.Spec.Size {
324324
ll.Unlock()
325-
err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeAssignedFalseSizeMissmatch, corev1.EventTypeWarning, "", err)
325+
err = r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeAssignedFalseSizeMismatch, corev1.EventTypeWarning, "", err)
326326
if err != nil {
327327
return nil, ctrl.Result{}, err
328328
}

internal/controller/netbox_testdata_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ var customFieldsCR = map[string]string{"example_field": "example value"}
5757
var customFieldsWithHashCR = map[string]string{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash}
5858

5959
var customFieldsWithHash = map[string]interface{}{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash}
60-
var customFieldsWithHashMissmatch = map[string]interface{}{"example_field": "example value", "netboxOperatorRestorationHash": "different hash"}
60+
var customFieldsWithHashMismatch = map[string]interface{}{"example_field": "example value", "netboxOperatorRestorationHash": "different hash"}
6161

6262
var netboxLabel = "Status"
6363
var value = "active"

internal/controller/prefix_controller.go

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -184,25 +184,37 @@ func (r *PrefixReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
184184

185185
netboxPrefixModel, err := r.NetboxClient.ReserveOrUpdatePrefix(prefixModel)
186186
if err != nil {
187-
if errors.Is(err, api.ErrRestorationHashMissmatch) && prefix.Status.PrefixId == 0 {
188-
// if there is a restoration has missmatch and the PrefixId status field is not set,
189-
// delete the prefix so it can be recreated by the prefix claim controller
190-
logger.Info("restoration hash missmatch, deleting prefix custom resource", "prefix", prefix.Spec.Prefix)
191-
err = r.Client.Delete(ctx, prefix)
192-
if err != nil {
193-
updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionIpaddressReadyFalse,
194-
corev1.EventTypeWarning, err.Error())
195-
return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+
196-
"after deletion of prefix cr failed: %w", updateStatusErr, err)
187+
if errors.Is(err, api.ErrRestorationHashMismatch) {
188+
if prefix.Status.PrefixId == 0 {
189+
// if there is a restoration hash mismatch and the PrefixId status field is not set,
190+
// delete the prefix so it can be recreated by the prefix claim controller
191+
// this will only affect resources that are created by a claim controller (and have a restoration hash custom field
192+
logger.Info("restoration hash mismatch, deleting prefix custom resource", "prefix", prefix.Spec.Prefix)
193+
err = r.Client.Delete(ctx, prefix)
194+
if err != nil {
195+
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse,
196+
corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
197+
return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+
198+
"after deletion of prefix cr failed: %w", updateStatusErr, err)
199+
}
200+
return ctrl.Result{Requeue: true}, nil
201+
}
202+
return ctrl.Result{}, nil
203+
} else {
204+
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse,
205+
corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
206+
return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+
207+
"after deletion of prefix cr failed: %w", updateStatusErr, err)
208+
}
209+
return ctrl.Result{Requeue: true}, nil
197210
}
198-
return ctrl.Result{}, nil
199211
}
200-
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionIpaddressReadyFalse,
201-
corev1.EventTypeWarning, prefix.Spec.Prefix); updateStatusErr != nil {
212+
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse,
213+
corev1.EventTypeWarning, err.Error()); updateStatusErr != nil {
202214
return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+
203215
"after reservation of prefix netbox failed: %w", updateStatusErr, err)
204216
}
205-
return ctrl.Result{}, nil
217+
return ctrl.Result{Requeue: true}, nil
206218
}
207219

208220
/* 3. unlock lease of parent prefix */

pkg/netbox/api/errors.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,5 @@ var (
2323
ErrParentPrefixNotFound = errors.New("parent prefix not found")
2424
ErrWrongMatchingPrefixSubnetFormat = errors.New("wrong matchingPrefix subnet format")
2525
ErrInvalidIpFamily = errors.New("invalid IP Family")
26-
ErrRestorationHashMissmatch = errors.New("restoration hash missmatch")
26+
ErrRestorationHashMismatch = errors.New("restoration hash mismatch")
2727
)

pkg/netbox/api/ip_address.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (r *NetboxClient) ReserveOrUpdateIpAddress(ipAddress *models.IPAddress) (*n
7070
//update ip address since it does exist and the restoration hash matches
7171
return r.UpdateIpAddress(ipToUpdate.ID, desiredIPAddress)
7272
}
73-
return nil, fmt.Errorf("%w, assigned ip address %s", ErrRestorationHashMissmatch, ipAddress.IpAddress)
73+
return nil, fmt.Errorf("%w, assigned ip address %s", ErrRestorationHashMismatch, ipAddress.IpAddress)
7474
}
7575
}
7676

0 commit comments

Comments
 (0)