Merge pull request #10920 from muang0/readiness-generation-check

Verify generation in readiness checks
pull/12698/head
Matt Farina 6 months ago committed by GitHub
commit b299359f66
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -183,11 +183,30 @@ func (c *ReadyChecker) IsReady(ctx context.Context, v *resource.Info) (bool, err
if !c.statefulSetReady(sts) {
return false, nil
}
case *corev1.ReplicationController, *extensionsv1beta1.ReplicaSet, *appsv1beta2.ReplicaSet, *appsv1.ReplicaSet:
ok, err = c.podsReadyForObject(ctx, v.Namespace, value)
}
if !ok || err != nil {
return false, err
case *corev1.ReplicationController:
rc, err := c.client.CoreV1().ReplicationControllers(v.Namespace).Get(ctx, v.Name, metav1.GetOptions{})
if err != nil {
return false, err
}
if !c.replicationControllerReady(rc) {
return false, nil
}
ready, err := c.podsReadyForObject(ctx, v.Namespace, value)
if !ready || err != nil {
return false, err
}
case *extensionsv1beta1.ReplicaSet, *appsv1beta2.ReplicaSet, *appsv1.ReplicaSet:
rs, err := c.client.AppsV1().ReplicaSets(v.Namespace).Get(ctx, v.Name, metav1.GetOptions{})
if err != nil {
return false, err
}
if !c.replicaSetReady(rs) {
return false, nil
}
ready, err := c.podsReadyForObject(ctx, v.Namespace, value)
if !ready || err != nil {
return false, err
}
}
return true, nil
}
@ -276,6 +295,16 @@ func (c *ReadyChecker) volumeReady(v *corev1.PersistentVolumeClaim) bool {
}
func (c *ReadyChecker) deploymentReady(rs *appsv1.ReplicaSet, dep *appsv1.Deployment) bool {
// Verify the replicaset readiness
if !c.replicaSetReady(rs) {
return false
}
// Verify the generation observed by the deployment controller matches the spec generation
if dep.Status.ObservedGeneration != dep.ObjectMeta.Generation {
c.log("Deployment is not ready: %s/%s. observedGeneration (%d) does not match spec generation (%d).", dep.Namespace, dep.Name, dep.Status.ObservedGeneration, dep.ObjectMeta.Generation)
return false
}
expectedReady := *dep.Spec.Replicas - deploymentutil.MaxUnavailable(*dep)
if !(rs.Status.ReadyReplicas >= expectedReady) {
c.log("Deployment is not ready: %s/%s. %d out of %d expected pods are ready", dep.Namespace, dep.Name, rs.Status.ReadyReplicas, expectedReady)
@ -285,6 +314,12 @@ func (c *ReadyChecker) deploymentReady(rs *appsv1.ReplicaSet, dep *appsv1.Deploy
}
func (c *ReadyChecker) daemonSetReady(ds *appsv1.DaemonSet) bool {
// Verify the generation observed by the daemonSet controller matches the spec generation
if ds.Status.ObservedGeneration != ds.ObjectMeta.Generation {
c.log("DaemonSet is not ready: %s/%s. observedGeneration (%d) does not match spec generation (%d).", ds.Namespace, ds.Name, ds.Status.ObservedGeneration, ds.ObjectMeta.Generation)
return false
}
// If the update strategy is not a rolling update, there will be nothing to wait for
if ds.Spec.UpdateStrategy.Type != appsv1.RollingUpdateDaemonSetStrategyType {
return true
@ -355,22 +390,22 @@ func (c *ReadyChecker) crdReady(crd apiextv1.CustomResourceDefinition) bool {
}
func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool {
// Verify the generation observed by the statefulSet controller matches the spec generation
if sts.Status.ObservedGeneration != sts.ObjectMeta.Generation {
c.log("StatefulSet is not ready: %s/%s. observedGeneration (%d) does not match spec generation (%d).", sts.Namespace, sts.Name, sts.Status.ObservedGeneration, sts.ObjectMeta.Generation)
return false
}
// If the update strategy is not a rolling update, there will be nothing to wait for
if sts.Spec.UpdateStrategy.Type != appsv1.RollingUpdateStatefulSetStrategyType {
c.log("StatefulSet skipped ready check: %s/%s. updateStrategy is %v", sts.Namespace, sts.Name, sts.Spec.UpdateStrategy.Type)
return true
}
// Make sure the status is up-to-date with the StatefulSet changes
if sts.Status.ObservedGeneration < sts.Generation {
c.log("StatefulSet is not ready: %s/%s. update has not yet been observed", sts.Namespace, sts.Name)
return false
}
// Dereference all the pointers because StatefulSets like them
var partition int
// 1 is the default for replicas if not set
var replicas = 1
replicas := 1
// For some reason, even if the update strategy is a rolling update, the
// actual rollingUpdate field can be nil. If it is, we can safely assume
// there is no partition value
@ -409,6 +444,24 @@ func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool {
return true
}
func (c *ReadyChecker) replicationControllerReady(rc *corev1.ReplicationController) bool {
// Verify the generation observed by the replicationController controller matches the spec generation
if rc.Status.ObservedGeneration != rc.ObjectMeta.Generation {
c.log("ReplicationController is not ready: %s/%s. observedGeneration (%d) does not match spec generation (%d).", rc.Namespace, rc.Name, rc.Status.ObservedGeneration, rc.ObjectMeta.Generation)
return false
}
return true
}
func (c *ReadyChecker) replicaSetReady(rs *appsv1.ReplicaSet) bool {
// Verify the generation observed by the replicaSet controller matches the spec generation
if rs.Status.ObservedGeneration != rs.ObjectMeta.Generation {
c.log("ReplicaSet is not ready: %s/%s. observedGeneration (%d) does not match spec generation (%d).", rs.Namespace, rs.Name, rs.Status.ObservedGeneration, rs.ObjectMeta.Generation)
return false
}
return true
}
func getPods(ctx context.Context, client kubernetes.Interface, namespace, selector string) ([]corev1.Pod, error) {
list, err := client.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{
LabelSelector: selector,

@ -43,27 +43,51 @@ func Test_ReadyChecker_deploymentReady(t *testing.T) {
{
name: "deployment is ready",
args: args{
rs: newReplicaSet("foo", 1, 1),
dep: newDeployment("foo", 1, 1, 0),
rs: newReplicaSet("foo", 1, 1, true),
dep: newDeployment("foo", 1, 1, 0, true),
},
want: true,
},
{
name: "deployment is not ready",
args: args{
rs: newReplicaSet("foo", 0, 0),
dep: newDeployment("foo", 1, 1, 0),
rs: newReplicaSet("foo", 0, 0, true),
dep: newDeployment("foo", 1, 1, 0, true),
},
want: false,
},
{
name: "deployment is ready when maxUnavailable is set",
args: args{
rs: newReplicaSet("foo", 2, 1),
dep: newDeployment("foo", 2, 1, 1),
rs: newReplicaSet("foo", 2, 1, true),
dep: newDeployment("foo", 2, 1, 1, true),
},
want: true,
},
{
name: "deployment is not ready when replicaset generations are out of sync",
args: args{
rs: newReplicaSet("foo", 1, 1, false),
dep: newDeployment("foo", 1, 1, 0, true),
},
want: false,
},
{
name: "deployment is not ready when deployment generations are out of sync",
args: args{
rs: newReplicaSet("foo", 1, 1, true),
dep: newDeployment("foo", 1, 1, 0, false),
},
want: false,
},
{
name: "deployment is not ready when generations are out of sync",
args: args{
rs: newReplicaSet("foo", 1, 1, false),
dep: newDeployment("foo", 1, 1, 0, false),
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -75,6 +99,74 @@ func Test_ReadyChecker_deploymentReady(t *testing.T) {
}
}
func Test_ReadyChecker_replicaSetReady(t *testing.T) {
type args struct {
rs *appsv1.ReplicaSet
}
tests := []struct {
name string
args args
want bool
}{
{
name: "replicaSet is ready",
args: args{
rs: newReplicaSet("foo", 1, 1, true),
},
want: true,
},
{
name: "replicaSet is not ready when generations are out of sync",
args: args{
rs: newReplicaSet("foo", 1, 1, false),
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := NewReadyChecker(fake.NewSimpleClientset(), nil)
if got := c.replicaSetReady(tt.args.rs); got != tt.want {
t.Errorf("replicaSetReady() = %v, want %v", got, tt.want)
}
})
}
}
func Test_ReadyChecker_replicationControllerReady(t *testing.T) {
type args struct {
rc *corev1.ReplicationController
}
tests := []struct {
name string
args args
want bool
}{
{
name: "replicationController is ready",
args: args{
rc: newReplicationController("foo", true),
},
want: true,
},
{
name: "replicationController is not ready when generations are out of sync",
args: args{
rc: newReplicationController("foo", false),
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := NewReadyChecker(fake.NewSimpleClientset(), nil)
if got := c.replicationControllerReady(tt.args.rc); got != tt.want {
t.Errorf("replicationControllerReady() = %v, want %v", got, tt.want)
}
})
}
}
func Test_ReadyChecker_daemonSetReady(t *testing.T) {
type args struct {
ds *appsv1.DaemonSet
@ -87,31 +179,38 @@ func Test_ReadyChecker_daemonSetReady(t *testing.T) {
{
name: "daemonset is ready",
args: args{
ds: newDaemonSet("foo", 0, 1, 1, 1),
ds: newDaemonSet("foo", 0, 1, 1, 1, true),
},
want: true,
},
{
name: "daemonset is not ready",
args: args{
ds: newDaemonSet("foo", 0, 0, 1, 1),
ds: newDaemonSet("foo", 0, 0, 1, 1, true),
},
want: false,
},
{
name: "daemonset pods have not been scheduled successfully",
args: args{
ds: newDaemonSet("foo", 0, 0, 1, 0),
ds: newDaemonSet("foo", 0, 0, 1, 0, true),
},
want: false,
},
{
name: "daemonset is ready when maxUnavailable is set",
args: args{
ds: newDaemonSet("foo", 1, 1, 2, 2),
ds: newDaemonSet("foo", 1, 1, 2, 2, true),
},
want: true,
},
{
name: "daemonset is not ready when generations are out of sync",
args: args{
ds: newDaemonSet("foo", 0, 1, 1, 1, false),
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -135,56 +234,49 @@ func Test_ReadyChecker_statefulSetReady(t *testing.T) {
{
name: "statefulset is ready",
args: args{
sts: newStatefulSet("foo", 1, 0, 1, 1),
sts: newStatefulSet("foo", 1, 0, 1, 1, true),
},
want: true,
},
{
name: "statefulset is not ready",
args: args{
sts: newStatefulSet("foo", 1, 0, 0, 1),
sts: newStatefulSet("foo", 1, 0, 0, 1, true),
},
want: false,
},
{
name: "statefulset is ready when partition is specified",
args: args{
sts: newStatefulSet("foo", 2, 1, 2, 1),
sts: newStatefulSet("foo", 2, 1, 2, 1, true),
},
want: true,
},
{
name: "statefulset is not ready when partition is set",
args: args{
sts: newStatefulSet("foo", 2, 1, 1, 0),
sts: newStatefulSet("foo", 2, 1, 1, 0, true),
},
want: false,
},
{
name: "statefulset is ready when partition is set and no change in template",
args: args{
sts: newStatefulSet("foo", 2, 1, 2, 2),
sts: newStatefulSet("foo", 2, 1, 2, 2, true),
},
want: true,
},
{
name: "statefulset is ready when partition is greater than replicas",
args: args{
sts: newStatefulSet("foo", 1, 2, 1, 1),
sts: newStatefulSet("foo", 1, 2, 1, 1, true),
},
want: true,
},
{
name: "statefulset is not ready when status of latest generation has not yet been observed",
args: args{
sts: newStatefulSetWithNewGeneration("foo", 1, 0, 1, 1),
},
want: false,
},
{
name: "statefulset is not ready when current revision for current replicas does not match update revision for updated replicas",
name: "statefulset is not ready when generations are out of sync",
args: args{
sts: newStatefulSetWithUpdateRevision("foo", 1, 0, 1, 1, "foo-bbbbbbb"),
sts: newStatefulSet("foo", 1, 0, 1, 1, false),
},
want: false,
},
@ -222,7 +314,7 @@ func Test_ReadyChecker_podsReadyForObject(t *testing.T) {
name: "pods ready for a replicaset",
args: args{
namespace: defaultNamespace,
obj: newReplicaSet("foo", 1, 1),
obj: newReplicaSet("foo", 1, 1, true),
},
existPods: []corev1.Pod{
*newPodWithCondition("foo", corev1.ConditionTrue),
@ -234,7 +326,7 @@ func Test_ReadyChecker_podsReadyForObject(t *testing.T) {
name: "pods not ready for a replicaset",
args: args{
namespace: defaultNamespace,
obj: newReplicaSet("foo", 1, 1),
obj: newReplicaSet("foo", 1, 1, true),
},
existPods: []corev1.Pod{
*newPodWithCondition("foo", corev1.ConditionFalse),
@ -371,11 +463,16 @@ func Test_ReadyChecker_volumeReady(t *testing.T) {
}
}
func newDaemonSet(name string, maxUnavailable, numberReady, desiredNumberScheduled, updatedNumberScheduled int) *appsv1.DaemonSet {
func newDaemonSet(name string, maxUnavailable, numberReady, desiredNumberScheduled, updatedNumberScheduled int, generationInSync bool) *appsv1.DaemonSet {
var generation, observedGeneration int64 = 1, 1
if !generationInSync {
generation = 2
}
return &appsv1.DaemonSet{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: defaultNamespace,
Name: name,
Namespace: defaultNamespace,
Generation: generation,
},
Spec: appsv1.DaemonSetSpec{
UpdateStrategy: appsv1.DaemonSetUpdateStrategy{
@ -403,16 +500,21 @@ func newDaemonSet(name string, maxUnavailable, numberReady, desiredNumberSchedul
DesiredNumberScheduled: int32(desiredNumberScheduled),
NumberReady: int32(numberReady),
UpdatedNumberScheduled: int32(updatedNumberScheduled),
ObservedGeneration: observedGeneration,
},
}
}
func newStatefulSet(name string, replicas, partition, readyReplicas, updatedReplicas int) *appsv1.StatefulSet {
func newStatefulSet(name string, replicas, partition, readyReplicas, updatedReplicas int, generationInSync bool) *appsv1.StatefulSet {
var generation, observedGeneration int64 = 1, 1
if !generationInSync {
generation = 2
}
return &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: defaultNamespace,
Generation: int64(1),
Generation: generation,
},
Spec: appsv1.StatefulSetSpec{
UpdateStrategy: appsv1.StatefulSetUpdateStrategy{
@ -438,32 +540,23 @@ func newStatefulSet(name string, replicas, partition, readyReplicas, updatedRepl
},
},
Status: appsv1.StatefulSetStatus{
ObservedGeneration: int64(1),
CurrentRevision: name + "-aaaaaaa",
UpdateRevision: name + "-aaaaaaa",
UpdatedReplicas: int32(updatedReplicas),
ReadyReplicas: int32(readyReplicas),
ObservedGeneration: observedGeneration,
},
}
}
func newStatefulSetWithNewGeneration(name string, replicas, partition, readyReplicas, updatedReplicas int) *appsv1.StatefulSet {
ss := newStatefulSet(name, replicas, partition, readyReplicas, updatedReplicas)
ss.Generation++
return ss
}
func newStatefulSetWithUpdateRevision(name string, replicas, partition, readyReplicas, updatedReplicas int, updateRevision string) *appsv1.StatefulSet {
ss := newStatefulSet(name, replicas, partition, readyReplicas, updatedReplicas)
ss.Status.UpdateRevision = updateRevision
return ss
}
func newDeployment(name string, replicas, maxSurge, maxUnavailable int) *appsv1.Deployment {
func newDeployment(name string, replicas, maxSurge, maxUnavailable int, generationInSync bool) *appsv1.Deployment {
var generation, observedGeneration int64 = 1, 1
if !generationInSync {
generation = 2
}
return &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: defaultNamespace,
Name: name,
Namespace: defaultNamespace,
Generation: generation,
},
Spec: appsv1.DeploymentSpec{
Strategy: appsv1.DeploymentStrategy{
@ -489,17 +582,37 @@ func newDeployment(name string, replicas, maxSurge, maxUnavailable int) *appsv1.
},
},
},
Status: appsv1.DeploymentStatus{
ObservedGeneration: observedGeneration,
},
}
}
func newReplicationController(name string, generationInSync bool) *corev1.ReplicationController {
var generation, observedGeneration int64 = 1, 1
if !generationInSync {
generation = 2
}
return &corev1.ReplicationController{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Generation: generation,
},
Status: corev1.ReplicationControllerStatus{
ObservedGeneration: observedGeneration,
},
}
}
func newReplicaSet(name string, replicas int, readyReplicas int) *appsv1.ReplicaSet {
d := newDeployment(name, replicas, 0, 0)
func newReplicaSet(name string, replicas int, readyReplicas int, generationInSync bool) *appsv1.ReplicaSet {
d := newDeployment(name, replicas, 0, 0, generationInSync)
return &appsv1.ReplicaSet{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: defaultNamespace,
Labels: d.Spec.Selector.MatchLabels,
OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(d, d.GroupVersionKind())},
Generation: d.Generation,
},
Spec: appsv1.ReplicaSetSpec{
Selector: d.Spec.Selector,
@ -507,7 +620,8 @@ func newReplicaSet(name string, replicas int, readyReplicas int) *appsv1.Replica
Template: d.Spec.Template,
},
Status: appsv1.ReplicaSetStatus{
ReadyReplicas: int32(readyReplicas),
ReadyReplicas: int32(readyReplicas),
ObservedGeneration: d.Status.ObservedGeneration,
},
}
}

Loading…
Cancel
Save