Skip to content

Commit 0fb07b3

Browse files
mweibelmweibel
committed
MachinePool: avoid SetNotReady during normal processing
1 parent 8a1f692 commit 0fb07b3

File tree

2 files changed

+178
-3
lines changed

2 files changed

+178
-3
lines changed

‎azure/scope/machinepool.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -615,19 +615,20 @@ func (m *MachinePoolScope) setProvisioningStateAndConditions(v infrav1.Provision
615615
} else {
616616
conditions.MarkFalse(m.AzureMachinePool, infrav1.ScaleSetDesiredReplicasCondition, infrav1.ScaleSetScaleDownReason, clusterv1.ConditionSeverityInfo, "")
617617
}
618-
m.SetNotReady()
618+
m.SetReady()
619619
case v == infrav1.Updating:
620620
conditions.MarkFalse(m.AzureMachinePool, infrav1.ScaleSetModelUpdatedCondition, infrav1.ScaleSetModelOutOfDateReason, clusterv1.ConditionSeverityInfo, "")
621-
m.SetNotReady()
621+
m.SetReady()
622622
case v == infrav1.Creating:
623623
conditions.MarkFalse(m.AzureMachinePool, infrav1.ScaleSetRunningCondition, infrav1.ScaleSetCreatingReason, clusterv1.ConditionSeverityInfo, "")
624624
m.SetNotReady()
625625
case v == infrav1.Deleting:
626626
conditions.MarkFalse(m.AzureMachinePool, infrav1.ScaleSetRunningCondition, infrav1.ScaleSetDeletingReason, clusterv1.ConditionSeverityInfo, "")
627627
m.SetNotReady()
628+
case v == infrav1.Failed:
629+
conditions.MarkFalse(m.AzureMachinePool, infrav1.ScaleSetRunningCondition, infrav1.ScaleSetProvisionFailedReason, clusterv1.ConditionSeverityInfo, "")
628630
default:
629631
conditions.MarkFalse(m.AzureMachinePool, infrav1.ScaleSetRunningCondition, string(v), clusterv1.ConditionSeverityInfo, "")
630-
m.SetNotReady()
631632
}
632633
}
633634

‎azure/scope/machinepool_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"k8s.io/utils/ptr"
4141
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
4242
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
43+
"sigs.k8s.io/cluster-api/util/conditions"
4344
"sigs.k8s.io/controller-runtime/pkg/client"
4445
"sigs.k8s.io/controller-runtime/pkg/client/fake"
4546

@@ -1577,6 +1578,179 @@ func TestMachinePoolScope_applyAzureMachinePoolMachines(t *testing.T) {
15771578
}
15781579
}
15791580

1581+
func TestMachinePoolScope_setProvisioningStateAndConditions(t *testing.T) {
1582+
scheme := runtime.NewScheme()
1583+
_ = clusterv1.AddToScheme(scheme)
1584+
_ = infrav1exp.AddToScheme(scheme)
1585+
1586+
tests := []struct {
1587+
Name string
1588+
Setup func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder)
1589+
Verify func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client)
1590+
ProvisioningState infrav1.ProvisioningState
1591+
}{
1592+
{
1593+
Name: "if provisioning state is set to Succeeded and replicas match, MachinePool is ready and conditions match",
1594+
Setup: func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder) {
1595+
mp.Spec.Replicas = ptr.To[int32](1)
1596+
amp.Status.Replicas = 1
1597+
},
1598+
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client) {
1599+
g.Expect(amp.Status.Ready).To(BeTrue())
1600+
g.Expect(conditions.Get(amp, infrav1.ScaleSetRunningCondition).Status).To(Equal(corev1.ConditionTrue))
1601+
g.Expect(conditions.Get(amp, infrav1.ScaleSetModelUpdatedCondition).Status).To(Equal(corev1.ConditionTrue))
1602+
g.Expect(conditions.Get(amp, infrav1.ScaleSetDesiredReplicasCondition).Status).To(Equal(corev1.ConditionTrue))
1603+
},
1604+
ProvisioningState: infrav1.Succeeded,
1605+
},
1606+
{
1607+
Name: "if provisioning state is set to Succeeded and replicas are higher on AzureMachinePool, MachinePool is ready and ScalingDown",
1608+
Setup: func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder) {
1609+
mp.Spec.Replicas = ptr.To[int32](1)
1610+
amp.Status.Replicas = 2
1611+
},
1612+
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client) {
1613+
g.Expect(amp.Status.Ready).To(BeTrue())
1614+
condition := conditions.Get(amp, infrav1.ScaleSetDesiredReplicasCondition)
1615+
g.Expect(condition.Status).To(Equal(corev1.ConditionFalse))
1616+
g.Expect(condition.Reason).To(Equal(infrav1.ScaleSetScaleDownReason))
1617+
},
1618+
ProvisioningState: infrav1.Succeeded,
1619+
},
1620+
{
1621+
Name: "if provisioning state is set to Succeeded and replicas are lower on AzureMachinePool, MachinePool is ready and ScalingUp",
1622+
Setup: func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder) {
1623+
mp.Spec.Replicas = ptr.To[int32](2)
1624+
amp.Status.Replicas = 1
1625+
},
1626+
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client) {
1627+
g.Expect(amp.Status.Ready).To(BeTrue())
1628+
condition := conditions.Get(amp, infrav1.ScaleSetDesiredReplicasCondition)
1629+
g.Expect(condition.Status).To(Equal(corev1.ConditionFalse))
1630+
g.Expect(condition.Reason).To(Equal(infrav1.ScaleSetScaleUpReason))
1631+
},
1632+
ProvisioningState: infrav1.Succeeded,
1633+
},
1634+
{
1635+
Name: "if provisioning state is set to Updating, MachinePool is ready and scale set model is set to OutOfDate",
1636+
Setup: func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder) {},
1637+
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client) {
1638+
g.Expect(amp.Status.Ready).To(BeTrue())
1639+
condition := conditions.Get(amp, infrav1.ScaleSetModelUpdatedCondition)
1640+
g.Expect(condition.Status).To(Equal(corev1.ConditionFalse))
1641+
g.Expect(condition.Reason).To(Equal(infrav1.ScaleSetModelOutOfDateReason))
1642+
},
1643+
ProvisioningState: infrav1.Updating,
1644+
},
1645+
{
1646+
Name: "if provisioning state is set to Creating, MachinePool is NotReady and scale set running condition is set to Creating",
1647+
Setup: func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder) {},
1648+
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client) {
1649+
g.Expect(amp.Status.Ready).To(BeFalse())
1650+
condition := conditions.Get(amp, infrav1.ScaleSetRunningCondition)
1651+
g.Expect(condition.Status).To(Equal(corev1.ConditionFalse))
1652+
g.Expect(condition.Reason).To(Equal(infrav1.ScaleSetCreatingReason))
1653+
},
1654+
ProvisioningState: infrav1.Creating,
1655+
},
1656+
{
1657+
Name: "if provisioning state is set to Deleting, MachinePool is NotReady and scale set running condition is set to Deleting",
1658+
Setup: func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder) {},
1659+
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client) {
1660+
g.Expect(amp.Status.Ready).To(BeFalse())
1661+
condition := conditions.Get(amp, infrav1.ScaleSetRunningCondition)
1662+
g.Expect(condition.Status).To(Equal(corev1.ConditionFalse))
1663+
g.Expect(condition.Reason).To(Equal(infrav1.ScaleSetDeletingReason))
1664+
},
1665+
ProvisioningState: infrav1.Deleting,
1666+
},
1667+
{
1668+
Name: "if provisioning state is set to Failed, MachinePool ready state is not adjusted, and scale set running condition is set to Failed",
1669+
Setup: func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder) {},
1670+
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client) {
1671+
condition := conditions.Get(amp, infrav1.ScaleSetRunningCondition)
1672+
g.Expect(condition.Status).To(Equal(corev1.ConditionFalse))
1673+
g.Expect(condition.Reason).To(Equal(infrav1.ScaleSetProvisionFailedReason))
1674+
},
1675+
ProvisioningState: infrav1.Failed,
1676+
},
1677+
{
1678+
Name: "if provisioning state is set to something not explicitly handled, MachinePool ready state is not adjusted, and scale set running condition is set to the ProvisioningState",
1679+
Setup: func(mp *expv1.MachinePool, amp *infrav1exp.AzureMachinePool, cb *fake.ClientBuilder) {},
1680+
Verify: func(g *WithT, amp *infrav1exp.AzureMachinePool, c client.Client) {
1681+
condition := conditions.Get(amp, infrav1.ScaleSetRunningCondition)
1682+
g.Expect(condition.Status).To(Equal(corev1.ConditionFalse))
1683+
g.Expect(condition.Reason).To(Equal(string(infrav1.Migrating)))
1684+
},
1685+
ProvisioningState: infrav1.Migrating,
1686+
},
1687+
}
1688+
for _, tt := range tests {
1689+
t.Run(tt.Name, func(t *testing.T) {
1690+
var (
1691+
g = NewWithT(t)
1692+
mockCtrl = gomock.NewController(t)
1693+
cb = fake.NewClientBuilder().WithScheme(scheme)
1694+
cluster = &clusterv1.Cluster{
1695+
ObjectMeta: metav1.ObjectMeta{
1696+
Name: "cluster1",
1697+
Namespace: "default",
1698+
},
1699+
Spec: clusterv1.ClusterSpec{
1700+
InfrastructureRef: &corev1.ObjectReference{
1701+
Name: "azCluster1",
1702+
},
1703+
},
1704+
Status: clusterv1.ClusterStatus{
1705+
InfrastructureReady: true,
1706+
},
1707+
}
1708+
mp = &expv1.MachinePool{
1709+
ObjectMeta: metav1.ObjectMeta{
1710+
Name: "mp1",
1711+
Namespace: "default",
1712+
OwnerReferences: []metav1.OwnerReference{
1713+
{
1714+
Name: "cluster1",
1715+
Kind: "Cluster",
1716+
APIVersion: clusterv1.GroupVersion.String(),
1717+
},
1718+
},
1719+
},
1720+
}
1721+
amp = &infrav1exp.AzureMachinePool{
1722+
ObjectMeta: metav1.ObjectMeta{
1723+
Name: "amp1",
1724+
Namespace: "default",
1725+
OwnerReferences: []metav1.OwnerReference{
1726+
{
1727+
Name: "mp1",
1728+
Kind: "MachinePool",
1729+
APIVersion: expv1.GroupVersion.String(),
1730+
},
1731+
},
1732+
},
1733+
}
1734+
vmssState = &azure.VMSS{}
1735+
)
1736+
defer mockCtrl.Finish()
1737+
1738+
tt.Setup(mp, amp, cb.WithObjects(amp, cluster))
1739+
s := &MachinePoolScope{
1740+
client: cb.Build(),
1741+
ClusterScoper: &ClusterScope{
1742+
Cluster: cluster,
1743+
},
1744+
MachinePool: mp,
1745+
AzureMachinePool: amp,
1746+
vmssState: vmssState,
1747+
}
1748+
s.setProvisioningStateAndConditions(tt.ProvisioningState)
1749+
tt.Verify(g, s.AzureMachinePool, s.client)
1750+
})
1751+
}
1752+
}
1753+
15801754
func TestBootstrapDataChanges(t *testing.T) {
15811755
ctx, cancel := context.WithCancel(context.Background())
15821756
defer cancel()

0 commit comments

Comments
 (0)