Skip to content

Commit c0caa60

Browse files
committed
Adds Conditions to MachineDeployment. In particular, it adds
`AvailableCondition` and `ReadyCondition`.
1 parent c5bbf36 commit c0caa60

10 files changed

+232
-13
lines changed

api/v1alpha3/conversion.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,9 +137,9 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error {
137137
dst.Spec.Strategy.RollingUpdate = &v1alpha4.MachineRollingUpdateDeployment{}
138138
}
139139
dst.Spec.Strategy.RollingUpdate.DeletePolicy = restored.Spec.Strategy.RollingUpdate.DeletePolicy
140-
141140
}
142141

142+
dst.Status.Conditions = restored.Status.Conditions
143143
return nil
144144
}
145145

@@ -158,6 +158,11 @@ func (dst *MachineDeployment) ConvertFrom(srcRaw conversion.Hub) error {
158158
return nil
159159
}
160160

161+
// Status.Conditions was introduced in v1alpha4, thus requiring a custom conversion function; the values is going to be preserved in an annotation thus allowing roundtrip without loosing informations
162+
func Convert_v1alpha4_MachineDeploymentStatus_To_v1alpha3_MachineDeploymentStatus(in *v1alpha4.MachineDeploymentStatus, out *MachineDeploymentStatus, s apiconversion.Scope) error {
163+
return autoConvert_v1alpha4_MachineDeploymentStatus_To_v1alpha3_MachineDeploymentStatus(in, out, s)
164+
}
165+
161166
func (src *MachineDeploymentList) ConvertTo(dstRaw conversion.Hub) error {
162167
dst := dstRaw.(*v1alpha4.MachineDeploymentList)
163168

api/v1alpha3/zz_generated.conversion.go

Lines changed: 6 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/v1alpha4/condition_consts.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,3 +201,14 @@ const (
201201
// from making any further remediations.
202202
TooManyUnhealthyReason = "TooManyUnhealthy"
203203
)
204+
205+
// Conditions and condition Reasons for MachineDeployments
206+
207+
const (
208+
// MachineDeploymentAvailableCondition means the MachineDeployment is available, that is, at least the minimum available
209+
// machines required (i.e. Spec.Replicas-MaxUnavailable when MachineDeploymentStrategyType = RollingUpdate) are up and running for at least minReadySeconds.
210+
MachineDeploymentAvailableCondition ConditionType = "Available"
211+
212+
// WaitingForAvailableMachinesReason (Severity=Warning) reflects the fact that the required minimum number of machines for a machinedeployment are not available.
213+
WaitingForAvailableMachinesReason = "WaitingForAvailableMachines"
214+
)

api/v1alpha4/machinedeployment_types.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,10 @@ type MachineDeploymentStatus struct {
211211
// Phase represents the current phase of a MachineDeployment (ScalingUp, ScalingDown, Running, Failed, or Unknown).
212212
// +optional
213213
Phase string `json:"phase,omitempty"`
214+
215+
// Conditions defines current service state of the MachineDeployment.
216+
// +optional
217+
Conditions Conditions `json:"conditions,omitempty"`
214218
}
215219

216220
// ANCHOR_END: MachineDeploymentStatus
@@ -287,3 +291,13 @@ type MachineDeploymentList struct {
287291
func init() {
288292
SchemeBuilder.Register(&MachineDeployment{}, &MachineDeploymentList{})
289293
}
294+
295+
// GetConditions returns the set of conditions for the machinedeployment.
296+
func (m *MachineDeployment) GetConditions() Conditions {
297+
return m.Status.Conditions
298+
}
299+
300+
// SetConditions updates the set of conditions on the machinedeployment.
301+
func (m *MachineDeployment) SetConditions(conditions Conditions) {
302+
m.Status.Conditions = conditions
303+
}

api/v1alpha4/zz_generated.deepcopy.go

Lines changed: 8 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,50 @@ spec:
864864
minReadySeconds) targeted by this deployment.
865865
format: int32
866866
type: integer
867+
conditions:
868+
description: Conditions defines current service state of the MachineDeployment.
869+
items:
870+
description: Condition defines an observation of a Cluster API resource
871+
operational state.
872+
properties:
873+
lastTransitionTime:
874+
description: Last time the condition transitioned from one status
875+
to another. This should be when the underlying condition changed.
876+
If that is not known, then using the time when the API field
877+
changed is acceptable.
878+
format: date-time
879+
type: string
880+
message:
881+
description: A human readable message indicating details about
882+
the transition. This field may be empty.
883+
type: string
884+
reason:
885+
description: The reason for the condition's last transition
886+
in CamelCase. The specific API may choose whether or not this
887+
field is considered a guaranteed API. This field may not be
888+
empty.
889+
type: string
890+
severity:
891+
description: Severity provides an explicit classification of
892+
Reason code, so the users or machines can immediately understand
893+
the current situation and act accordingly. The Severity field
894+
MUST be set only when Status=False.
895+
type: string
896+
status:
897+
description: Status of the condition, one of True, False, Unknown.
898+
type: string
899+
type:
900+
description: Type of condition in CamelCase or in foo.example.com/CamelCase.
901+
Many .condition.type values are consistent across resources
902+
like Available, but because arbitrary conditions can be useful
903+
(see .node.status.conditions), the ability to deconflict is
904+
important.
905+
type: string
906+
required:
907+
- status
908+
- type
909+
type: object
910+
type: array
867911
observedGeneration:
868912
description: The generation observed by the deployment controller.
869913
format: int64

controllers/machinedeployment_controller.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
3232
"sigs.k8s.io/cluster-api/util"
3333
"sigs.k8s.io/cluster-api/util/annotations"
34+
"sigs.k8s.io/cluster-api/util/conditions"
3435
"sigs.k8s.io/cluster-api/util/patch"
3536
"sigs.k8s.io/cluster-api/util/predicates"
3637
ctrl "sigs.k8s.io/controller-runtime"
@@ -129,7 +130,12 @@ func (r *MachineDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Re
129130

130131
defer func() {
131132
// Always attempt to patch the object and status after each reconciliation.
132-
if err := patchHelper.Patch(ctx, deployment); err != nil {
133+
// Patch ObservedGeneration only if the reconciliation completed successfully
134+
patchOpts := []patch.Option{}
135+
if reterr == nil {
136+
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
137+
}
138+
if err := patchMachineDeployment(ctx, patchHelper, deployment, patchOpts...); err != nil {
133139
reterr = kerrors.NewAggregate([]error{reterr, err})
134140
}
135141
}()
@@ -148,6 +154,24 @@ func (r *MachineDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Re
148154
return result, err
149155
}
150156

157+
func patchMachineDeployment(ctx context.Context, patchHelper *patch.Helper, d *clusterv1.MachineDeployment, options ...patch.Option) error {
158+
// Always update the readyCondition by summarizing the state of other conditions.
159+
conditions.SetSummary(d,
160+
conditions.WithConditions(
161+
clusterv1.MachineDeploymentAvailableCondition,
162+
),
163+
)
164+
165+
// Patch the object, ignoring conflicts on the conditions owned by this controller.
166+
options = append(options,
167+
patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
168+
clusterv1.ReadyCondition,
169+
clusterv1.MachineDeploymentAvailableCondition,
170+
}},
171+
)
172+
return patchHelper.Patch(ctx, d, options...)
173+
}
174+
151175
func (r *MachineDeploymentReconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster, d *clusterv1.MachineDeployment) (ctrl.Result, error) {
152176
log := ctrl.LoggerFrom(ctx)
153177
log.V(4).Info("Reconcile MachineDeployment")

controllers/machinedeployment_controller_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
3434
"sigs.k8s.io/cluster-api/controllers/external"
3535
"sigs.k8s.io/cluster-api/util"
36+
"sigs.k8s.io/cluster-api/util/conditions"
3637
)
3738

3839
var _ reconcile.Reconciler = &MachineDeploymentReconciler{}
@@ -389,6 +390,13 @@ func TestMachineDeploymentReconciler(t *testing.T) {
389390
return len(machineSets.Items)
390391
}, timeout*5).Should(BeEquivalentTo(0))
391392

393+
t.Log("Verifying MachineDeployment has correct Conditions")
394+
g.Eventually(func() bool {
395+
key := client.ObjectKey{Name: deployment.Name, Namespace: deployment.Namespace}
396+
g.Expect(env.Get(ctx, key, deployment)).To(Succeed())
397+
return conditions.IsTrue(deployment, clusterv1.MachineDeploymentAvailableCondition)
398+
}, timeout).Should(BeTrue())
399+
392400
// Validate that the controller set the cluster name label in selector.
393401
g.Expect(deployment.Status.Selector).To(ContainSubstring(testCluster.Name))
394402
})

controllers/machinedeployment_sync.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
3333
"sigs.k8s.io/cluster-api/controllers/mdutil"
3434
"sigs.k8s.io/cluster-api/util"
35+
"sigs.k8s.io/cluster-api/util/conditions"
3536
"sigs.k8s.io/cluster-api/util/patch"
3637
ctrl "sigs.k8s.io/controller-runtime"
3738
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -353,6 +354,16 @@ func (r *MachineDeploymentReconciler) scale(ctx context.Context, deployment *clu
353354
// syncDeploymentStatus checks if the status is up-to-date and sync it if necessary.
354355
func (r *MachineDeploymentReconciler) syncDeploymentStatus(allMSs []*clusterv1.MachineSet, newMS *clusterv1.MachineSet, d *clusterv1.MachineDeployment) error {
355356
d.Status = calculateStatus(allMSs, newMS, d)
357+
358+
// minReplicasNeeded will be equal to d.Spec.Replicas when the strategy is not RollingUpdateMachineDeploymentStrategyType.
359+
minReplicasNeeded := *(d.Spec.Replicas) - mdutil.MaxUnavailable(*d)
360+
361+
if d.Status.AvailableReplicas >= minReplicasNeeded {
362+
// NOTE: The structure of calculateStatus() does not allow us to update the machinedeployment directly, we can only update the status obj it returns. Ideally, we should change calculateStatus() --> updateStatus() to be consistent with the rest of the code base, until then, we update conditions here.
363+
conditions.MarkTrue(d, clusterv1.MachineDeploymentAvailableCondition)
364+
} else {
365+
conditions.MarkFalse(d, clusterv1.MachineDeploymentAvailableCondition, clusterv1.WaitingForAvailableMachinesReason, clusterv1.ConditionSeverityWarning, "Minimum availability requires %d replicas, current %d available", minReplicasNeeded, d.Status.AvailableReplicas)
366+
}
356367
return nil
357368
}
358369

@@ -380,6 +391,7 @@ func calculateStatus(allMSs []*clusterv1.MachineSet, newMS *clusterv1.MachineSet
380391
ReadyReplicas: mdutil.GetReadyReplicaCountForMachineSets(allMSs),
381392
AvailableReplicas: availableReplicas,
382393
UnavailableReplicas: unavailableReplicas,
394+
Conditions: deployment.Status.Conditions,
383395
}
384396

385397
if *deployment.Spec.Replicas == status.ReadyReplicas {

controllers/machinedeployment_sync_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
. "github.com/onsi/gomega"
2525
"github.com/pkg/errors"
26+
corev1 "k8s.io/api/core/v1"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2728
"k8s.io/client-go/tools/record"
2829
"k8s.io/utils/pointer"
@@ -394,3 +395,100 @@ func TestScaleMachineSet(t *testing.T) {
394395
})
395396
}
396397
}
398+
399+
func newTestMachineDeployment(pds *int32, replicas, statusReplicas, updatedReplicas, availableReplicas int32, conditions clusterv1.Conditions) *clusterv1.MachineDeployment {
400+
d := &clusterv1.MachineDeployment{
401+
ObjectMeta: metav1.ObjectMeta{
402+
Name: "progress-test",
403+
},
404+
Spec: clusterv1.MachineDeploymentSpec{
405+
ProgressDeadlineSeconds: pds,
406+
Replicas: &replicas,
407+
Strategy: &clusterv1.MachineDeploymentStrategy{
408+
Type: clusterv1.RollingUpdateMachineDeploymentStrategyType,
409+
RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{
410+
MaxUnavailable: intOrStrPtr(0),
411+
MaxSurge: intOrStrPtr(1),
412+
DeletePolicy: pointer.StringPtr("Oldest"),
413+
},
414+
},
415+
},
416+
Status: clusterv1.MachineDeploymentStatus{
417+
Replicas: statusReplicas,
418+
UpdatedReplicas: updatedReplicas,
419+
AvailableReplicas: availableReplicas,
420+
Conditions: conditions,
421+
},
422+
}
423+
return d
424+
}
425+
426+
// helper to create MS with given availableReplicas.
427+
func newTestMachinesetWithReplicas(name string, specReplicas, statusReplicas, availableReplicas int32) *clusterv1.MachineSet {
428+
return &clusterv1.MachineSet{
429+
ObjectMeta: metav1.ObjectMeta{
430+
Name: name,
431+
CreationTimestamp: metav1.Time{},
432+
Namespace: metav1.NamespaceDefault,
433+
},
434+
Spec: clusterv1.MachineSetSpec{
435+
Replicas: pointer.Int32Ptr(specReplicas),
436+
},
437+
Status: clusterv1.MachineSetStatus{
438+
AvailableReplicas: availableReplicas,
439+
Replicas: statusReplicas,
440+
},
441+
}
442+
}
443+
444+
func TestSyncDeploymentStatus(t *testing.T) {
445+
pds := int32(60)
446+
tests := []struct {
447+
name string
448+
d *clusterv1.MachineDeployment
449+
oldMachineSets []*clusterv1.MachineSet
450+
newMachineSet *clusterv1.MachineSet
451+
expectedConditions []*clusterv1.Condition
452+
}{
453+
{
454+
name: "Deployment not available: MachineDeploymentAvailableCondition should exist and be false",
455+
d: newTestMachineDeployment(&pds, 3, 2, 2, 2, clusterv1.Conditions{}),
456+
oldMachineSets: []*clusterv1.MachineSet{},
457+
newMachineSet: newTestMachinesetWithReplicas("foo", 3, 2, 2),
458+
expectedConditions: []*clusterv1.Condition{
459+
{
460+
Type: clusterv1.MachineDeploymentAvailableCondition,
461+
Status: corev1.ConditionFalse,
462+
Severity: clusterv1.ConditionSeverityWarning,
463+
Reason: clusterv1.WaitingForAvailableMachinesReason,
464+
},
465+
},
466+
},
467+
{
468+
name: "Deployment Available: MachineDeploymentAvailableCondition should exist and be true",
469+
d: newTestMachineDeployment(&pds, 3, 3, 3, 3, clusterv1.Conditions{}),
470+
oldMachineSets: []*clusterv1.MachineSet{},
471+
newMachineSet: newTestMachinesetWithReplicas("foo", 3, 3, 3),
472+
expectedConditions: []*clusterv1.Condition{
473+
{
474+
Type: clusterv1.MachineDeploymentAvailableCondition,
475+
Status: corev1.ConditionTrue,
476+
},
477+
},
478+
},
479+
}
480+
481+
for _, test := range tests {
482+
t.Run(test.name, func(t *testing.T) {
483+
g := NewWithT(t)
484+
r := &MachineDeploymentReconciler{
485+
Client: fake.NewClientBuilder().Build(),
486+
recorder: record.NewFakeRecorder(32),
487+
}
488+
allMachineSets := append(test.oldMachineSets, test.newMachineSet)
489+
err := r.syncDeploymentStatus(allMachineSets, test.newMachineSet, test.d)
490+
g.Expect(err).ToNot(HaveOccurred())
491+
assertConditions(t, test.d, test.expectedConditions...)
492+
})
493+
}
494+
}

0 commit comments

Comments
 (0)