Skip to content

Commit

Permalink
Adds Conditions to MachineDeployment. In particular, it adds
Browse files Browse the repository at this point in the history
`AvailableCondition` and `ReadyCondition`.
  • Loading branch information
Arvinderpal committed Jul 15, 2021
1 parent c5bbf36 commit 377cd74
Show file tree
Hide file tree
Showing 10 changed files with 232 additions and 13 deletions.
7 changes: 6 additions & 1 deletion api/v1alpha3/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error {
dst.Spec.Strategy.RollingUpdate = &v1alpha4.MachineRollingUpdateDeployment{}
}
dst.Spec.Strategy.RollingUpdate.DeletePolicy = restored.Spec.Strategy.RollingUpdate.DeletePolicy

}

dst.Status.Conditions = restored.Status.Conditions
return nil
}

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

// 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
func Convert_v1alpha4_MachineDeploymentStatus_To_v1alpha3_MachineDeploymentStatus(in *v1alpha4.MachineDeploymentStatus, out *MachineDeploymentStatus, s apiconversion.Scope) error {
return autoConvert_v1alpha4_MachineDeploymentStatus_To_v1alpha3_MachineDeploymentStatus(in, out, s)
}

func (src *MachineDeploymentList) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*v1alpha4.MachineDeploymentList)

Expand Down
16 changes: 6 additions & 10 deletions api/v1alpha3/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions api/v1alpha4/condition_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,3 +201,14 @@ const (
// from making any further remediations.
TooManyUnhealthyReason = "TooManyUnhealthy"
)

// Conditions and condition Reasons for MachineDeployments

const (
// MachineDeploymentAvailableCondition means the MachineDeployment is available, that is, at least the minimum available
// machines required (i.e. Spec.Replicas-MaxUnavailable when MachineDeploymentStrategyType = RollingUpdate) are up and running for at least minReadySeconds.
MachineDeploymentAvailableCondition ConditionType = "Available"

// WaitingForAvailableMachinesReason (Severity=Warning) reflects the fact that the required minimum number of machines for a machinedeployment are not available.
WaitingForAvailableMachinesReason = "WaitingForAvailableMachines"
)
14 changes: 14 additions & 0 deletions api/v1alpha4/machinedeployment_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ type MachineDeploymentStatus struct {
// Phase represents the current phase of a MachineDeployment (ScalingUp, ScalingDown, Running, Failed, or Unknown).
// +optional
Phase string `json:"phase,omitempty"`

// Conditions defines current service state of the MachineDeployment.
// +optional
Conditions Conditions `json:"conditions,omitempty"`
}

// ANCHOR_END: MachineDeploymentStatus
Expand Down Expand Up @@ -287,3 +291,13 @@ type MachineDeploymentList struct {
func init() {
SchemeBuilder.Register(&MachineDeployment{}, &MachineDeploymentList{})
}

// GetConditions returns the set of conditions for the machinedeployment.
func (m *MachineDeployment) GetConditions() Conditions {
return m.Status.Conditions
}

// SetConditions updates the set of conditions on the machinedeployment.
func (m *MachineDeployment) SetConditions(conditions Conditions) {
m.Status.Conditions = conditions
}
9 changes: 8 additions & 1 deletion api/v1alpha4/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 44 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,50 @@ spec:
minReadySeconds) targeted by this deployment.
format: int32
type: integer
conditions:
description: Conditions defines current service state of the MachineDeployment.
items:
description: Condition defines an observation of a Cluster API resource
operational state.
properties:
lastTransitionTime:
description: Last time the condition transitioned from one status
to another. This should be when the underlying condition changed.
If that is not known, then using the time when the API field
changed is acceptable.
format: date-time
type: string
message:
description: A human readable message indicating details about
the transition. This field may be empty.
type: string
reason:
description: The reason for the condition's last transition
in CamelCase. The specific API may choose whether or not this
field is considered a guaranteed API. This field may not be
empty.
type: string
severity:
description: Severity provides an explicit classification of
Reason code, so the users or machines can immediately understand
the current situation and act accordingly. The Severity field
MUST be set only when Status=False.
type: string
status:
description: Status of the condition, one of True, False, Unknown.
type: string
type:
description: Type of condition in CamelCase or in foo.example.com/CamelCase.
Many .condition.type values are consistent across resources
like Available, but because arbitrary conditions can be useful
(see .node.status.conditions), the ability to deconflict is
important.
type: string
required:
- status
- type
type: object
type: array
observedGeneration:
description: The generation observed by the deployment controller.
format: int64
Expand Down
26 changes: 25 additions & 1 deletion controllers/machinedeployment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/cluster-api/util/predicates"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -129,7 +130,12 @@ func (r *MachineDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Re

defer func() {
// Always attempt to patch the object and status after each reconciliation.
if err := patchHelper.Patch(ctx, deployment); err != nil {
// Patch ObservedGeneration only if the reconciliation completed successfully
patchOpts := []patch.Option{}
if reterr == nil {
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
}
if err := patchMachineDeployment(ctx, patchHelper, deployment, patchOpts...); err != nil {
reterr = kerrors.NewAggregate([]error{reterr, err})
}
}()
Expand All @@ -148,6 +154,24 @@ func (r *MachineDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Re
return result, err
}

func patchMachineDeployment(ctx context.Context, patchHelper *patch.Helper, d *clusterv1.MachineDeployment, options ...patch.Option) error {
// Always update the readyCondition by summarizing the state of other conditions.
conditions.SetSummary(d,
conditions.WithConditions(
clusterv1.MachineDeploymentAvailableCondition,
),
)

// Patch the object, ignoring conflicts on the conditions owned by this controller.
options = append(options,
patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
clusterv1.ReadyCondition,
clusterv1.MachineDeploymentAvailableCondition,
}},
)
return patchHelper.Patch(ctx, d, options...)
}

func (r *MachineDeploymentReconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster, d *clusterv1.MachineDeployment) (ctrl.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.V(4).Info("Reconcile MachineDeployment")
Expand Down
8 changes: 8 additions & 0 deletions controllers/machinedeployment_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
"sigs.k8s.io/cluster-api/controllers/external"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/conditions"
)

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

t.Log("Verifying MachineDeployment has correct Conditions")
g.Eventually(func() bool {
key := client.ObjectKey{Name: deployment.Name, Namespace: deployment.Namespace}
g.Expect(testEnv.Get(ctx, key, deployment)).To(Succeed())
return conditions.IsTrue(deployment, clusterv1.MachineDeploymentAvailableCondition)
}, timeout).Should(BeTrue())

// Validate that the controller set the cluster name label in selector.
g.Expect(deployment.Status.Selector).To(ContainSubstring(testCluster.Name))
})
Expand Down
12 changes: 12 additions & 0 deletions controllers/machinedeployment_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
"sigs.k8s.io/cluster-api/controllers/mdutil"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -353,6 +354,16 @@ func (r *MachineDeploymentReconciler) scale(ctx context.Context, deployment *clu
// syncDeploymentStatus checks if the status is up-to-date and sync it if necessary.
func (r *MachineDeploymentReconciler) syncDeploymentStatus(allMSs []*clusterv1.MachineSet, newMS *clusterv1.MachineSet, d *clusterv1.MachineDeployment) error {
d.Status = calculateStatus(allMSs, newMS, d)

// minReplicasNeeded will be equal to d.Spec.Replicas when the strategy is not RollingUpdateMachineDeploymentStrategyType.
minReplicasNeeded := *(d.Spec.Replicas) - mdutil.MaxUnavailable(*d)

if d.Status.AvailableReplicas >= minReplicasNeeded {
// 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.
conditions.MarkTrue(d, clusterv1.MachineDeploymentAvailableCondition)
} else {
conditions.MarkFalse(d, clusterv1.MachineDeploymentAvailableCondition, clusterv1.WaitingForAvailableMachinesReason, clusterv1.ConditionSeverityWarning, "Minimum availability requires %d replicas, current %d available", minReplicasNeeded, d.Status.AvailableReplicas)
}
return nil
}

Expand Down Expand Up @@ -380,6 +391,7 @@ func calculateStatus(allMSs []*clusterv1.MachineSet, newMS *clusterv1.MachineSet
ReadyReplicas: mdutil.GetReadyReplicaCountForMachineSets(allMSs),
AvailableReplicas: availableReplicas,
UnavailableReplicas: unavailableReplicas,
Conditions: deployment.Status.Conditions,
}

if *deployment.Spec.Replicas == status.ReadyReplicas {
Expand Down
98 changes: 98 additions & 0 deletions controllers/machinedeployment_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

. "github.com/onsi/gomega"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"
Expand Down Expand Up @@ -394,3 +395,100 @@ func TestScaleMachineSet(t *testing.T) {
})
}
}

func newTestMachineDeployment(pds *int32, replicas, statusReplicas, updatedReplicas, availableReplicas int32, conditions clusterv1.Conditions) *clusterv1.MachineDeployment {
d := &clusterv1.MachineDeployment{
ObjectMeta: metav1.ObjectMeta{
Name: "progress-test",
},
Spec: clusterv1.MachineDeploymentSpec{
ProgressDeadlineSeconds: pds,
Replicas: &replicas,
Strategy: &clusterv1.MachineDeploymentStrategy{
Type: clusterv1.RollingUpdateMachineDeploymentStrategyType,
RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{
MaxUnavailable: intOrStrPtr(0),
MaxSurge: intOrStrPtr(1),
DeletePolicy: pointer.StringPtr("Oldest"),
},
},
},
Status: clusterv1.MachineDeploymentStatus{
Replicas: statusReplicas,
UpdatedReplicas: updatedReplicas,
AvailableReplicas: availableReplicas,
Conditions: conditions,
},
}
return d
}

// helper to create MS with given availableReplicas.
func newTestMachinesetWithReplicas(name string, specReplicas, statusReplicas, availableReplicas int32) *clusterv1.MachineSet {
return &clusterv1.MachineSet{
ObjectMeta: metav1.ObjectMeta{
Name: name,
CreationTimestamp: metav1.Time{},
Namespace: metav1.NamespaceDefault,
},
Spec: clusterv1.MachineSetSpec{
Replicas: pointer.Int32Ptr(specReplicas),
},
Status: clusterv1.MachineSetStatus{
AvailableReplicas: availableReplicas,
Replicas: statusReplicas,
},
}
}

func TestSyncDeploymentStatus(t *testing.T) {
pds := int32(60)
tests := []struct {
name string
d *clusterv1.MachineDeployment
oldMachineSets []*clusterv1.MachineSet
newMachineSet *clusterv1.MachineSet
expectedConditions []*clusterv1.Condition
}{
{
name: "Deployment not available: MachineDeploymentAvailableCondition should exist and be false",
d: newTestMachineDeployment(&pds, 3, 2, 2, 2, clusterv1.Conditions{}),
oldMachineSets: []*clusterv1.MachineSet{},
newMachineSet: newTestMachinesetWithReplicas("foo", 3, 2, 2),
expectedConditions: []*clusterv1.Condition{
{
Type: clusterv1.MachineDeploymentAvailableCondition,
Status: corev1.ConditionFalse,
Severity: clusterv1.ConditionSeverityWarning,
Reason: clusterv1.WaitingForAvailableMachinesReason,
},
},
},
{
name: "Deployment Available: MachineDeploymentAvailableCondition should exist and be true",
d: newTestMachineDeployment(&pds, 3, 3, 3, 3, clusterv1.Conditions{}),
oldMachineSets: []*clusterv1.MachineSet{},
newMachineSet: newTestMachinesetWithReplicas("foo", 3, 3, 3),
expectedConditions: []*clusterv1.Condition{
{
Type: clusterv1.MachineDeploymentAvailableCondition,
Status: corev1.ConditionTrue,
},
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := NewWithT(t)
r := &MachineDeploymentReconciler{
Client: fake.NewClientBuilder().Build(),
recorder: record.NewFakeRecorder(32),
}
allMachineSets := append(test.oldMachineSets, test.newMachineSet)
err := r.syncDeploymentStatus(allMachineSets, test.newMachineSet, test.d)
g.Expect(err).ToNot(HaveOccurred())
assertConditions(t, test.d, test.expectedConditions...)
})
}
}

0 comments on commit 377cd74

Please sign in to comment.