Skip to content

Commit 4d91930

Browse files
author
Hoanganh.Mai
committed
add check for tenant existence
1 parent 0508ece commit 4d91930

File tree

9 files changed

+229
-9
lines changed

9 files changed

+229
-9
lines changed

‎internal/controller/expected_netboxmock_calls_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ func mockTenancyTenancyTenantsList(tenancyMock *mock_interfaces.MockTenancyInter
234234
}
235235

236236
// -----------------------------
237-
// Restet Mock Functions
237+
// Reset Mock Functions
238238
// -----------------------------
239239

240240
func resetMockFunctions(ipamMockA *mock_interfaces.MockIpamInterface, ipamMockB *mock_interfaces.MockIpamInterface, tenancyMock *mock_interfaces.MockTenancyInterface) {

‎internal/controller/ipaddressclaim_controller.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controller
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"strings"
2324
"time"
@@ -26,6 +27,7 @@ import (
2627
"github.com/netbox-community/netbox-operator/pkg/config"
2728
"github.com/netbox-community/netbox-operator/pkg/netbox/api"
2829
"github.com/netbox-community/netbox-operator/pkg/netbox/models"
30+
"github.com/netbox-community/netbox-operator/pkg/netbox/utils"
2931
"github.com/swisscom/leaselocker"
3032
corev1 "k8s.io/api/core/v1"
3133
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -137,9 +139,14 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
137139
},
138140
})
139141
if err != nil {
142+
if errors.Is(err, utils.ErrNotFound) {
143+
debugLogger.Error(err, "error in getting an available IP address")
144+
return ctrl.Result{}, nil
145+
}
146+
140147
return ctrl.Result{}, err
141148
}
142-
debugLogger.Info(fmt.Sprintf("ip address is not reserved in netbox, assignined new ip address: %s", ipAddressModel.IpAddress))
149+
debugLogger.Info(fmt.Sprintf("ip address is not reserved in netbox, assigned new ip address: %s", ipAddressModel.IpAddress))
143150
} else {
144151
// 5.b reassign reserved ip address from netbox
145152
// do nothing, ip address restored

‎pkg/netbox/api/ip_address_claim.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
"github.com/netbox-community/go-netbox/v3/netbox/client/ipam"
2525
"github.com/netbox-community/netbox-operator/pkg/netbox/models"
26+
"github.com/netbox-community/netbox-operator/pkg/netbox/utils"
2627
)
2728

2829
const (
@@ -57,6 +58,10 @@ func (r *NetboxClient) RestoreExistingIpByHash(customFieldName string, hash stri
5758

5859
// GetAvailableIpAddressByClaim searches an available IpAddress in Netbox matching IpAddressClaim requirements
5960
func (r *NetboxClient) GetAvailableIpAddressByClaim(ipAddressClaim *models.IPAddressClaim) (*models.IPAddress, error) {
61+
_, err := r.GetTenantDetails(ipAddressClaim.Metadata.Tenant)
62+
if err != nil {
63+
return nil, err
64+
}
6065

6166
responseParentPrefix, err := r.GetPrefix(&models.Prefix{
6267
Prefix: ipAddressClaim.ParentPrefix,
@@ -66,7 +71,7 @@ func (r *NetboxClient) GetAvailableIpAddressByClaim(ipAddressClaim *models.IPAdd
6671
return nil, err
6772
}
6873
if len(responseParentPrefix.Payload.Results) == 0 {
69-
return nil, errors.New("parent prefix not found")
74+
return nil, utils.NetboxNotFoundError("parent prefix")
7075
}
7176

7277
parentPrefixId := responseParentPrefix.Payload.Results[0].ID

‎pkg/netbox/api/ip_address_claim_test.go‎

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package api
1818

1919
import (
20+
"errors"
2021
"testing"
2122

2223
"github.com/netbox-community/go-netbox/v3/netbox/client/ipam"
@@ -166,8 +167,10 @@ func TestIPAddressClaim(t *testing.T) {
166167
},
167168
})
168169

170+
expectedErrMsg := "failed to fetch parent prefix: not found"
171+
169172
// assert error
170-
AssertError(t, err, "parent prefix not found")
173+
AssertError(t, err, expectedErrMsg)
171174
// assert nil output
172175
assert.Nil(t, actual)
173176
})
@@ -231,3 +234,77 @@ func TestIPAddressClaim(t *testing.T) {
231234
assert.Equal(t, ipAddressRestore, actual.IpAddress)
232235
})
233236
}
237+
238+
func TestIPAddressClaim_GetNoAvailableIPAddressWithTenancyChecks(t *testing.T) {
239+
ctrl := gomock.NewController(t)
240+
defer ctrl.Finish()
241+
242+
mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl)
243+
244+
tenantName := "Tenant1"
245+
246+
inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName)
247+
248+
parentPrefix := "10.112.140.0/24"
249+
t.Run("No IP address asigned with an error when getting the tenant list", func(t *testing.T) {
250+
251+
// expected error
252+
expectedErrorMsg := "cannot get the list" // testcase-defined error
253+
254+
mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(nil, errors.New(expectedErrorMsg)).AnyTimes()
255+
256+
// init client
257+
client := &NetboxClient{
258+
Tenancy: mockTenancy,
259+
}
260+
261+
actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{
262+
ParentPrefix: parentPrefix,
263+
Metadata: &models.NetboxMetadata{
264+
Tenant: tenantName,
265+
},
266+
})
267+
268+
// assert error
269+
assert.Containsf(t, err.Error(), expectedErrorMsg, "Error should contain: %v, got: %v", expectedErrorMsg, err.Error())
270+
// assert nil output
271+
assert.Equal(t, actual, (*models.IPAddress)(nil))
272+
})
273+
274+
t.Run("No IP address asigned with non-existing tenant", func(t *testing.T) {
275+
276+
// non existing tenant
277+
nonExistingTenant := "non-existing-tenant"
278+
279+
// empty tenant list
280+
emptyTenantList := &tenancy.TenancyTenantsListOK{
281+
Payload: &tenancy.TenancyTenantsListOKBody{
282+
Results: []*netboxModels.Tenant{},
283+
},
284+
}
285+
286+
// expected error
287+
expectedErrorMsg := "failed to fetch tenant: not found"
288+
289+
// mock empty list call
290+
mockTenancy.EXPECT().TenancyTenantsList(gomock.Not(inputTenant), nil).Return(emptyTenantList, nil).AnyTimes()
291+
292+
// init client
293+
client := &NetboxClient{
294+
Tenancy: mockTenancy,
295+
}
296+
297+
actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{
298+
ParentPrefix: parentPrefix,
299+
Metadata: &models.NetboxMetadata{
300+
Tenant: nonExistingTenant,
301+
},
302+
})
303+
304+
// assert error
305+
assert.EqualErrorf(t, err, expectedErrorMsg, "Error should be: %v, got: %v", expectedErrorMsg, err)
306+
// assert nil output
307+
assert.Equal(t, actual, (*models.IPAddress)(nil))
308+
})
309+
310+
}

‎pkg/netbox/api/prefix_claim.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ func isRequestingTheEntireParentPrefix(prefixClaim *models.PrefixClaim) (bool, e
6767

6868
// GetAvailablePrefixByClaim searches an available Prefix in Netbox matching PrefixClaim requirements
6969
func (r *NetboxClient) GetAvailablePrefixByClaim(prefixClaim *models.PrefixClaim) (*models.Prefix, error) {
70+
_, err := r.GetTenantDetails(prefixClaim.Metadata.Tenant)
71+
if err != nil {
72+
return nil, err
73+
}
74+
7075
responseParentPrefix, err := r.GetPrefix(&models.Prefix{
7176
Prefix: prefixClaim.ParentPrefix,
7277
Metadata: prefixClaim.Metadata,

‎pkg/netbox/api/prefix_claim_test.go‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ func TestPrefixClaim_GetAvailablePrefixesByParentPrefix(t *testing.T) {
3838
//prefix mock input
3939
parentPrefixId := int64(3)
4040
availablePrefixListInput := ipam.NewIpamPrefixesAvailablePrefixesListParams().WithID(parentPrefixId)
41+
4142
//prefix mock output
4243
childPrefix1 := "10.112.140.0/24"
4344
childPrefix2 := "10.120.180.0/24"
@@ -129,6 +130,9 @@ func TestPrefixClaim_GetAvailablePrefixByClaim_WithWrongParent(t *testing.T) {
129130
actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
130131
ParentPrefix: prefix,
131132
PrefixLength: "/28",
133+
Metadata: &models.NetboxMetadata{
134+
Tenant: tenantName,
135+
},
132136
})
133137
assert.Nil(t, actual)
134138
assert.EqualError(t, err, "parent prefix not found")
@@ -196,6 +200,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaim(t *testing.T) {
196200
actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
197201
ParentPrefix: parentPrefix,
198202
PrefixLength: "/28",
203+
Metadata: &models.NetboxMetadata{
204+
Tenant: tenantName,
205+
},
199206
})
200207

201208
assert.Nil(t, err)
@@ -272,6 +279,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimNoAvailablePrefixMatchesSize(t *test
272279
actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
273280
ParentPrefix: parentPrefix,
274281
PrefixLength: "/28",
282+
Metadata: &models.NetboxMetadata{
283+
Tenant: tenantName,
284+
},
275285
})
276286

277287
assert.Nil(t, err)
@@ -340,6 +350,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimNoAvailablePrefixMatchesSizeCriteria
340350
_, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
341351
ParentPrefix: parentPrefix,
342352
PrefixLength: "/28",
353+
Metadata: &models.NetboxMetadata{
354+
Tenant: tenantName,
355+
},
343356
})
344357

345358
assert.True(t, errors.Is(err, ErrNoPrefixMatchsSizeCriteria))
@@ -415,6 +428,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimInvalidFormatFromNetbox(t *testing.T
415428
actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
416429
ParentPrefix: parentPrefix,
417430
PrefixLength: "/28",
431+
Metadata: &models.NetboxMetadata{
432+
Tenant: tenantName,
433+
},
418434
})
419435

420436
assert.Nil(t, err)
@@ -483,7 +499,112 @@ func TestPrefixClaim_GetBestFitPrefixByClaimInvalidPrefixClaim(t *testing.T) {
483499
_, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
484500
ParentPrefix: parentPrefix,
485501
PrefixLength: "/28.",
502+
Metadata: &models.NetboxMetadata{
503+
Tenant: tenantName,
504+
},
486505
})
487506

488507
assert.True(t, errors.Is(err, strconv.ErrSyntax))
489508
}
509+
510+
func TestPrefixClaim_GetNoAvailablePrefixesWithNonExistingTenant(t *testing.T) {
511+
ctrl := gomock.NewController(t)
512+
defer ctrl.Finish()
513+
mockPrefixIpam := mock_interfaces.NewMockIpamInterface(ctrl)
514+
mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl)
515+
516+
// example of tenant
517+
tenantId := int64(2)
518+
tenantName := "Tenant1"
519+
tenantOutputSlug := "tenant1"
520+
521+
inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName)
522+
523+
expectedTenant := &tenancy.TenancyTenantsListOK{
524+
Payload: &tenancy.TenancyTenantsListOKBody{
525+
Results: []*netboxModels.Tenant{
526+
{
527+
ID: tenantId,
528+
Name: &tenantName,
529+
Slug: &tenantOutputSlug,
530+
},
531+
},
532+
},
533+
}
534+
535+
// expected error
536+
expectedErrorMsg := "failed to fetch tenant: not found"
537+
538+
// empty tenant list
539+
emptyTenantList := &tenancy.TenancyTenantsListOK{
540+
Payload: &tenancy.TenancyTenantsListOKBody{
541+
Results: []*netboxModels.Tenant{},
542+
},
543+
}
544+
545+
// non-existing tenant
546+
nonExistingTenant := "non-existing-tenant"
547+
548+
parentPrefix := "10.112.140.0/24"
549+
550+
mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(expectedTenant, nil).AnyTimes()
551+
mockTenancy.EXPECT().TenancyTenantsList(gomock.Not(inputTenant), nil).Return(emptyTenantList, nil).AnyTimes()
552+
553+
netboxClient := &NetboxClient{
554+
Ipam: mockPrefixIpam,
555+
Tenancy: mockTenancy,
556+
}
557+
558+
prefix, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
559+
ParentPrefix: parentPrefix,
560+
PrefixLength: "/28.",
561+
Metadata: &models.NetboxMetadata{
562+
Tenant: nonExistingTenant,
563+
},
564+
})
565+
566+
assert.EqualErrorf(t, err, expectedErrorMsg, "Error should be: %v, got: %v", expectedErrorMsg, err)
567+
assert.Equal(t, prefix, (*models.Prefix)(nil))
568+
}
569+
570+
func TestPrefixClaim_GetNoAvailablePrefixesWithErrorWhenGettingTenantList(t *testing.T) {
571+
ctrl := gomock.NewController(t)
572+
defer ctrl.Finish()
573+
mockPrefixIpam := mock_interfaces.NewMockIpamInterface(ctrl)
574+
mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl)
575+
576+
// non-existing tenant
577+
tenantName := "Tenant1"
578+
579+
inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName)
580+
581+
// expected errors
582+
getTenantDetailsErrorMsg := "failed to fetch Tenant details"
583+
tenancyTenantsListErrorMsg := "cannot get the list" // testcase defined error
584+
585+
parentPrefix := "10.112.140.0/24"
586+
587+
mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(nil, errors.New(tenancyTenantsListErrorMsg)).AnyTimes()
588+
589+
netboxClient := &NetboxClient{
590+
Ipam: mockPrefixIpam,
591+
Tenancy: mockTenancy,
592+
}
593+
594+
prefix, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
595+
ParentPrefix: parentPrefix,
596+
PrefixLength: "/28.",
597+
Metadata: &models.NetboxMetadata{
598+
Tenant: tenantName,
599+
},
600+
})
601+
602+
// assert 1st level error - GetTenantDetails()
603+
assert.Containsf(t, err.Error(), getTenantDetailsErrorMsg, "expected error containing %q, got %s", getTenantDetailsErrorMsg, err)
604+
605+
// assert 2nd level error - TenanyTenantsList()
606+
assert.Containsf(t, err.Error(), tenancyTenantsListErrorMsg, "expected error containing %q, got %s", tenancyTenantsListErrorMsg, err)
607+
608+
// assert nil output
609+
assert.Equal(t, prefix, (*models.Prefix)(nil))
610+
}

‎pkg/netbox/api/tenancy.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ limitations under the License.
1717
package api
1818

1919
import (
20-
"errors"
21-
2220
"github.com/netbox-community/go-netbox/v3/netbox/client/tenancy"
2321

2422
"github.com/netbox-community/netbox-operator/pkg/netbox/models"
@@ -32,7 +30,7 @@ func (r *NetboxClient) GetTenantDetails(name string) (*models.Tenant, error) {
3230
return nil, utils.NetboxError("failed to fetch Tenant details", err)
3331
}
3432
if len(response.Payload.Results) == 0 {
35-
return nil, errors.New("tenant not found")
33+
return nil, utils.NetboxNotFoundError("tenant")
3634
}
3735

3836
return &models.Tenant{

‎pkg/netbox/api/tenancy_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,5 @@ func TestTenancy_GetWrongTenantDetails(t *testing.T) {
8080

8181
actual, err := netboxClient.GetTenantDetails(wrongTenant)
8282
assert.Nil(t, actual)
83-
assert.EqualError(t, err, "tenant not found")
83+
assert.EqualError(t, err, "failed to fetch tenant: not found")
8484
}

‎pkg/netbox/utils/error.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,17 @@ limitations under the License.
1717
package utils
1818

1919
import (
20+
"fmt"
21+
2022
"github.com/pkg/errors"
2123
)
2224

25+
var ErrNotFound = errors.New("not found")
26+
2327
func NetboxError(message string, err error) error {
28+
return fmt.Errorf(message+": %w", err)
29+
}
2430

25-
return errors.Errorf(message, err.Error())
31+
func NetboxNotFoundError(name string) error {
32+
return NetboxError("failed to fetch "+name, ErrNotFound)
2633
}

0 commit comments

Comments
 (0)