Skip to content

Commit

Permalink
✨ Handle IP blocks based on VDC location (#172)
Browse files Browse the repository at this point in the history
**What is the purpose of this pull request/Why do we need it?**
If you specify a different VDC ID in the IonosCloudMachine spec, IP
blocks are now correctly booked in this location.

**Issue #, if available:**
#154

**Description of changes:**
* IP blocks are booked in the location specified in the
IonosCloudMachine spec
* Location is written into the Machine status

**Special notes for your reviewer:**

**Checklist:**
- [ ] Documentation updated
- [x] Unit Tests added
- [ ] E2E Tests added
- [x] Includes
[emojis](https://github.com/kubernetes-sigs/kubebuilder-release-tools?tab=readme-ov-file#kubebuilder-project-versioning)
  • Loading branch information
jriedel-ionos authored Jul 15, 2024
1 parent de3f46e commit d6ac397
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 3 deletions.
4 changes: 4 additions & 0 deletions api/v1alpha1/ionoscloudmachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,10 @@ type IonosCloudMachineStatus struct {
// cloud resource that is being provisioned.
//+optional
CurrentRequest *ProvisioningRequest `json:"currentRequest,omitempty"`

// Location is the location of the datacenter the VM is provisioned in.
//+optional
Location string `json:"location"`
}

// MachineNetworkInfo contains information about the network configuration of the VM.
Expand Down
1 change: 1 addition & 0 deletions api/v1alpha1/ionoscloudmachine_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ var _ = Describe("IonosCloudMachine Tests", func() {
}
m.Status.FailureReason = ptr.To(errors.InvalidConfigurationMachineError)
m.Status.FailureMessage = ptr.To("Failure message")
m.Status.Location = "de/fra"

m.Status.MachineNetworkInfo = &MachineNetworkInfo{
NICInfo: []NICInfo{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ spec:
can be added as events to the IonosCloudMachine object and/or logged in the
controller's output.
type: string
location:
description: Location is the location of the datacenter the VM is
provisioned in.
type: string
machineNetworkInfo:
description: |-
MachineNetworkInfo contains information about the network configuration of the VM.
Expand Down
2 changes: 2 additions & 0 deletions internal/ionoscloud/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,6 @@ type Client interface {
GetRequests(ctx context.Context, method, path string) ([]sdk.Request, error)
// PatchNIC updates the NIC identified by nicID with the provided properties, returning the request location.
PatchNIC(ctx context.Context, datacenterID, serverID, nicID string, properties sdk.NicProperties) (string, error)
// GetDatacenterLocationByID returns the location of the data center identified by datacenterID.
GetDatacenterLocationByID(ctx context.Context, datacenterID string) (string, error)
}
14 changes: 14 additions & 0 deletions internal/ionoscloud/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,3 +463,17 @@ func validateNICParameters(datacenterID, serverID, nicID string) (err error) {

return nil
}

// GetDatacenterLocationByID returns the location of the data center identified by datacenterID.
func (c *IonosCloudClient) GetDatacenterLocationByID(ctx context.Context, datacenterID string) (string, error) {
if datacenterID == "" {
return "", errDatacenterIDIsEmpty
}

datacenter, _, err := c.API.DataCentersApi.DatacentersFindById(ctx, datacenterID).Execute()
if err != nil {
return "", fmt.Errorf(apiCallErrWrapper, err)
}

return *datacenter.Properties.Location, nil
}
57 changes: 57 additions & 0 deletions internal/ionoscloud/clienttest/mock_client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 33 additions & 3 deletions internal/service/cloud/ipblock.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,14 @@ func (s *Service) getFailoverIPBlock(ctx context.Context, ms *scope.Machine) (*s
return nil, fmt.Errorf("failed to list IP blocks: %w", err)
}

location, err := s.getLocation(ctx, ms)
if err != nil {
return nil, err
}

for _, block := range ptr.Deref(blocks.GetItems(), nil) {
props := block.GetProperties()
if ptr.Deref(props.GetLocation(), "") != ms.ClusterScope.Location() {
if ptr.Deref(props.GetLocation(), "") != location {
continue
}
if ptr.Deref(props.GetName(), "") == s.failoverIPBlockName(ms) {
Expand Down Expand Up @@ -334,9 +339,16 @@ func (s *Service) reserveControlPlaneEndpointIPBlock(ctx context.Context, cs *sc

func (s *Service) reserveMachineDeploymentFailoverIPBlock(ctx context.Context, ms *scope.Machine) error {
log := s.logger.WithName("reserveMachineDeploymentFailoverIPBlock")
location, err := s.getLocation(ctx, ms)
if err != nil {
return err
}

ms.IonosMachine.Status.Location = location

return s.reserveIPBlock(
ctx, s.failoverIPBlockName(ms),
ms.ClusterScope.Location(), log,
location, log,
ms.IonosMachine.SetCurrentRequest,
)
}
Expand Down Expand Up @@ -401,10 +413,14 @@ func (s *Service) getLatestControlPlaneEndpointIPBlockCreationRequest(

// getLatestFailoverIPBlockCreateRequest returns the latest failover IP block creation request.
func (s *Service) getLatestFailoverIPBlockCreateRequest(ctx context.Context, ms *scope.Machine) (*requestInfo, error) {
location, err := s.getLocation(ctx, ms)
if err != nil {
return nil, err
}
return s.getLatestIPBlockRequestByNameAndLocation(
ctx, http.MethodPost,
s.failoverIPBlockName(ms),
ms.ClusterScope.Location(),
location,
)
}

Expand Down Expand Up @@ -460,3 +476,17 @@ func ignoreErrUserSetIPNotFound(err error) error {
}
return err
}

// getLocation checks if the location of the machine is already set in the status.
// If not, it queries Cloud API to get the location of the datacenter.
func (s *Service) getLocation(ctx context.Context, ms *scope.Machine) (string, error) {
if ms.IonosMachine.Status.Location == "" {
location, err := s.ionosClient.GetDatacenterLocationByID(ctx, ms.IonosMachine.Spec.DatacenterID)
if err != nil {
return "", fmt.Errorf("failed to get location of datacenter: %w", err)
}
ms.IonosMachine.Status.Location = location
return location, nil
}
return ms.IonosMachine.Status.Location, nil
}
7 changes: 7 additions & 0 deletions internal/service/cloud/ipblock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ func (s *ipBlockTestSuite) TestReserveMachineDeploymentIPBlockRequestSuccess() {
}
labels[clusterv1.MachineDeploymentNameLabel] = "test-deployment"

s.mockGetDatacenterLocationByIDCall(exampleDatacenterID).Return(exampleLocation, nil).Once()
s.mockReserveIPBlockCall(
s.service.failoverIPBlockName(s.machineScope),
s.clusterScope.Location(),
Expand All @@ -178,6 +179,7 @@ func (s *ipBlockTestSuite) TestReserveMachineDeploymentIPBlockRequestSuccess() {
s.NoError(err)
req := s.machineScope.IonosMachine.Status.CurrentRequest
s.validateRequest(http.MethodPost, sdk.RequestStatusQueued, requestPath, req)
s.Equal(exampleLocation, s.machineScope.IonosMachine.Status.Location)
}

func (s *ipBlockTestSuite) validateRequest(method, state, path string, req *infrav1.ProvisioningRequest) {
Expand Down Expand Up @@ -399,6 +401,7 @@ func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletion() {
s.infraMachine.Spec.FailoverIP = ptr.To(infrav1.CloudResourceConfigAuto)
ipBlock := exampleIPBlockWithName(s.service.failoverIPBlockName(s.machineScope))

s.mockGetDatacenterLocationByIDCall(exampleDatacenterID).Return(exampleLocation, nil).Once()
s.mockListIPBlocksCall().Return(&sdk.IpBlocks{Items: &[]sdk.IpBlock{*ipBlock}}, nil).Once()
s.mockGetIPBlocksRequestsDeleteCall(exampleIPBlockID).Return(nil, nil).Once()
s.mockDeleteIPBlockCall().Return(exampleRequestPath, nil).Once()
Expand All @@ -418,6 +421,7 @@ func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletionSkipped() {
NicUuid: nil,
}}

s.mockGetDatacenterLocationByIDCall(exampleDatacenterID).Return(exampleLocation, nil).Once()
s.mockListIPBlocksCall().Return(&sdk.IpBlocks{Items: &[]sdk.IpBlock{*ipBlock}}, nil).Once()
s.mockGetIPBlocksRequestsDeleteCall(exampleIPBlockID).Return(nil, nil).Once()
s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{lan}}, nil).Once()
Expand All @@ -430,6 +434,7 @@ func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletionSkipped() {
func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletionPendingCreation() {
s.infraMachine.Spec.FailoverIP = ptr.To(infrav1.CloudResourceConfigAuto)

s.mockGetDatacenterLocationByIDCall(exampleDatacenterID).Return(exampleLocation, nil).Once()
s.mockListIPBlocksCall().Return(nil, nil).Once()
s.mockGetIPBlocksRequestsPostCall().Return([]sdk.Request{
s.buildIPBlockRequestWithName(
Expand All @@ -446,6 +451,7 @@ func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletionPendingDeletion()
s.infraMachine.Spec.FailoverIP = ptr.To(infrav1.CloudResourceConfigAuto)
ipBlock := exampleIPBlockWithName(s.service.failoverIPBlockName(s.machineScope))

s.mockGetDatacenterLocationByIDCall(exampleDatacenterID).Return(exampleLocation, nil).Once()
s.mockListIPBlocksCall().Return(&sdk.IpBlocks{Items: &[]sdk.IpBlock{*ipBlock}}, nil).Once()

deleteRequest := s.buildIPBlockRequestWithName(
Expand Down Expand Up @@ -473,6 +479,7 @@ func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletionDeletionFinished(
s.infraMachine.Spec.FailoverIP = ptr.To(infrav1.CloudResourceConfigAuto)
ipBlock := exampleIPBlockWithName(s.service.failoverIPBlockName(s.machineScope))

s.mockGetDatacenterLocationByIDCall(exampleDatacenterID).Return(exampleLocation, nil).Once()
s.mockListIPBlocksCall().Return(&sdk.IpBlocks{Items: &[]sdk.IpBlock{*ipBlock}}, nil).Once()

deleteRequest := s.buildIPBlockRequestWithName("", sdk.RequestStatusDone, http.MethodDelete, exampleIPBlockID)
Expand Down
2 changes: 2 additions & 0 deletions internal/service/cloud/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ func (s *lanSuite) TestReconcileIPFailoverForWorkerWithAUTOSettings() {
s.infraMachine.Spec.FailoverIP = ptr.To(infrav1.CloudResourceConfigAuto)
testServer := s.defaultServer(s.infraMachine, exampleDHCPIP, exampleWorkerFailoverIP)
testLAN := s.exampleLAN()
s.mockGetDatacenterLocationByIDCall(exampleDatacenterID).Return(exampleLocation, nil)

ipBlock := s.exampleIPBlock()

Expand Down Expand Up @@ -285,6 +286,7 @@ func (s *lanSuite) TestReconcileIPFailoverReserveIPBlock() {
s.infraMachine.SetLabels(map[string]string{clusterv1.MachineDeploymentNameLabel: deploymentLabel})
s.infraMachine.Spec.FailoverIP = ptr.To(infrav1.CloudResourceConfigAuto)

s.mockGetDatacenterLocationByIDCall(exampleDatacenterID).Return(exampleLocation, nil).Once()
s.mockListIPBlocksCall().Return(nil, nil).Once()
s.mockGetIPBlocksRequestsPostCall().Return(nil, nil).Once()
s.mockReserveIPBlockCall(s.service.failoverIPBlockName(s.machineScope), s.clusterScope.Location()).
Expand Down
5 changes: 5 additions & 0 deletions internal/service/cloud/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const (
exampleSecondaryServerID = "dd426c63-cd1d-4c02-aca3-13b4a27c2ebd"
exampleRequestPath = "/test"
exampleLocation = "de/txl"
exampleDatacenterID = "ccf27092-34e8-499e-a2f5-2bdee9d34a12"
)

type ServiceTestSuite struct {
Expand Down Expand Up @@ -370,3 +371,7 @@ func (s *ServiceTestSuite) mockGetServerCall(serverID string) *clienttest.MockCl
func (s *ServiceTestSuite) mockListLANsCall() *clienttest.MockClient_ListLANs_Call {
return s.ionosClient.EXPECT().ListLANs(s.ctx, s.machineScope.DatacenterID())
}

func (s *ServiceTestSuite) mockGetDatacenterLocationByIDCall(datacenterID string) *clienttest.MockClient_GetDatacenterLocationByID_Call {
return s.ionosClient.EXPECT().GetDatacenterLocationByID(s.ctx, datacenterID)
}

0 comments on commit d6ac397

Please sign in to comment.