Merge pull request #105 from openshift-cherrypick-robot/cherry-pick-1…

…04-to-release-4.12

[release-4.12] OCPBUGS-30809: Don't create availability set when using spot instances

pkg/cloud/azure/actuators/machine/reconciler.go

@@ -783,6 +783,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{

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,
 					},
 				},
 			}

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())
 			})