Merge pull request #87 from JoelSpeed/max-concurrent-reconciles

OCPBUGS-23544: Improving performance of creating VMs in Azure

Author: openshift-merge-bot[bot]

cmd/manager/main.go

@@ -93,6 +93,12 @@ func main() {
 		"The duration that non-leader candidates will wait after observing a leadership renewal until attempting to acquire leadership of a led but unrenewed leader slot. This is effectively the maximum duration that a leader can be stopped before it is replaced by another candidate. This is only applicable if leader election is enabled.",
 	)
 
+	maxConcurrentReconciles := flag.Int(
+		"max-concurrent-reconciles",
+		1,
+		"Maximum number of concurrent reconciles per controller instance.",
+	)
+
 	klog.InitFlags(nil)
 	flag.Set("logtostderr", "true")
 	flag.Parse()
@@ -173,7 +179,9 @@ func main() {
 		klog.Fatal(err)
 	}
 
-	if err := machine.AddWithActuator(mgr, machineActuator); err != nil {
+	if err := machine.AddWithActuatorOpts(mgr, machineActuator, controller.Options{
+		MaxConcurrentReconciles: *maxConcurrentReconciles,
+	}); err != nil {
 		klog.Fatal(err)
 	}
 

go.mod

@@ -15,8 +15,8 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/onsi/ginkgo/v2 v2.12.1
 	github.com/onsi/gomega v1.28.0
-	github.com/openshift/api v0.0.0-20230928105710-23b54c280f99
-	github.com/openshift/machine-api-operator v0.2.1-0.20230929171041-2cc7fcf262f3
+	github.com/openshift/api v0.0.0-20231120222239-b86761094ee3
+	github.com/openshift/machine-api-operator v0.2.1-0.20240116103101-9ee88ebc3723
 	github.com/pkg/errors v0.9.1
 	github.com/spf13/cobra v1.7.0
 	golang.org/x/crypto v0.14.0
@@ -38,7 +38,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.6.0
 	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.0
 	github.com/jongio/azidext/go/azidext v0.4.0
-	github.com/openshift/client-go v0.0.0-20230926161409-848405da69e1
+	github.com/openshift/client-go v0.0.0-20231121143148-910ca30a1a9a
 	github.com/openshift/library-go v0.0.0-20230927113136-405c34317fa4
 )
 

go.sum

@@ -289,14 +289,14 @@ github.com/onsi/ginkgo/v2 v2.12.1 h1:uHNEO1RP2SpuZApSkel9nEh1/Mu+hmQe7Q+Pepg5OYA
 github.com/onsi/ginkgo/v2 v2.12.1/go.mod h1:TE309ZR8s5FsKKpuB1YAQYBzCaAfUgatB/xlT/ETL/o=
 github.com/onsi/gomega v1.28.0 h1:i2rg/p9n/UqIDAMFUJ6qIUUMcsqOuUHgbpbu235Vr1c=
 github.com/onsi/gomega v1.28.0/go.mod h1:A1H2JE76sI14WIP57LMKj7FVfCHx3g3BcZVjJG8bjX8=
-github.com/openshift/api v0.0.0-20230928105710-23b54c280f99 h1:Af0gMLdVtRwzOREh3Ghw362KXzalKQeR/DWpDEf6ZBk=
-github.com/openshift/api v0.0.0-20230928105710-23b54c280f99/go.mod h1:qNtV0315F+f8ld52TLtPvrfivZpdimOzTi3kn9IVbtU=
-github.com/openshift/client-go v0.0.0-20230926161409-848405da69e1 h1:W1N/3nVciqmjPjn2xldHjb0AwwCQzlGxLvX5BCgE8H4=
-github.com/openshift/client-go v0.0.0-20230926161409-848405da69e1/go.mod h1:ihUJrhBcYAGYQrJu/gP2OMgfVds5f5z5kbeLNBqjHLo=
+github.com/openshift/api v0.0.0-20231120222239-b86761094ee3 h1:nLhV2lbWrJ3E3hx0/97G3ZZvppC67cNwo+CLp7/PAbA=
+github.com/openshift/api v0.0.0-20231120222239-b86761094ee3/go.mod h1:qNtV0315F+f8ld52TLtPvrfivZpdimOzTi3kn9IVbtU=
+github.com/openshift/client-go v0.0.0-20231121143148-910ca30a1a9a h1:4FVrw8hz0Wb3izbf6JfOEK+pJTYpEvteRR73mCh2g/A=
+github.com/openshift/client-go v0.0.0-20231121143148-910ca30a1a9a/go.mod h1:arApQobmOjZqtxw44TwnQdUCH+t9DgZ8geYPFqksHws=
 github.com/openshift/library-go v0.0.0-20230927113136-405c34317fa4 h1:nNPH6wOCPP6XLDyHECflAlgW7xLorcUq7wl0vqyRQ34=
 github.com/openshift/library-go v0.0.0-20230927113136-405c34317fa4/go.mod h1:hl8bxWuFMM72N4YH7FKLGWtYhDz/A0xwvaa8Yr5fxYU=
-github.com/openshift/machine-api-operator v0.2.1-0.20230929171041-2cc7fcf262f3 h1:9nIeQjMOOTNHucCmjxVWZHOg364HYDGjl5rp2CQ+A0I=
-github.com/openshift/machine-api-operator v0.2.1-0.20230929171041-2cc7fcf262f3/go.mod h1:NxAKfWiKFTYsLeR/jVrPTqlGty66apTYjUTEhT1olMw=
+github.com/openshift/machine-api-operator v0.2.1-0.20240116103101-9ee88ebc3723 h1:OwL32bxSXZD56rF4+Pa6EhAw8a5HIsbVQXyl5m4v+cI=
+github.com/openshift/machine-api-operator v0.2.1-0.20240116103101-9ee88ebc3723/go.mod h1:Qb9z7lDje2fNZQwds153HNs16CpSYqnZd9R3LfMOD4M=
 github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI=
 github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=

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

@@ -343,8 +343,9 @@ func TestReconcileVMSuceededState(t *testing.T) {
 		t.Errorf("failed to create machine: %+v", err)
 	}
 
-	if fakeVMService.GetCallCount != 1 {
-		t.Errorf("expected get to be called just once")
+	if fakeVMService.GetCallCount != 2 {
+		// One call for create, and one for the update that happens when the create is successful.
+		t.Errorf("expected get to be called exactly twice")
 	}
 
 	if fakeVMService.DeleteCallCount != 0 {
@@ -359,11 +360,18 @@ func TestReconcileVMSuceededState(t *testing.T) {
 // FakeVMCheckZonesService generic fake vm zone service
 type FakeVMCheckZonesService struct {
 	checkZones []string
+	created    bool
 }
 
 // Get returns fake success.
 func (s *FakeVMCheckZonesService) Get(ctx context.Context, spec azure.Spec) (interface{}, error) {
-	return nil, errors.New("vm not found")
+	if !s.created {
+		return nil, errors.New("vm not found")
+	}
+
+	return &compute.VirtualMachine{
+		VirtualMachineProperties: &compute.VirtualMachineProperties{},
+	}, nil
 }
 
 // CreateOrUpdate returns fake success.
@@ -374,10 +382,12 @@ func (s *FakeVMCheckZonesService) CreateOrUpdate(ctx context.Context, spec azure
 	}
 
 	if len(s.checkZones) <= 0 {
+		s.created = true
 		return nil
 	}
 	for _, zone := range s.checkZones {
 		if strings.EqualFold(zone, vmSpec.Zone) {
+			s.created = true
 			return nil
 		}
 	}

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

@@ -27,6 +27,7 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
 	"github.com/Azure/go-autorest/autorest"
+	autorestazure "github.com/Azure/go-autorest/autorest/azure"
 	machinev1 "github.com/openshift/api/machine/v1beta1"
 	machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
 	"github.com/openshift/machine-api-operator/pkg/metrics"
@@ -139,6 +140,13 @@ func (s *Reconciler) CreateMachine(ctx context.Context) error {
 		return fmt.Errorf("failed to create vm %s: %w", s.scope.Machine.Name, err)
 	}
 
+	// Once we have created the machine, attempt to update it.
+	// This should set the network addresses and provider ID which should be set as soon
+	// as the machine is created, moving it to the provisioned phase.
+	if err := s.Update(ctx); err != nil {
+		return fmt.Errorf("failed to update machine %s: %w", s.scope.Machine.Name, err)
+	}
+
 	return nil
 }
 
@@ -700,8 +708,8 @@ func (s *Reconciler) createVirtualMachine(ctx context.Context, nicName, asName s
 			vmSpec.CustomData = userData
 		}
 
-		err = s.virtualMachinesSvc.CreateOrUpdate(ctx, vmSpec)
-		if err != nil {
+		// If we get an AsynOpIncompleteError, this means the VM is being created and we completed the request successfully.
+		if err := s.virtualMachinesSvc.CreateOrUpdate(ctx, vmSpec); err != nil && !errors.Is(err, autorestazure.NewAsyncOpIncompleteError("compute.VirtualMachinesCreateOrUpdateFuture")) {
 			metrics.RegisterFailedInstanceCreate(&metrics.MachineLabels{
 				Name:      s.scope.Machine.Name,
 				Namespace: s.scope.Machine.Namespace,
@@ -712,6 +720,7 @@ func (s *Reconciler) createVirtualMachine(ctx context.Context, nicName, asName s
 			if errors.As(err, &detailedError) && detailedError.Message == "Failure sending request" {
 				return machinecontroller.InvalidMachineConfiguration("failure sending request for machine %s: %v", s.scope.Machine.Name, err)
 			}
+
 			return fmt.Errorf("failed to create VM: %w", err)
 		}
 	} else if err != nil {