From b93b73508b8c1acd39dc6d91bc8a826619d15256 Mon Sep 17 00:00:00 2001 From: Ludwig Date: Wed, 3 Jul 2024 12:59:30 +0200 Subject: [PATCH] add more tests and fix code path --- api/v1alpha1/ionoscloudcluster_types_test.go | 6 +++ internal/service/cloud/ipblock.go | 8 ++-- internal/service/cloud/ipblock_test.go | 18 ++++++++ internal/service/cloud/network_test.go | 45 +++++++++++++++++--- internal/service/cloud/suite_test.go | 42 +++++++++++++++++- 5 files changed, 107 insertions(+), 12 deletions(-) diff --git a/api/v1alpha1/ionoscloudcluster_types_test.go b/api/v1alpha1/ionoscloudcluster_types_test.go index 447a6407..976a23e7 100644 --- a/api/v1alpha1/ionoscloudcluster_types_test.go +++ b/api/v1alpha1/ionoscloudcluster_types_test.go @@ -83,6 +83,12 @@ var _ = Describe("IonosCloudCluster", func() { Expect(k8sClient.Create(context.Background(), cluster)). Should(MatchError(ContainSubstring("credentialsRef.name must be provided"))) }) + It("should not allow creating clusters with empty location", func() { + cluster := defaultCluster() + cluster.Spec.Location = "" + Expect(k8sClient.Create(context.Background(), cluster)). + Should(MatchError(ContainSubstring(" spec.location in body should be at least 1 chars long"))) + }) }) Context("Update", func() { diff --git a/internal/service/cloud/ipblock.go b/internal/service/cloud/ipblock.go index 6a2ca3a4..44b887d0 100644 --- a/internal/service/cloud/ipblock.go +++ b/internal/service/cloud/ipblock.go @@ -100,6 +100,10 @@ func (s *Service) ReconcileControlPlaneEndpointDeletion( s.getControlPlaneEndpointIPBlock, s.getLatestControlPlaneEndpointIPBlockCreationRequest, ) + + if ignoreErrUserSetIPNotFound(ignoreNotFound(err)) != nil { + return false, err + } // NOTE(gfariasalves): we ignore the error if it is a "user set IP not found" error, because it doesn't matter here. // This error is only relevant when we are trying to create a new IP block. If it shows up here, it means that: // a) this IP block was created by the user, and they have deleted it, or, @@ -111,10 +115,6 @@ func (s *Service) ReconcileControlPlaneEndpointDeletion( return false, nil } - if ignoreErrUserSetIPNotFound(ignoreNotFound(err)) != nil { - return false, err - } - // NOTE: this check covers the case where customers have set the control plane endpoint IP themselves. // If this is the case we don't request for the deletion of the IP block. if ipBlock != nil && ptr.Deref(ipBlock.GetProperties().GetName(), unknownValue) != s.controlPlaneEndpointIPBlockName(cs) { diff --git a/internal/service/cloud/ipblock_test.go b/internal/service/cloud/ipblock_test.go index 32e81b44..b2f6a43d 100644 --- a/internal/service/cloud/ipblock_test.go +++ b/internal/service/cloud/ipblock_test.go @@ -383,6 +383,18 @@ func (s *ipBlockTestSuite) TestReconcileControlPlaneEndpointDeletionRequestNewDe s.Equal(exampleRequestPath, s.clusterScope.IonosCluster.Status.CurrentClusterRequest.RequestPath) } +func (s *ipBlockTestSuite) TestReconcileControlPlaneEndpointDeletionErrorOnDuplicateIPBlock() { + block := exampleIPBlock() + dup := exampleIPBlock() + s.mockListIPBlocksCall().Return(&sdk.IpBlocks{Items: &[]sdk.IpBlock{ + *block, + *dup, + }}, nil).Once() + requeue, err := s.service.ReconcileControlPlaneEndpointDeletion(s.ctx, s.clusterScope) + s.False(requeue) + s.Error(err) +} + func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletion() { s.infraMachine.Spec.FailoverIP = ptr.To(infrav1.CloudResourceConfigAuto) ipBlock := exampleIPBlockWithName(s.service.failoverIPBlockName(s.machineScope)) @@ -472,6 +484,12 @@ func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletionDeletionFinished( s.Nil(s.machineScope.IonosMachine.Status.CurrentRequest) } +func (s *ipBlockTestSuite) TestReconcileFailoverIPBlockDeletionShouldSkipDeletion() { + requeue, err := s.service.ReconcileFailoverIPBlockDeletion(s.ctx, s.machineScope) + s.NoError(err) + s.False(requeue) + s.Nil(s.machineScope.IonosMachine.Status.CurrentRequest) +} func (s *ipBlockTestSuite) mockDeleteIPBlockCall() *clienttest.MockClient_DeleteIPBlock_Call { return s.ionosClient.EXPECT().DeleteIPBlock(s.ctx, exampleIPBlockID) } diff --git a/internal/service/cloud/network_test.go b/internal/service/cloud/network_test.go index 4e61d5c6..39ec106c 100644 --- a/internal/service/cloud/network_test.go +++ b/internal/service/cloud/network_test.go @@ -371,7 +371,7 @@ func setControlPlaneLabel(ctx context.Context, k8sClient client.Client, machine return k8sClient.Update(ctx, machine) } -func (s *lanSuite) reconcileIPFailoverDeletion(testServer *sdk.Server, testLAN sdk.Lan) { +func (s *lanSuite) reconcileIPFailoverDeletion(testServer *sdk.Server, testLAN sdk.Lan) (requeue bool, err error) { s.mockGetServerCall(exampleServerID).Return(testServer, nil).Once() s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{testLAN}}, nil).Once() s.setupSuccessfulLANPatchMocks() @@ -384,9 +384,7 @@ func (s *lanSuite) reconcileIPFailoverDeletion(testServer *sdk.Server, testLAN s } s.mockPatchLANCall(props).Return(exampleRequestPath, nil).Once() - s.assertSuccessfulDeletion( - s.service.ReconcileIPFailoverDeletion(s.ctx, s.machineScope), - ) + return s.service.ReconcileIPFailoverDeletion(s.ctx, s.machineScope) } func (s *lanSuite) TestReconcileIPFailoverDeletionWorker() { @@ -409,7 +407,7 @@ func (s *lanSuite) TestReconcileIPFailoverDeletionWorker() { NicUuid: ptr.To(exampleNICID), }} - s.reconcileIPFailoverDeletion(testServer, testLAN) + s.assertSuccessfulDeletion(s.reconcileIPFailoverDeletion(testServer, testLAN)) } func (s *lanSuite) TestReconcileIPFailoverDeletionControlPlane() { @@ -430,7 +428,7 @@ func (s *lanSuite) TestReconcileIPFailoverDeletionControlPlane() { NicUuid: ptr.To(exampleNICID), }} - s.reconcileIPFailoverDeletion(testServer, testLAN) + s.assertSuccessfulDeletion(s.reconcileIPFailoverDeletion(testServer, testLAN)) } func (s *lanSuite) TestReconcileIPFailoverDeletionControlPlaneSwitchNIC() { @@ -484,6 +482,41 @@ func (s *lanSuite) TestReconcileIPFailoverDeletionControlPlaneSwitchNIC() { ) } +func (s *lanSuite) TestReconcileFailoverDeletionWorkerNoSwap() { + const deploymentLabel = "test-deployment" + labels := s.infraMachine.GetLabels() + labels[clusterv1.MachineDeploymentNameLabel] = deploymentLabel + s.infraMachine.SetLabels(labels) + + s.infraMachine.Spec.FailoverIP = ptr.To(exampleWorkerFailoverIP) + + secondaryMachine := s.infraMachine.DeepCopy() + secondaryMachine.SetName("test-machine-2") + secondaryMachine.SetResourceVersion("") + secondaryMachine.SetCreationTimestamp(metav1.NewTime(time.Now())) + testServer := s.defaultServer(s.infraMachine, exampleDHCPIP, exampleWorkerFailoverIP) + + s.NoError(s.k8sClient.Create(s.ctx, secondaryMachine)) + s.NoError(s.k8sClient.Update(s.ctx, s.infraMachine)) + + testLAN := s.exampleLAN() + testLAN.Properties.IpFailover = &[]sdk.IPFailover{{ + Ip: ptr.To(exampleArbitraryIP), + NicUuid: ptr.To(arbitraryNICID), + }, { + Ip: ptr.To(exampleWorkerFailoverIP), + NicUuid: ptr.To(exampleSecondaryNICID), + }} + + s.mockGetServerCall(exampleServerID).Return(testServer, nil).Once() + s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{testLAN}}, nil).Once() + s.mockGetLANPatchRequestCall().Return(nil, nil).Once() + requeue, err := s.service.ReconcileIPFailoverDeletion(s.ctx, s.machineScope) + + s.NoError(err) + s.False(requeue) +} + func (s *lanSuite) setupSuccessfulLANPatchMocks() { s.T().Helper() patchRequest := s.examplePatchRequest(sdk.RequestStatusDone) diff --git a/internal/service/cloud/suite_test.go b/internal/service/cloud/suite_test.go index cc6fdb8a..6f8ffca4 100644 --- a/internal/service/cloud/suite_test.go +++ b/internal/service/cloud/suite_test.go @@ -19,6 +19,7 @@ package cloud import ( "context" "fmt" + "github.com/google/uuid" "net/http" "path" "strconv" @@ -137,7 +138,7 @@ func (s *ServiceTestSuite) SetupTest() { Spec: clusterv1.MachineSpec{ ClusterName: s.capiCluster.Name, Version: ptr.To("v1.26.12"), - ProviderID: ptr.To("ionos://dd426c63-cd1d-4c02-aca3-13b4a27c2ebf"), + ProviderID: ptr.To("ionos://" + exampleServerID), }, } s.infraMachine = &infrav1.IonosCloudMachine{ @@ -150,7 +151,7 @@ func (s *ServiceTestSuite) SetupTest() { }, }, Spec: infrav1.IonosCloudMachineSpec{ - ProviderID: ptr.To("ionos://dd426c63-cd1d-4c02-aca3-13b4a27c2ebf"), + ProviderID: ptr.To("ionos://" + exampleServerID), DatacenterID: "ccf27092-34e8-499e-a2f5-2bdee9d34a12", NumCores: 2, AvailabilityZone: infrav1.AvailabilityZoneAuto, @@ -339,6 +340,39 @@ func (s *ServiceTestSuite) defaultServerComponents() (sdk.ServerProperties, sdk. return props, entities } +func (s *ServiceTestSuite) newIonosCloudMachine(name string) *infrav1.IonosCloudMachine { + providerID := uuid.New().String() + return &infrav1.IonosCloudMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: name, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: s.capiCluster.Name, + clusterv1.MachineDeploymentNameLabel: "test-md", + }, + }, + Spec: infrav1.IonosCloudMachineSpec{ + ProviderID: ptr.To("ionos://" + providerID), + DatacenterID: "ccf27092-34e8-499e-a2f5-2bdee9d34a12", + NumCores: 2, + AvailabilityZone: infrav1.AvailabilityZoneAuto, + MemoryMB: 4096, + CPUFamily: ptr.To("AMD_OPTERON"), + Disk: &infrav1.Volume{ + Name: name + "-hdd", + DiskType: infrav1.VolumeDiskTypeHDD, + SizeGB: 20, + AvailabilityZone: infrav1.AvailabilityZoneAuto, + Image: &infrav1.ImageSpec{ + ID: "3e3e3e3e-3e3e-3e3e-3e3e-3e3e3e3e3e3e", + }, + }, + Type: infrav1.ServerTypeEnterprise, + }, + Status: infrav1.IonosCloudMachineStatus{}, + } +} + func (s *ServiceTestSuite) mockGetIPBlocksRequestsPostCall() *clienttest.MockClient_GetRequests_Call { return s.ionosClient.EXPECT().GetRequests(s.ctx, http.MethodPost, ipBlocksPath) } @@ -370,3 +404,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()) } + +type ionosCloudMachineParams struct { + name string +}