From 5c01e3706ac80889d00e885b3abd37ffa17b585c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radek=20Ma=C5=88=C3=A1k?= Date: Mon, 8 Jan 2024 15:37:09 +0100 Subject: [PATCH 1/2] Don't create availability set when using spot instances --- .../azure/actuators/machine/reconciler.go | 5 +++++ .../azure/actuators/machine/reconciler_test.go | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/pkg/cloud/azure/actuators/machine/reconciler.go b/pkg/cloud/azure/actuators/machine/reconciler.go index fc44b5fd9..22c565a6d 100644 --- a/pkg/cloud/azure/actuators/machine/reconciler.go +++ b/pkg/cloud/azure/actuators/machine/reconciler.go @@ -788,6 +788,11 @@ func (s *Reconciler) getOrCreateAvailabilitySet() (string, error) { return "", nil } + if s.scope.MachineConfig.SpotVMOptions != nil { + klog.V(4).Infof("MachineSet %s uses spot instances, skipping availability set creation", s.scope.Machine.Name) + return "", nil + } + klog.V(4).Infof("No availability zones were found for %s, an availability set will be created", s.scope.Machine.Name) if err := s.availabilitySetsSvc.CreateOrUpdate(context.Background(), &availabilitysets.Spec{ diff --git a/pkg/cloud/azure/actuators/machine/reconciler_test.go b/pkg/cloud/azure/actuators/machine/reconciler_test.go index 414ebd716..a29d5081a 100644 --- a/pkg/cloud/azure/actuators/machine/reconciler_test.go +++ b/pkg/cloud/azure/actuators/machine/reconciler_test.go @@ -333,6 +333,7 @@ func TestCreateAvailabilitySet(t *testing.T) { availabilitySetsSvc func() *mock_azure.MockService availabilityZonesSvc func() *mock_azure.MockService inputASName string + spotVMOptions *machinev1.SpotVMOptions }{ { name: "Error when availability zones client fails", @@ -414,6 +415,22 @@ func TestCreateAvailabilitySet(t *testing.T) { return availabilitySetsSvc }, }, + { + name: "Skip availability set creation when using Spot instances", + availabilityZonesSvc: func() *mock_azure.MockService { + availabilityZonesSvc := mock_azure.NewMockService(mockCtrl) + availabilityZonesSvc.EXPECT().Get(gomock.Any(), gomock.Any()).Return([]string{}, nil).Times(1) + return availabilityZonesSvc + }, + availabilitySetsSvc: func() *mock_azure.MockService { + availabilitySetsSvc := mock_azure.NewMockService(mockCtrl) + availabilitySetsSvc.EXPECT().CreateOrUpdate(gomock.Any(), gomock.Any()).Return(nil).Times(0) + return availabilitySetsSvc + }, + spotVMOptions: &machinev1.SpotVMOptions{ + MaxPrice: resource.NewQuantity(-1, resource.DecimalSI), + }, + }, { name: "Skip availability set creation when name was specified in provider spec", labels: map[string]string{}, @@ -486,6 +503,7 @@ func TestCreateAvailabilitySet(t *testing.T) { MachineConfig: &machinev1.AzureMachineProviderSpec{ VMSize: "Standard_D2_v2", AvailabilitySet: tc.inputASName, + SpotVMOptions: tc.spotVMOptions, }, }, } From 0fa855edd931c1e15f31cad1d44d53e39efddcea Mon Sep 17 00:00:00 2001 From: Michail Resvanis Date: Tue, 11 Jul 2023 13:29:44 +0200 Subject: [PATCH 2/2] Fix termination tests randomly timing out Unit tests are run in random order. Termination tests also verify the polling logic of the respective code, by atomically incrementing a counter in each call of the HTTP handler. The issues fixed with these changes are: 1. counter var not being reset after each termination test using polling 2. using the polling iterations loop in termination tests that were not using the HTTP handler that increments the counter 3. polling iterations loop not using sleep between iterations, which spikes CPU usage The first two issues led to the termination tests sometimes passing, depending on which termination test run first. If it was a test that incremented the polling counter, then all other test would be successful as well. If it was a test that didn't increment the polling counter, then the termination tests would be stuck in an endless loop, until the unit tests reached a timeout. Signed-off-by: Michail Resvanis --- pkg/termination/termination_test.go | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/pkg/termination/termination_test.go b/pkg/termination/termination_test.go index 2e33f9e58..7007fa6e3 100644 --- a/pkg/termination/termination_test.go +++ b/pkg/termination/termination_test.go @@ -124,6 +124,8 @@ var _ = Describe("Handler Suite", func() { var counter int32 BeforeEach(func() { + counter = 0 + // Ensure the polling logic is excercised in tests httpHandler = newMockHTTPHandler(func(rw http.ResponseWriter, req *http.Request) { if atomic.LoadInt32(&counter) == 4 { @@ -135,15 +137,15 @@ var _ = Describe("Handler Suite", func() { }) }) - JustBeforeEach(func() { - // Ensure the polling logic is excercised in tests - for atomic.LoadInt32(&counter) < 4 { - continue - } - }) - Context("and the handler is stopped", func() { JustBeforeEach(func() { + // Ensure the polling logic is tested as well + for atomic.LoadInt32(&counter) < 4 { + // use 50ms since polling is set to 100ms + time.Sleep(50 * time.Millisecond) + continue + } + close(stop) }) @@ -157,6 +159,15 @@ var _ = Describe("Handler Suite", func() { }) Context("and the instance termination notice is fulfilled", func() { + JustBeforeEach(func() { + // Ensure the polling logic is tested as well + for atomic.LoadInt32(&counter) < 4 { + // use 50ms since polling is set to 100ms + time.Sleep(50 * time.Millisecond) + continue + } + }) + It("should mark the node for deletion", func() { Eventually(nodeMarkedForDeletion(testNode.Name)).Should(BeTrue()) })