test integration with CAPI cluster-autoscaler

azure/scope/machinepool.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2"
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -930,7 +931,7 @@ func (m *MachinePoolScope) HasReplicasExternallyManaged(ctx context.Context) boo
 }
 
 // ReconcileReplicas ensures MachinePool replicas match VMSS capacity if replicas are externally managed by an autoscaler.
-func (m *MachinePoolScope) ReconcileReplicas(ctx context.Context, vmss *azure.VMSS) error {
+func (m *MachinePoolScope) ReconcileReplicas(ctx context.Context, vmss *armcompute.VirtualMachineScaleSet, scaleSetSpec *scalesets.ScaleSetSpec) error {
 	if !m.HasReplicasExternallyManaged(ctx) {
 		return nil
 	}
@@ -940,8 +941,10 @@ func (m *MachinePoolScope) ReconcileReplicas(ctx context.Context, vmss *azure.VM
 		replicas = *m.MachinePool.Spec.Replicas
 	}
 
-	if capacity := int32(vmss.Capacity); capacity != replicas {
+	capacity := int32(ptr.Deref[int64](vmss.SKU.Capacity, 0))
+	if capacity != replicas {
 		m.UpdateCAPIMachinePoolReplicas(ctx, &capacity)
+		scaleSetSpec.Capacity = int64(capacity)
 	}
 
 	return nil

azure/scope/managedmachinepool.go

@@ -32,6 +32,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-azure/util/versions"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
+	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -367,3 +368,18 @@ func getManagedMachinePoolVersion(managedControlPlane *infrav1.AzureManagedContr
 	}
 	return ptr.To(strings.TrimPrefix(higherVersion, "v"))
 }
+
+// ReconcileReplicas ensures MachinePool replicas match VMSS capacity if replicas are externally managed by an autoscaler.
+func (s *ManagedMachinePoolScope) ReconcileReplicas(ctx context.Context, vmssReplicas int) error {
+	if !annotations.ReplicasManagedByExternalAutoscaler(s.MachinePool) {
+		return nil
+	}
+
+	if s.MachinePool.Spec.Replicas != nil {
+		if vmssReplicas != int(*s.MachinePool.Spec.Replicas) {
+			s.MachinePool.Spec.Replicas = ptr.To(int32(vmssReplicas))
+		}
+	}
+
+	return nil
+}

azure/services/agentpools/agentpools.go

@@ -46,6 +46,7 @@ type AgentPoolScope interface {
 	RemoveCAPIMachinePoolAnnotation(key string)
 	SetSubnetName()
 	IsPreviewEnabled() bool
+	ReconcileReplicas(ctx context.Context, vmssReplicas int) error
 }
 
 // New creates a new service.
@@ -70,8 +71,6 @@ func postCreateOrUpdateResourceHook(ctx context.Context, scope AgentPoolScope, o
 	if ptr.Deref(agentPool.Status.EnableAutoScaling, false) {
 		scope.SetCAPIMachinePoolAnnotation(clusterv1.ReplicasManagedByAnnotation, "true")
 		scope.SetCAPIMachinePoolReplicas(agentPool.Status.Count)
-	} else { // Otherwise, remove the annotation.
-		scope.RemoveCAPIMachinePoolAnnotation(clusterv1.ReplicasManagedByAnnotation)
 	}
 	return nil
 }

azure/services/agentpools/agentpools_test.go

@@ -45,8 +45,6 @@ func TestPostCreateOrUpdateResourceHook(t *testing.T) {
 		mockCtrl := gomock.NewController(t)
 		scope := mock_agentpools.NewMockAgentPoolScope(mockCtrl)
 
-		scope.EXPECT().RemoveCAPIMachinePoolAnnotation(clusterv1.ReplicasManagedByAnnotation)
-
 		managedCluster := &asocontainerservicev1.ManagedClustersAgentPool{
 			Status: asocontainerservicev1.ManagedClusters_AgentPool_STATUS{
 				EnableAutoScaling: ptr.To(false),

azure/services/scalesets/scalesets.go

@@ -45,7 +45,7 @@ type (
 		SetAnnotation(string, string)
 		SetProviderID(string)
 		SetVMSSState(*azure.VMSS)
-		ReconcileReplicas(context.Context, *azure.VMSS) error
+		ReconcileReplicas(context.Context, *armcompute.VirtualMachineScaleSet, *ScaleSetSpec) error
 	}
 
 	// Service provides operations on Azure resources.
@@ -96,9 +96,13 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) {
 		return errors.Errorf("%T is not of type ScaleSetSpec", spec)
 	}
 
-	_, err := s.Client.Get(ctx, spec)
+	result, err := s.Client.Get(ctx, spec)
+	vmss, ok := result.(armcompute.VirtualMachineScaleSet)
+	// Local updates if the  VMSS already exists
 	if err == nil {
-		// We can only get the existing instances if the VMSS already exists
+		if err := s.Scope.ReconcileReplicas(ctx, &vmss, scaleSetSpec); err != nil {
+			return errors.Wrap(err, "unable to reconcile VMSS replicas")
+		}
 		scaleSetSpec.VMSSInstances, err = s.Client.ListInstances(ctx, spec.ResourceGroupName(), spec.ResourceName())
 		if err != nil {
 			err = errors.Wrapf(err, "failed to get existing VMSS instances")
@@ -109,20 +113,11 @@ func (s *Service) Reconcile(ctx context.Context) (retErr error) {
 		return errors.Wrapf(err, "failed to get existing VMSS")
 	}
 
-	result, err := s.CreateOrUpdateResource(ctx, scaleSetSpec, serviceName)
+	_, err = s.CreateOrUpdateResource(ctx, scaleSetSpec, serviceName)
 	s.Scope.UpdatePutStatus(infrav1.BootstrapSucceededCondition, serviceName, err)
 
-	if err == nil && result != nil {
-		vmss, ok := result.(armcompute.VirtualMachineScaleSet)
-		if !ok {
-			return errors.Errorf("%T is not an armcompute.VirtualMachineScaleSet", result)
-		}
-
+	if err == nil {
 		fetchedVMSS := converters.SDKToVMSS(vmss, scaleSetSpec.VMSSInstances)
-		if err := s.Scope.ReconcileReplicas(ctx, &fetchedVMSS); err != nil {
-			return errors.Wrap(err, "unable to reconcile VMSS replicas")
-		}
-
 		// Transform the VMSS resource representation to conform to the cloud-provider-azure representation
 		providerID, err := azprovider.ConvertResourceGroupNameToLower(azureutil.ProviderIDPrefix + fetchedVMSS.ID)
 		if err != nil {

controllers/azuremanagedmachinepool_controller_test.go

@@ -69,6 +69,7 @@ func TestAzureManagedMachinePoolReconcile(t *testing.T) {
 				agentpools.SetSubnetName()
 				agentpools.AgentPoolSpec().Return(&fakeAgentPoolSpec)
 				agentpools.NodeResourceGroup().Return("fake-rg")
+				agentpools.ReconcileReplicas(gomock2.AContext(), 1)
 				agentpools.SetAgentPoolProviderIDList(providerIDs)
 				agentpools.SetAgentPoolReplicas(int32(len(providerIDs))).Return()
 				agentpools.SetAgentPoolReady(true).Return()

controllers/azuremanagedmachinepool_reconciler.go

@@ -164,6 +164,9 @@ func (s *azureManagedMachinePoolService) Reconcile(ctx context.Context) error {
 		providerIDs[i] = providerID
 	}
 
+	if err := s.scope.ReconcileReplicas(ctx, len(instances)); err != nil {
+		return errors.Wrap(err, "unable to reconcile VMSS replicas")
+	}
 	s.scope.SetAgentPoolProviderIDList(providerIDs)
 	s.scope.SetAgentPoolReplicas(int32(len(providerIDs)))
 	s.scope.SetAgentPoolReady(true)

go.mod

@@ -72,6 +72,13 @@ require (
 	sigs.k8s.io/cloud-provider-azure/pkg/azclient/configloader v0.0.1 // indirect
 )
 
+require (
+	github.com/chai2010/gettext-go v1.0.2 // indirect
+	github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d // indirect
+	github.com/mitchellh/go-wordwrap v1.0.1 // indirect
+	github.com/russross/blackfriday/v2 v2.1.0 // indirect
+)
+
 require (
 	github.com/Azure/azure-sdk-for-go v68.0.0+incompatible // indirect
 	github.com/Azure/azure-sdk-for-go/sdk/internal v1.7.0 // indirect

go.sum

@@ -137,6 +137,8 @@ github.com/cenkalti/backoff/v4 v4.2.1/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyY
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
 github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
+github.com/chai2010/gettext-go v1.0.2 h1:1Lwwip6Q2QGsAdl/ZKPCwTe9fe0CjlUbqj5bFNSjIRk=
+github.com/chai2010/gettext-go v1.0.2/go.mod h1:y+wnP2cHYaVj19NZhYKAwEMH2CI1gNHeQQ+5AjwawxA=
 github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
 github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
@@ -186,6 +188,8 @@ github.com/evanphx/json-patch v5.9.0+incompatible h1:fBXyNpNMuTTDdquAq/uisOr2lSh
 github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk=
 github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg=
 github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ=
+github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d h1:105gxyaGwCFad8crR9dcMQWvV9Hvulu6hwUh4tWPJnM=
+github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d/go.mod h1:ZZMPRZwes7CROmyNKgQzC3XPs6L/G2EJLHddWejkmf4=
 github.com/fatih/camelcase v1.0.0 h1:hxNvNX/xYBp0ovncs8WyWZrOrpBNub/JfaMvbURyft8=
 github.com/fatih/camelcase v1.0.0/go.mod h1:yN2Sb0lFhZJUdVvtELVWefmrXpuZESvPmqwoZc+/fpc=
 github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM=
@@ -352,6 +356,8 @@ github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa1
 github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s=
 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
+github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0=
+github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0=
 github.com/mitchellh/mapstructure v1.5.0 h1:jeMsZIYE/09sWLaz43PL7Gy6RuMjD2eJVyuac5Z2hdY=
 github.com/mitchellh/mapstructure v1.5.0/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
 github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
@@ -408,6 +414,7 @@ github.com/prometheus/procfs v0.12.0 h1:jluTpSng7V9hY0O2R9DzzJHYb2xULk9VTR1V1R/k
 github.com/prometheus/procfs v0.12.0/go.mod h1:pcuDEFsWDnvcgNzo4EEweacyhjeA9Zk3cnaOZAZEfOo=
 github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
 github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
+github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/sagikazarmark/locafero v0.4.0 h1:HApY1R9zGo4DBgr7dqsTH/JJxLTTsOt7u6keLGt6kNQ=
 github.com/sagikazarmark/locafero v0.4.0/go.mod h1:Pe1W6UlPYUk/+wc/6KFhbORCfqzgYEpgQ3O5fPuL3H4=

hack/util.sh

@@ -61,6 +61,14 @@ capz::util::should_build_ccm() {
     echo "false"
 }
 
+capz::util::should_build_cluster_autoscaler() {
+    # TEST_CLUSTER_AUTOSCALER is an environment variable set by a prow job to indicate that the cluster-autoscaler runtime should be built and tested.
+    if [[ -n "${TEST_CLUSTER_AUTOSCALER:-}" ]]; then
+        echo "true" && return
+    fi
+    echo "false"
+}
+
 # all test regions must support AvailabilityZones
 capz::util::get_random_region() {
     local REGIONS=("canadacentral" "eastus" "eastus2" "northeurope" "uksouth" "westeurope" "westus2" "westus3")

scripts/ci-build-cluster-autoscaler.sh

@@ -0,0 +1,85 @@
+#!/bin/bash
+
+# Copyright 2024 The Kubernetes Authors.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+###############################################################################
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+REPO_ROOT=$(dirname "${BASH_SOURCE[0]}")/..
+cd "${REPO_ROOT}" || exit 1
+
+# shellcheck source=hack/ensure-go.sh
+source "${REPO_ROOT}/hack/ensure-go.sh"
+# shellcheck source=hack/parse-prow-creds.sh
+source "${REPO_ROOT}/hack/parse-prow-creds.sh"
+
+: "${REGISTRY:?Environment variable empty or not defined.}"
+
+# cluster-autoscaler image
+export CLUSTER_AUTOSCALER_IMAGE_NAME=cluster-autoscaler-amd64
+
+setup() {
+    CLUSTER_AUTOSCALER_ROOT="${CLUSTER_AUTOSCALER_ROOT:-""}"
+    if [[ -z "${CLUSTER_AUTOSCALER_ROOT}" ]]; then
+        CLUSTER_AUTOSCALER_ROOT="$(go env GOPATH)/src/k8s.io/autoscaler/cluster-autoscaler"
+        export CLUSTER_AUTOSCALER_ROOT
+    fi
+
+    # the azure-cloud-provider repo expects IMAGE_REGISTRY.
+    export IMAGE_REGISTRY=${REGISTRY}
+    pushd "${CLUSTER_AUTOSCALER_ROOT}" && TAG=$(git rev-parse --short=7 HEAD) &&
+      export TAG && popd
+    export CLUSTER_AUTOSCALER_IMAGE="${REGISTRY}/${CLUSTER_AUTOSCALER_IMAGE_NAME}:${TAG}"
+    # We use CLUSTER_AUTOSCALER_IMAGE_REPO to pass to the helm install command
+    export CLUSTER_AUTOSCALER_IMAGE_REPO="${REGISTRY}/${CLUSTER_AUTOSCALER_IMAGE_NAME}"
+    # We use CLUSTER_AUTOSCALER_IMAGE_TAG to pass to the helm install command
+    export CLUSTER_AUTOSCALER_IMAGE_TAG="${TAG}"
+    echo "Image registry is ${REGISTRY}"
+    echo "Image Tag is ${TAG}"
+    echo "Image reference is ${CLUSTER_AUTOSCALER_IMAGE}"
+}
+
+main() {
+    if [[ "$(can_reuse_artifacts)" =~ "false" ]]; then
+        echo "Building Linux amd64 cluster-autoscaler"
+        export GOFLAGS=-mod=mod
+        make -C "${CLUSTER_AUTOSCALER_ROOT}" build
+        make -C "${CLUSTER_AUTOSCALER_ROOT}" make-image
+        docker push "${CLUSTER_AUTOSCALER_IMAGE}"
+    fi
+}
+
+# can_reuse_artifacts returns true if there exists CCM artifacts built from a PR that we can reuse
+can_reuse_artifacts() {
+    if ! docker pull "${CLUSTER_AUTOSCALER_IMAGE}"; then
+        echo "false" && return
+    fi
+    echo "true"
+}
+
+capz::ci-build-cluster-autoscaler::cleanup() {
+    echo "cluster-autoscaler cleanup"
+    if [[ -d "${CLUSTER_AUTOSCALER_ROOT:-}" ]]; then
+        make -C "${CLUSTER_AUTOSCALER_ROOT}" clean || true
+    fi
+}
+
+trap capz::ci-build-cluster-autoscaler::cleanup EXIT
+
+setup
+main

scripts/ci-e2e.sh

@@ -42,7 +42,7 @@ source "${REPO_ROOT}/hack/util.sh"
 capz::util::ensure_azure_envs
 
 export LOCAL_ONLY=${LOCAL_ONLY:-"true"}
-export USE_LOCAL_KIND_REGISTRY=${USE_LOCAL_KIND_REGISTRY:-${LOCAL_ONLY}} 
+export USE_LOCAL_KIND_REGISTRY=${USE_LOCAL_KIND_REGISTRY:-${LOCAL_ONLY}}
 export BUILD_MANAGER_IMAGE=${BUILD_MANAGER_IMAGE:-"true"}
 
 if [[ "${USE_LOCAL_KIND_REGISTRY}" == "false" ]]; then
@@ -64,7 +64,12 @@ if [[ "$(capz::util::should_build_ccm)" == "true" ]]; then
   echo "Will use the ${IMAGE_REGISTRY}/${CNM_IMAGE_NAME}:${IMAGE_TAG_CNM} cloud-node-manager image for external cloud-provider-azure cluster"
 fi
 
-export GINKGO_NODES=10
+if [[ "$(capz::util::should_build_cluster_autoscaler)" == "true" ]]; then
+  # shellcheck source=scripts/ci-build-cluster-autoscaler.sh
+  source "${REPO_ROOT}/scripts/ci-build-cluster-autoscaler.sh"
+fi
+
+export GINKGO_NODES=${GINKGO_NODES:-10}
 
 export AZURE_LOCATION="${AZURE_LOCATION:-$(capz::util::get_random_region)}"
 export AZURE_LOCATION_GPU="${AZURE_LOCATION_GPU:-$(capz::util::get_random_region_gpu)}"