Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check for tenant existence #72

Merged
merged 1 commit into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/controller/expected_netboxmock_calls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func mockTenancyTenancyTenantsList(tenancyMock *mock_interfaces.MockTenancyInter
}

// -----------------------------
// Restet Mock Functions
// Reset Mock Functions
// -----------------------------

func resetMockFunctions(ipamMockA *mock_interfaces.MockIpamInterface, ipamMockB *mock_interfaces.MockIpamInterface, tenancyMock *mock_interfaces.MockTenancyInterface) {
Expand Down
9 changes: 7 additions & 2 deletions internal/controller/ipaddressclaim_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,14 @@ func (r *IpAddressClaimReconciler) Reconcile(ctx context.Context, req ctrl.Reque
},
})
if err != nil {
return ctrl.Result{}, err
setConditionErr := r.SetConditionAndCreateEvent(ctx, o, netboxv1.ConditionIpAssignedFalse, corev1.EventTypeWarning, err.Error())
if setConditionErr != nil {
return ctrl.Result{}, fmt.Errorf("error updating status: %w, when getting an available IP address failed: %w", setConditionErr, err)
}

return ctrl.Result{Requeue: true}, nil
}
debugLogger.Info(fmt.Sprintf("ip address is not reserved in netbox, assignined new ip address: %s", ipAddressModel.IpAddress))
debugLogger.Info(fmt.Sprintf("ip address is not reserved in netbox, assigned new ip address: %s", ipAddressModel.IpAddress))
} else {
// 5.b reassign reserved ip address from netbox
// do nothing, ip address restored
Expand Down
7 changes: 6 additions & 1 deletion internal/controller/prefixclaim_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@ func (r *PrefixClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
},
})
if err != nil {
return ctrl.Result{}, err
setConditionErr := r.SetConditionAndCreateEvent(ctx, prefixClaim, netboxv1.ConditionPrefixAssignedFalse, corev1.EventTypeWarning, err.Error())
if setConditionErr != nil {
return ctrl.Result{}, fmt.Errorf("error updating status: %w, when getting an available Prefix failed: %w", setConditionErr, err)
}

return ctrl.Result{Requeue: true}, nil
}
debugLogger.Info(fmt.Sprintf("prefix is not reserved in netbox, assignined new prefix: %s", prefixModel.Prefix))
} else {
Expand Down
7 changes: 6 additions & 1 deletion pkg/netbox/api/ip_address_claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

"github.com/netbox-community/go-netbox/v3/netbox/client/ipam"
"github.com/netbox-community/netbox-operator/pkg/netbox/models"
"github.com/netbox-community/netbox-operator/pkg/netbox/utils"
)

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

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

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

parentPrefixId := responseParentPrefix.Payload.Results[0].ID
Expand Down
80 changes: 79 additions & 1 deletion pkg/netbox/api/ip_address_claim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package api

import (
"errors"
"testing"

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

expectedErrMsg := "failed to fetch parent prefix: not found"

// assert error
AssertError(t, err, "parent prefix not found")
AssertError(t, err, expectedErrMsg)
// assert nil output
assert.Nil(t, actual)
})
Expand Down Expand Up @@ -231,3 +234,78 @@ func TestIPAddressClaim(t *testing.T) {
assert.Equal(t, ipAddressRestore, actual.IpAddress)
})
}

func TestIPAddressClaim_GetNoAvailableIPAddressWithTenancyChecks(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl)

parentPrefix := "10.112.140.0/24"
t.Run("No IP address asigned with an error when getting the tenant list", func(t *testing.T) {

tenantName := "Tenant1"

inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName)

// expected error
expectedErrorMsg := "cannot get the list" // testcase-defined error

mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(nil, errors.New(expectedErrorMsg)).AnyTimes()

// init client
client := &NetboxClient{
Tenancy: mockTenancy,
}

actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{
ParentPrefix: parentPrefix,
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})

// assert error
assert.Containsf(t, err.Error(), expectedErrorMsg, "Error should contain: %v, got: %v", expectedErrorMsg, err.Error())
// assert nil output
assert.Equal(t, actual, (*models.IPAddress)(nil))
})

t.Run("No IP address asigned with non-existing tenant", func(t *testing.T) {

// non existing tenant
nonExistingTenant := "non-existing-tenant"

inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&nonExistingTenant)

// empty tenant list
emptyTenantList := &tenancy.TenancyTenantsListOK{
Payload: &tenancy.TenancyTenantsListOKBody{
Results: []*netboxModels.Tenant{},
},
}

// expected error
expectedErrorMsg := "failed to fetch tenant: not found"

// mock empty list call
mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(emptyTenantList, nil).AnyTimes()

// init client
client := &NetboxClient{
Tenancy: mockTenancy,
}

actual, err := client.GetAvailableIpAddressByClaim(&models.IPAddressClaim{
ParentPrefix: parentPrefix,
Metadata: &models.NetboxMetadata{
Tenant: nonExistingTenant,
},
})

// assert error
assert.EqualErrorf(t, err, expectedErrorMsg, "Error should be: %v, got: %v", expectedErrorMsg, err)
// assert nil output
assert.Equal(t, actual, (*models.IPAddress)(nil))
})
}
5 changes: 5 additions & 0 deletions pkg/netbox/api/prefix_claim.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ func isRequestingTheEntireParentPrefix(prefixClaim *models.PrefixClaim) (bool, e

// GetAvailablePrefixByClaim searches an available Prefix in Netbox matching PrefixClaim requirements
func (r *NetboxClient) GetAvailablePrefixByClaim(prefixClaim *models.PrefixClaim) (*models.Prefix, error) {
_, err := r.GetTenantDetails(prefixClaim.Metadata.Tenant)
if err != nil {
return nil, err
}

responseParentPrefix, err := r.GetPrefix(&models.Prefix{
Prefix: prefixClaim.ParentPrefix,
Metadata: prefixClaim.Metadata,
Expand Down
101 changes: 101 additions & 0 deletions pkg/netbox/api/prefix_claim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestPrefixClaim_GetAvailablePrefixesByParentPrefix(t *testing.T) {
//prefix mock input
parentPrefixId := int64(3)
availablePrefixListInput := ipam.NewIpamPrefixesAvailablePrefixesListParams().WithID(parentPrefixId)

//prefix mock output
childPrefix1 := "10.112.140.0/24"
childPrefix2 := "10.120.180.0/24"
Expand Down Expand Up @@ -129,6 +130,9 @@ func TestPrefixClaim_GetAvailablePrefixByClaim_WithWrongParent(t *testing.T) {
actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
ParentPrefix: prefix,
PrefixLength: "/28",
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})
assert.Nil(t, actual)
assert.EqualError(t, err, "parent prefix not found")
Expand Down Expand Up @@ -196,6 +200,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaim(t *testing.T) {
actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
ParentPrefix: parentPrefix,
PrefixLength: "/28",
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})

assert.Nil(t, err)
Expand Down Expand Up @@ -272,6 +279,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimNoAvailablePrefixMatchesSize(t *test
actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
ParentPrefix: parentPrefix,
PrefixLength: "/28",
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})

assert.Nil(t, err)
Expand Down Expand Up @@ -340,6 +350,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimNoAvailablePrefixMatchesSizeCriteria
_, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
ParentPrefix: parentPrefix,
PrefixLength: "/28",
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})

assert.True(t, errors.Is(err, ErrNoPrefixMatchsSizeCriteria))
Expand Down Expand Up @@ -415,6 +428,9 @@ func TestPrefixClaim_GetBestFitPrefixByClaimInvalidFormatFromNetbox(t *testing.T
actual, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
ParentPrefix: parentPrefix,
PrefixLength: "/28",
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})

assert.Nil(t, err)
Expand Down Expand Up @@ -483,7 +499,92 @@ func TestPrefixClaim_GetBestFitPrefixByClaimInvalidPrefixClaim(t *testing.T) {
_, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
ParentPrefix: parentPrefix,
PrefixLength: "/28.",
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})

assert.True(t, errors.Is(err, strconv.ErrSyntax))
}

func TestPrefixClaim_GetNoAvailablePrefixesWithNonExistingTenant(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl)

// non-existing tenant
tenantName := "non-existing-tenant"

inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName)

// expected error
expectedErrorMsg := "failed to fetch tenant: not found"

// empty tenant list
emptyTenantList := &tenancy.TenancyTenantsListOK{
Payload: &tenancy.TenancyTenantsListOKBody{
Results: []*netboxModels.Tenant{},
},
}

parentPrefix := "10.112.140.0/24"

mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(emptyTenantList, nil).AnyTimes()

netboxClient := &NetboxClient{
Tenancy: mockTenancy,
}

prefix, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
ParentPrefix: parentPrefix,
PrefixLength: "/28.",
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})

assert.EqualErrorf(t, err, expectedErrorMsg, "Error should be: %v, got: %v", expectedErrorMsg, err)
assert.Equal(t, prefix, (*models.Prefix)(nil))
}

func TestPrefixClaim_GetNoAvailablePrefixesWithErrorWhenGettingTenantList(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockTenancy := mock_interfaces.NewMockTenancyInterface(ctrl)

// non-existing tenant
tenantName := "non-existing tenant"

inputTenant := tenancy.NewTenancyTenantsListParams().WithName(&tenantName)

// expected errors
getTenantDetailsErrorMsg := "failed to fetch Tenant details"
tenancyTenantsListErrorMsg := "cannot get the list" // testcase defined error

parentPrefix := "10.112.140.0/24"

mockTenancy.EXPECT().TenancyTenantsList(inputTenant, nil).Return(nil, errors.New(tenancyTenantsListErrorMsg)).AnyTimes()

netboxClient := &NetboxClient{
Tenancy: mockTenancy,
}

prefix, err := netboxClient.GetAvailablePrefixByClaim(&models.PrefixClaim{
ParentPrefix: parentPrefix,
PrefixLength: "/28.",
Metadata: &models.NetboxMetadata{
Tenant: tenantName,
},
})

// assert 1st level error - GetTenantDetails()
assert.Containsf(t, err.Error(), getTenantDetailsErrorMsg, "expected error containing %q, got %s", getTenantDetailsErrorMsg, err)

// assert 2nd level error - TenanyTenantsList()
assert.Containsf(t, err.Error(), tenancyTenantsListErrorMsg, "expected error containing %q, got %s", tenancyTenantsListErrorMsg, err)

// assert nil output
assert.Equal(t, prefix, (*models.Prefix)(nil))
}
4 changes: 1 addition & 3 deletions pkg/netbox/api/tenancy.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ limitations under the License.
package api

import (
"errors"

"github.com/netbox-community/go-netbox/v3/netbox/client/tenancy"

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

return &models.Tenant{
Expand Down
2 changes: 1 addition & 1 deletion pkg/netbox/api/tenancy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,5 @@ func TestTenancy_GetWrongTenantDetails(t *testing.T) {

actual, err := netboxClient.GetTenantDetails(wrongTenant)
assert.Nil(t, actual)
assert.EqualError(t, err, "tenant not found")
assert.EqualError(t, err, "failed to fetch tenant: not found")
}
9 changes: 8 additions & 1 deletion pkg/netbox/utils/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@ limitations under the License.
package utils

import (
"fmt"

"github.com/pkg/errors"
)

var ErrNotFound = errors.New("not found")

func NetboxError(message string, err error) error {
return fmt.Errorf(message+": %w", err)
}

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