Skip to content

Commit 44b542e

Browse files
brueleabruelea
committed
check restoration hash when resource is update
1 parent 57c7e61 commit 44b542e

14 files changed

+414
-53
lines changed

‎config/samples/netbox_v1_prefixclaim_parentprefixselector.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ spec:
1414
prefixLength: "/31"
1515
parentPrefixSelector:
1616
tenant: "MY_TENANT"
17-
site: "DM-Buffalo"
1817
family: "IPv4"
1918
environment: "Production"
20-
poolName: "Pool 1"
19+
poolName: "Pool 1"

‎internal/controller/expected_netboxmock_calls_test.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ func mockIpAddressListWithIpAddressFilter(ipamMock *mock_interfaces.MockIpamInte
4141
return &ipam.IpamIPAddressesListOK{Payload: nil}, err
4242
}
4343
fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesList was called with expected input\n")
44-
return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressList()}, nil
44+
return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithHash(customFieldsWithHash)}, nil
4545
}).MinTimes(1)
4646
}
4747

@@ -92,6 +92,22 @@ func mockIpAddressListWithHashFilter(ipamMock *mock_interfaces.MockIpamInterface
9292
}).MinTimes(1)
9393
}
9494

95+
func mockIpAddressListWithHashFilterMissmatch(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
96+
ipamMock.EXPECT().IpamIPAddressesList(gomock.Any(), gomock.Any(), gomock.Any()).
97+
DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamIPAddressesListOK, error) {
98+
got := params.(*ipam.IpamIPAddressesListParams)
99+
diff := deep.Equal(got, ExpectedIpAddressListParamsWithIpAddressData)
100+
// skip check for the 3rd input parameter as it is a method, method is a non comparable type
101+
if len(diff) > 0 {
102+
err := fmt.Errorf("netboxmock: unexpected call to ipam.IpamIPAddressesList, diff to expected params diff: %+v", diff)
103+
catchUnexpectedParams <- err
104+
return &ipam.IpamIPAddressesListOK{Payload: nil}, err
105+
}
106+
fmt.Printf("NETBOXMOCK\t ipam.IpamIPAddressesList (empty reslut) was called with expected input,\n")
107+
return &ipam.IpamIPAddressesListOK{Payload: mockedResponseIPAddressListWithHash(customFieldsWithHashMissmatch)}, nil
108+
}).MinTimes(1)
109+
}
110+
95111
func mockPrefixesListWithPrefixFilter(ipamMock *mock_interfaces.MockIpamInterface, catchUnexpectedParams chan error) {
96112
ipamMock.EXPECT().IpamPrefixesList(gomock.Any(), gomock.Any()).
97113
DoAndReturn(func(params interface{}, authInfo interface{}, opts ...interface{}) (*ipam.IpamPrefixesListOK, error) {

‎internal/controller/ipaddress_controller.go

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

171171
netboxIpAddressModel, err := r.NetboxClient.ReserveOrUpdateIpAddress(ipAddressModel)
172172
if err != nil {
173-
updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
174-
corev1.EventTypeWarning, o.Spec.IpAddress)
175-
return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
176-
"after reservation of ip in netbox failed: %w", updateStatusErr, err)
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())
181+
return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
182+
"after deletion of ip address cr failed: %w", updateStatusErr, err)
183+
}
184+
return ctrl.Result{}, nil
185+
}
186+
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpaddressReadyFalse,
187+
corev1.EventTypeWarning, o.Spec.IpAddress); updateStatusErr != nil {
188+
return ctrl.Result{}, fmt.Errorf("failed to update ip address status: %w, "+
189+
"after reservation of ip in netbox failed: %w", updateStatusErr, err)
190+
}
191+
return ctrl.Result{}, nil
177192
}
178193

179194
// 3. unlock lease of parent prefix

‎internal/controller/ipaddress_controller_test.go

Lines changed: 48 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"sigs.k8s.io/controller-runtime/pkg/client"
2929

3030
netboxv1 "github.com/netbox-community/netbox-operator/api/v1"
31+
apierrors "k8s.io/apimachinery/pkg/api/errors"
3132
)
3233

3334
var _ = Describe("IpAddress Controller", Ordered, func() {
@@ -51,6 +52,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
5152
cr *netboxv1.IpAddress, // our CR as typed object
5253
IpamMocksIpAddress []func(*mock_interfaces.MockIpamInterface, chan error),
5354
TenancyMocks []func(*mock_interfaces.MockTenancyInterface, chan error),
55+
restorationHashMissmatch bool, // To check for deletion if restoration hash does not match
5456
expectedConditionReady bool, // Expected state of the ConditionReady condition
5557
expectedCRStatus netboxv1.IpAddressStatus, // Expected status of the CR
5658
) {
@@ -81,31 +83,40 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
8183
By("Creating IpAddress CR")
8284
Eventually(k8sClient.Create(ctx, cr), timeout, interval).Should(Succeed())
8385

84-
// check that reconcile loop did run a least once by checking that conditions are set
8586
createdCR := &netboxv1.IpAddress{}
86-
Eventually(func() bool {
87-
err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
88-
return err == nil && len(createdCR.Status.Conditions) > 0
89-
}, timeout, interval).Should(BeTrue())
90-
91-
// Now check if conditions are set as expected
92-
Eventually(func() bool {
93-
err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
94-
return err == nil &&
95-
apismeta.IsStatusConditionTrue(createdCR.Status.Conditions, netboxv1.ConditionIpaddressReadyTrue.Type) == expectedConditionReady
96-
}, timeout, interval).Should(BeTrue())
97-
98-
// Check that the expected ip address is present in the status
99-
Expect(createdCR.Status.IpAddressId).To(Equal(expectedCRStatus.IpAddressId))
100-
101-
// Cleanup the netbox resources
102-
Expect(k8sClient.Delete(ctx, createdCR)).Should(Succeed())
103-
104-
// Wait until the resource is deleted to make sure that it will not interfere with the next test case
105-
Eventually(func() bool {
106-
err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
107-
return err != client.IgnoreNotFound(err)
108-
}, timeout, interval).Should(BeTrue())
87+
88+
if restorationHashMissmatch {
89+
Eventually(func() bool {
90+
err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
91+
return apierrors.IsNotFound(err)
92+
}, timeout, interval).Should(BeTrue())
93+
} else {
94+
95+
// check that reconcile loop did run a least once by checking that conditions are set
96+
Eventually(func() bool {
97+
err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
98+
return err == nil && len(createdCR.Status.Conditions) > 0
99+
}, timeout, interval).Should(BeTrue())
100+
101+
// Now check if conditions are set as expected
102+
Eventually(func() bool {
103+
err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
104+
return err == nil &&
105+
apismeta.IsStatusConditionTrue(createdCR.Status.Conditions, netboxv1.ConditionIpaddressReadyTrue.Type) == expectedConditionReady
106+
}, timeout, interval).Should(BeTrue())
107+
108+
// Check that the expected ip address is present in the status
109+
Expect(createdCR.Status.IpAddressId).To(Equal(expectedCRStatus.IpAddressId))
110+
111+
// Cleanup the netbox resources
112+
Expect(k8sClient.Delete(ctx, createdCR)).Should(Succeed())
113+
114+
// Wait until the resource is deleted to make sure that it will not interfere with the next test case
115+
Eventually(func() bool {
116+
err := k8sClient.Get(ctx, types.NamespacedName{Name: cr.GetName(), Namespace: cr.GetNamespace()}, createdCR)
117+
return err != client.IgnoreNotFound(err)
118+
}, timeout, interval).Should(BeTrue())
119+
}
109120

110121
catchCtxCancel()
111122
},
@@ -119,7 +130,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
119130
[]func(*mock_interfaces.MockTenancyInterface, chan error){
120131
mockTenancyTenancyTenantsList,
121132
},
122-
true, ExpectedIpAddressStatus),
133+
false, true, ExpectedIpAddressStatus),
123134
Entry("Create IpAddress CR, ip address already reserved in NetBox, preserved in netbox, ",
124135
defaultIpAddressCR(true),
125136
[]func(*mock_interfaces.MockIpamInterface, chan error){
@@ -129,7 +140,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
129140
[]func(*mock_interfaces.MockTenancyInterface, chan error){
130141
mockTenancyTenancyTenantsList,
131142
},
132-
true, ExpectedIpAddressStatus),
143+
false, true, ExpectedIpAddressStatus),
133144
Entry("Create IpAddress CR, ip address already reserved in NetBox",
134145
defaultIpAddressCR(false),
135146
[]func(*mock_interfaces.MockIpamInterface, chan error){
@@ -140,7 +151,7 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
140151
[]func(*mock_interfaces.MockTenancyInterface, chan error){
141152
mockTenancyTenancyTenantsList,
142153
},
143-
true, ExpectedIpAddressStatus),
154+
false, true, ExpectedIpAddressStatus),
144155
Entry("Create IpAddress CR, reserve or update failure",
145156
defaultIpAddressCR(false),
146157
[]func(*mock_interfaces.MockIpamInterface, chan error){
@@ -151,6 +162,15 @@ var _ = Describe("IpAddress Controller", Ordered, func() {
151162
[]func(*mock_interfaces.MockTenancyInterface, chan error){
152163
mockTenancyTenancyTenantsList,
153164
},
154-
false, ExpectedIpAddressFailedStatus),
165+
false, false, ExpectedIpAddressFailedStatus),
166+
Entry("Create IpAddress CR, restoration hash missmatch",
167+
defaultIpAddressCreatedByClaim(true),
168+
[]func(*mock_interfaces.MockIpamInterface, chan error){
169+
mockIpAddressListWithHashFilterMissmatch,
170+
},
171+
[]func(*mock_interfaces.MockTenancyInterface, chan error){
172+
mockTenancyTenancyTenantsList,
173+
},
174+
true, false, nil),
155175
)
156176
})

‎internal/controller/iprange_controller.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controller
1919
import (
2020
"context"
2121
"encoding/json"
22+
"errors"
2223
"fmt"
2324
"maps"
2425
"strconv"
@@ -142,6 +143,20 @@ func (r *IpRangeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
142143

143144
netboxIpRangeModel, err := r.NetboxClient.ReserveOrUpdateIpRange(ipRangeModel)
144145
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,
153+
corev1.EventTypeWarning, "", err); err != nil {
154+
return ctrl.Result{}, err
155+
}
156+
}
157+
return ctrl.Result{}, nil
158+
}
159+
145160
if loggingErr := r.logErrorSetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpRangeReadyFalse,
146161
corev1.EventTypeWarning, fmt.Sprintf("%s-%s ", o.Spec.StartAddress, o.Spec.EndAddress), err); loggingErr != nil {
147162
return ctrl.Result{}, fmt.Errorf("logging error: %w. Original error from ReserveOrUpdateIpRange: %w", loggingErr, err)

‎internal/controller/netbox_testdata_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ var restorationHash = "6f6c67651f0b43b2969ba2ae35c74fc91815513b"
5555

5656
var customFields = map[string]string{"example_field": "example value"}
5757
var customFieldsWithHash = map[string]string{"example_field": "example value", "netboxOperatorRestorationHash": restorationHash}
58+
var customFieldsWithHashMissmatch = map[string]string{"example_field": "example value", "netboxOperatorRestorationHash": "different hash"}
5859

5960
var netboxLabel = "Status"
6061
var value = "active"
@@ -176,6 +177,22 @@ func mockedResponsePrefixList() *ipam.IpamPrefixesListOKBody {
176177
}
177178
}
178179

180+
func mockedResponseIPAddressListWithHash(customFields map[string]string) *ipam.IpamIPAddressesListOKBody {
181+
return &ipam.IpamIPAddressesListOKBody{
182+
Results: []*netboxModels.IPAddress{
183+
{
184+
ID: mockedResponseIPAddress().ID,
185+
Address: mockedResponseIPAddress().Address,
186+
Comments: mockedResponseIPAddress().Comments,
187+
CustomFields: customFields,
188+
Description: mockedResponseIPAddress().Description,
189+
Display: mockedResponseIPAddress().Display,
190+
Tenant: mockedResponseIPAddress().Tenant,
191+
},
192+
},
193+
}
194+
}
195+
179196
func mockedResponseIPAddressList() *ipam.IpamIPAddressesListOKBody {
180197
return &ipam.IpamIPAddressesListOKBody{
181198
Results: []*netboxModels.IPAddress{

‎internal/controller/prefix_controller.go

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

185185
netboxPrefixModel, err := r.NetboxClient.ReserveOrUpdatePrefix(prefixModel)
186186
if err != nil {
187-
updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionPrefixReadyFalse, corev1.EventTypeWarning, prefix.Spec.Prefix)
188-
return ctrl.Result{}, fmt.Errorf("failed at update prefix status: %w, "+"after reservation of prefix in netbox failed: %w", updateStatusErr, err)
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)
197+
}
198+
return ctrl.Result{}, nil
199+
}
200+
if updateStatusErr := r.SetConditionAndCreateEvent(ctx, prefix, netboxv1.ConditionIpaddressReadyFalse,
201+
corev1.EventTypeWarning, prefix.Spec.Prefix); updateStatusErr != nil {
202+
return ctrl.Result{}, fmt.Errorf("failed to update prefix status: %w, "+
203+
"after reservation of prefix netbox failed: %w", updateStatusErr, err)
204+
}
205+
return ctrl.Result{}, nil
189206
}
190207

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

‎pkg/netbox/api/errors.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +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")
2627
)

‎pkg/netbox/api/ip_address.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ limitations under the License.
1717
package api
1818

1919
import (
20+
"fmt"
2021
"net/http"
2122

2223
"github.com/netbox-community/go-netbox/v3/netbox/client/ipam"
2324
netboxModels "github.com/netbox-community/go-netbox/v3/netbox/models"
25+
"github.com/netbox-community/netbox-operator/pkg/config"
2426

2527
"github.com/netbox-community/netbox-operator/pkg/netbox/models"
2628
"github.com/netbox-community/netbox-operator/pkg/netbox/utils"
@@ -33,14 +35,18 @@ func (r *NetboxClient) ReserveOrUpdateIpAddress(ipAddress *models.IPAddress) (*n
3335
}
3436

3537
desiredIPAddress := &netboxModels.WritableIPAddress{
36-
Address: &ipAddress.IpAddress,
37-
Comments: ipAddress.Metadata.Comments + warningComment,
38-
CustomFields: ipAddress.Metadata.Custom,
39-
Description: TruncateDescription(ipAddress.Metadata.Description),
40-
Status: "active",
38+
Address: &ipAddress.IpAddress,
39+
Description: TruncateDescription(""),
40+
Status: "active",
4141
}
4242

43-
if ipAddress.Metadata.Tenant != "" {
43+
if ipAddress.Metadata != nil {
44+
desiredIPAddress.CustomFields = ipAddress.Metadata.Custom
45+
desiredIPAddress.Comments = ipAddress.Metadata.Comments + warningComment
46+
desiredIPAddress.Description = TruncateDescription(ipAddress.Metadata.Description)
47+
}
48+
49+
if ipAddress.Metadata != nil && ipAddress.Metadata.Tenant != "" {
4450
tenantDetails, err := r.GetTenantDetails(ipAddress.Metadata.Tenant)
4551
if err != nil {
4652
return nil, err
@@ -52,7 +58,22 @@ func (r *NetboxClient) ReserveOrUpdateIpAddress(ipAddress *models.IPAddress) (*n
5258
if len(responseIpAddress.Payload.Results) == 0 {
5359
return r.CreateIpAddress(desiredIPAddress)
5460
}
55-
//update ip address since it does exist
61+
62+
ipToUpdate := responseIpAddress.Payload.Results[0]
63+
64+
// if the desired ip address has a restoration hash
65+
// check that the ip address to update has the same restoration hash
66+
restorationHashKey := config.GetOperatorConfig().NetboxRestorationHashFieldName
67+
if ipAddress.Metadata != nil {
68+
if restorationHash, ok := ipAddress.Metadata.Custom[restorationHashKey]; ok {
69+
if ipToUpdate.CustomFields != nil && ipToUpdate.CustomFields.(map[string]interface{})[restorationHashKey] == restorationHash {
70+
//update ip address since it does exist and the restoration hash matches
71+
return r.UpdateIpAddress(ipToUpdate.ID, desiredIPAddress)
72+
}
73+
return nil, fmt.Errorf("%w, assigned ip address %s", ErrRestorationHashMissmatch, ipAddress.IpAddress)
74+
}
75+
}
76+
5677
ipAddressId := responseIpAddress.Payload.Results[0].ID
5778
return r.UpdateIpAddress(ipAddressId, desiredIPAddress)
5879
}

0 commit comments

Comments
 (0)