Skip to content

Commit

Permalink
Revert "prefer CAPI replicas-managed-by annotation" for now
Browse files Browse the repository at this point in the history
This reverts commit b8f29c0.
  • Loading branch information
dthorsen committed Jan 12, 2023
1 parent 5aa9a83 commit af0edc1
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 8 deletions.
4 changes: 4 additions & 0 deletions azure/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,8 @@ const (
// See https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/
// for annotation formatting rules.
RGTagsLastAppliedAnnotation = "sigs.k8s.io/cluster-api-provider-azure-last-applied-tags-rg"

// ReplicasManagedByAutoscalerAnnotation is set to true in the corresponding capi machine pool
// when an external autoscaler manages the node count of the associated machine pool.
ReplicasManagedByAutoscalerAnnotation = "cluster.x-k8s.io/replicas-managed-by-autoscaler"
)
3 changes: 1 addition & 2 deletions azure/scope/machinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import (
"sigs.k8s.io/cluster-api/controllers/noderefutil"
capierrors "sigs.k8s.io/cluster-api/errors"
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"
Expand Down Expand Up @@ -741,7 +740,7 @@ func (m *MachinePoolScope) UpdateCAPIMachinePoolReplicas(ctx context.Context, re

// HasReplicasExternallyManaged returns true if the externally managed annotation is set on the CAPI MachinePool resource.
func (m *MachinePoolScope) HasReplicasExternallyManaged(ctx context.Context) bool {
return annotations.ReplicasManagedByExternalAutoscaler(m.MachinePool)
return m.MachinePool.Annotations[azure.ReplicasManagedByAutoscalerAnnotation] == "true"
}

// ReconcileReplicas ensures MachinePool replicas match VMSS capacity if replicas are externally managed by an autoscaler.
Expand Down
5 changes: 2 additions & 3 deletions azure/services/agentpools/agentpools.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/async"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)

const serviceName = "agentpools"
Expand Down Expand Up @@ -85,10 +84,10 @@ func (s *Service) Reconcile(ctx context.Context) error {
}
// When autoscaling is set, add the annotation to the machine pool and update the replica count.
if to.Bool(agentPool.EnableAutoScaling) {
s.scope.SetCAPIMachinePoolAnnotation(clusterv1.ReplicasManagedByAnnotation, "true")
s.scope.SetCAPIMachinePoolAnnotation(azure.ReplicasManagedByAutoscalerAnnotation, "true")
s.scope.SetCAPIMachinePoolReplicas(agentPool.Count)
} else { // Otherwise, remove the annotation.
s.scope.RemoveCAPIMachinePoolAnnotation(clusterv1.ReplicasManagedByAnnotation)
s.scope.RemoveCAPIMachinePoolAnnotation(azure.ReplicasManagedByAutoscalerAnnotation)
}
}
} else {
Expand Down
6 changes: 3 additions & 3 deletions azure/services/agentpools/agentpools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ import (
"github.com/golang/mock/gomock"
. "github.com/onsi/gomega"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/agentpools/mock_agentpools"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async"
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
)

var (
Expand Down Expand Up @@ -73,7 +73,7 @@ func TestReconcileAgentPools(t *testing.T) {
expect: func(s *mock_agentpools.MockAgentPoolScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AgentPoolSpec().Return(&fakeAgentPoolSpec)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeAgentPoolSpec, serviceName).Return(fakeAgentPoolWithAutoscalingAndCount(true, 1), nil)
s.SetCAPIMachinePoolAnnotation(clusterv1.ReplicasManagedByAnnotation, "true")
s.SetCAPIMachinePoolAnnotation(azure.ReplicasManagedByAutoscalerAnnotation, "true")
s.SetCAPIMachinePoolReplicas(fakeAgentPoolWithAutoscalingAndCount(true, 1).Count)
s.UpdatePutStatus(infrav1.AgentPoolsReadyCondition, serviceName, nil)
},
Expand All @@ -84,7 +84,7 @@ func TestReconcileAgentPools(t *testing.T) {
expect: func(s *mock_agentpools.MockAgentPoolScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
s.AgentPoolSpec().Return(&fakeAgentPoolSpec)
r.CreateOrUpdateResource(gomockinternal.AContext(), &fakeAgentPoolSpec, serviceName).Return(fakeAgentPoolWithAutoscalingAndCount(false, 1), nil)
s.RemoveCAPIMachinePoolAnnotation(clusterv1.ReplicasManagedByAnnotation)
s.RemoveCAPIMachinePoolAnnotation(azure.ReplicasManagedByAutoscalerAnnotation)

s.UpdatePutStatus(infrav1.AgentPoolsReadyCondition, serviceName, nil)
},
Expand Down

0 comments on commit af0edc1

Please sign in to comment.