Merge pull request #96 from JoelSpeed/max-concurrent-reconciles-4.14

OCPBUGS-28745: Improving performance of VMs created in Azure

cmd/manager/main.go

@@ -91,6 +91,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()
@@ -165,7 +171,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,16 +15,16 @@ require (
 	github.com/mitchellh/mapstructure v1.5.0
 	github.com/onsi/ginkgo/v2 v2.9.5
 	github.com/onsi/gomega v1.27.7
-	github.com/openshift/api v0.0.0-20230707123100-21c0ce73add5
-	github.com/openshift/machine-api-operator v0.2.1-0.20230531233206-931f6f67c1c7
+	github.com/openshift/api v0.0.0-20230711095040-ca06f4a23b64
+	github.com/openshift/machine-api-operator v0.2.1-0.20240205151550-4390dd703858
 	github.com/pkg/errors v0.9.1
 	github.com/spf13/cobra v1.7.0
 	golang.org/x/crypto v0.14.0
 	golang.org/x/net v0.17.0 // indirect
 
 	// kube 1.27
-	k8s.io/api v0.27.2
-	k8s.io/apimachinery v0.27.2
+	k8s.io/api v0.27.3
+	k8s.io/apimachinery v0.27.3
 	k8s.io/client-go v0.27.2
 	k8s.io/klog/v2 v2.100.1
 	k8s.io/utils v0.0.0-20230505201702-9f6742963106
@@ -39,7 +39,7 @@ require (
 	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-20230503144108-75015d2347cb
-	github.com/openshift/library-go v0.0.0-20230508110756-9b7abe2c9cbf
+	github.com/openshift/library-go v0.0.0-20230706195801-561433066966
 )
 
 require (

go.sum

@@ -467,14 +467,14 @@ github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGV
 github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
 github.com/onsi/gomega v1.27.7 h1:fVih9JD6ogIiHUN6ePK7HJidyEDpWGVB5mzM7cWNXoU=
 github.com/onsi/gomega v1.27.7/go.mod h1:1p8OOlwo2iUUDsHnOrjE5UKYJ+e3W8eQ3qSlRahPmr4=
-github.com/openshift/api v0.0.0-20230707123100-21c0ce73add5 h1:htabZ4P4mMAPTaTU0H0p++o/fmowJD5lnHR16ZBq3Js=
-github.com/openshift/api v0.0.0-20230707123100-21c0ce73add5/go.mod h1:yimSGmjsI+XF1mr+AKBs2//fSXIOhhetHGbMlBEfXbs=
+github.com/openshift/api v0.0.0-20230711095040-ca06f4a23b64 h1:j7LIIr4Vrdy4Dpd4bw2j53UXUSjA1eXXC0x89g9kyAI=
+github.com/openshift/api v0.0.0-20230711095040-ca06f4a23b64/go.mod h1:yimSGmjsI+XF1mr+AKBs2//fSXIOhhetHGbMlBEfXbs=
 github.com/openshift/client-go v0.0.0-20230503144108-75015d2347cb h1:Nij5OnaECrkmcRQMAE9LMbQXPo95aqFnf+12B7SyFVI=
 github.com/openshift/client-go v0.0.0-20230503144108-75015d2347cb/go.mod h1:Rhb3moCqeiTuGHAbXBOlwPubUMlOZEkrEWTRjIF3jzs=
-github.com/openshift/library-go v0.0.0-20230508110756-9b7abe2c9cbf h1:ZpFAN2qprgp7jEhGPrOAwP8mmuYC9BRYzvDefg+k4GM=
-github.com/openshift/library-go v0.0.0-20230508110756-9b7abe2c9cbf/go.mod h1:PJVatR/oS/EaFciwylyAr9hORSqQHrC+5bXf4L0wsBY=
-github.com/openshift/machine-api-operator v0.2.1-0.20230531233206-931f6f67c1c7 h1:6/Yok3qh3FPnjw+OsefCEPbV0KFEVkr00NaEYDpAY6M=
-github.com/openshift/machine-api-operator v0.2.1-0.20230531233206-931f6f67c1c7/go.mod h1:cYJjVQyNskmxEixGczlLytGF9iacFubTD/UbGvu5EEY=
+github.com/openshift/library-go v0.0.0-20230706195801-561433066966 h1:qJZaVzxJy7s6Cp1908rkSR64YCrpiKMZAkfYhsZPPCw=
+github.com/openshift/library-go v0.0.0-20230706195801-561433066966/go.mod h1:PegtilvJPBJXjJG3AV8uL1a0SAnBr6K67ShNiWVb40M=
+github.com/openshift/machine-api-operator v0.2.1-0.20240205151550-4390dd703858 h1:/DpOuj4eiY+7uevet+qkSxl4hWPFc5MWlcxXomu2Cdg=
+github.com/openshift/machine-api-operator v0.2.1-0.20240205151550-4390dd703858/go.mod h1:ZEJ53GfekJMJB6+SLHzGIV5pJavQT0G+LUn62cuVc0c=
 github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI=
@@ -1014,16 +1014,16 @@ honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 k8s.io/api v0.17.0/go.mod h1:npsyOePkeP0CPwyGfXDHxvypiYMJxBWAMpQxCaJ4ZxI=
 k8s.io/api v0.18.0-beta.2/go.mod h1:2oeNnWEqcSmaM/ibSh3t7xcIqbkGXhzZdn4ezV9T4m0=
-k8s.io/api v0.27.2 h1:+H17AJpUMvl+clT+BPnKf0E3ksMAzoBBg7CntpSuADo=
-k8s.io/api v0.27.2/go.mod h1:ENmbocXfBT2ADujUXcBhHV55RIT31IIEvkntP6vZKS4=
+k8s.io/api v0.27.3 h1:yR6oQXXnUEBWEWcvPWS0jQL575KoAboQPfJAuKNrw5Y=
+k8s.io/api v0.27.3/go.mod h1:C4BNvZnQOF7JA/0Xed2S+aUyJSfTGkGFxLXz9MnpIpg=
 k8s.io/apiextensions-apiserver v0.17.0/go.mod h1:XiIFUakZywkUl54fVXa7QTEHcqQz9HG55nHd1DCoHj8=
 k8s.io/apiextensions-apiserver v0.18.0-beta.2/go.mod h1:Hnrg5jx8/PbxRbUoqDGxtQkULjwx8FDW4WYJaKNK+fk=
 k8s.io/apiextensions-apiserver v0.27.2 h1:iwhyoeS4xj9Y7v8YExhUwbVuBhMr3Q4bd/laClBV6Bo=
 k8s.io/apiextensions-apiserver v0.27.2/go.mod h1:Oz9UdvGguL3ULgRdY9QMUzL2RZImotgxvGjdWRq6ZXQ=
 k8s.io/apimachinery v0.17.0/go.mod h1:b9qmWdKlLuU9EBh+06BtLcSf/Mu89rWL33naRxs1uZg=
 k8s.io/apimachinery v0.18.0-beta.2/go.mod h1:9SnR/e11v5IbyPCGbvJViimtJ0SwHG4nfZFjU77ftcA=
-k8s.io/apimachinery v0.27.2 h1:vBjGaKKieaIreI+oQwELalVG4d8f3YAMNpWLzDXkxeg=
-k8s.io/apimachinery v0.27.2/go.mod h1:XNfZ6xklnMCOGGFNqXG7bUrQCoR04dh/E7FprV6pb+E=
+k8s.io/apimachinery v0.27.3 h1:Ubye8oBufD04l9QnNtW05idcOe9Z3GQN8+7PqmuVcUM=
+k8s.io/apimachinery v0.27.3/go.mod h1:XNfZ6xklnMCOGGFNqXG7bUrQCoR04dh/E7FprV6pb+E=
 k8s.io/apiserver v0.17.0/go.mod h1:ABM+9x/prjINN6iiffRVNCBR2Wk7uY4z+EtEGZD48cg=
 k8s.io/apiserver v0.18.0-beta.2/go.mod h1:bnblMkMoCFnIfVnVftd0SXJPzyvrk3RtaqSbblphF/A=
 k8s.io/apiserver v0.27.2 h1:p+tjwrcQEZDrEorCZV2/qE8osGTINPuS5ZNqWAvKm5E=

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"
 	"github.com/Azure/go-autorest/autorest/to"
 	machinev1 "github.com/openshift/api/machine/v1beta1"
 	machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine"
@@ -140,6 +141,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
 }
 
@@ -701,8 +709,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,
@@ -713,6 +721,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 {

vendor/modules.txt

@@ -314,7 +314,7 @@ github.com/onsi/gomega/matchers/support/goraph/edge
 github.com/onsi/gomega/matchers/support/goraph/node
 github.com/onsi/gomega/matchers/support/goraph/util
 github.com/onsi/gomega/types
-# github.com/openshift/api v0.0.0-20230707123100-21c0ce73add5
+# github.com/openshift/api v0.0.0-20230711095040-ca06f4a23b64
 ## explicit; go 1.20
 github.com/openshift/api
 github.com/openshift/api/apiserver
@@ -410,7 +410,7 @@ github.com/openshift/client-go/machine/clientset/versioned/typed/machine/v1beta1
 github.com/openshift/client-go/machine/informers/externalversions/internalinterfaces
 github.com/openshift/client-go/machine/informers/externalversions/machine/v1beta1
 github.com/openshift/client-go/machine/listers/machine/v1beta1
-# github.com/openshift/library-go v0.0.0-20230508110756-9b7abe2c9cbf
+# github.com/openshift/library-go v0.0.0-20230706195801-561433066966
 ## explicit; go 1.20
 github.com/openshift/library-go/pkg/config/clusterstatus
 github.com/openshift/library-go/pkg/config/leaderelection
@@ -427,7 +427,7 @@ github.com/openshift/library-go/pkg/operator/resource/resourcemerge
 github.com/openshift/library-go/pkg/operator/resource/resourceread
 github.com/openshift/library-go/pkg/operator/resourcesynccontroller
 github.com/openshift/library-go/pkg/operator/v1helpers
-# github.com/openshift/machine-api-operator v0.2.1-0.20230531233206-931f6f67c1c7
+# github.com/openshift/machine-api-operator v0.2.1-0.20240205151550-4390dd703858
 ## explicit; go 1.19
 github.com/openshift/machine-api-operator/pkg/controller/machine
 github.com/openshift/machine-api-operator/pkg/metrics
@@ -641,7 +641,7 @@ gopkg.in/yaml.v2
 # gopkg.in/yaml.v3 v3.0.1
 ## explicit
 gopkg.in/yaml.v3
-# k8s.io/api v0.27.2
+# k8s.io/api v0.27.3
 ## explicit; go 1.20
 k8s.io/api/admission/v1
 k8s.io/api/admission/v1beta1
@@ -707,7 +707,7 @@ k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset
 k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme
 k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1
 k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1
-# k8s.io/apimachinery v0.27.2
+# k8s.io/apimachinery v0.27.3
 ## explicit; go 1.20
 k8s.io/apimachinery/pkg/api/equality
 k8s.io/apimachinery/pkg/api/errors