From 28305b5f3ccce82a901301db56ec9ffb9fbe20fa Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Mon, 7 Apr 2025 15:12:27 +0200 Subject: [PATCH] Rewording to match code reviews feedbacks and fix specs Signed-off-by: Benoit Tigeot --- pkg/kube/client.go | 39 ----------------------------- pkg/kube/ready.go | 56 +++++++++++++++++++++--------------------- pkg/kube/ready_test.go | 6 ----- pkg/kube/wait.go | 4 +-- 4 files changed, 30 insertions(+), 75 deletions(-) diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 9768d41cf..a62b83b3e 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -340,45 +340,6 @@ func getResource(info *resource.Info) (runtime.Object, error) { return obj, nil } -// Wait waits up to the given timeout for the specified resources to be ready. -func (c *Client) Wait(resources ResourceList, timeout time.Duration) error { - cs, err := c.getKubeClient() - if err != nil { - return err - } - checker := NewReadyChecker(cs, PausedAsReady(true)) - w := waiter{ - c: checker, - log: c.Log, - timeout: timeout, - } - return w.waitForResources(resources) -} - -// WaitWithJobs wait up to the given timeout for the specified resources to be ready, including jobs. -func (c *Client) WaitWithJobs(resources ResourceList, timeout time.Duration) error { - cs, err := c.getKubeClient() - if err != nil { - return err - } - checker := NewReadyChecker(cs, PausedAsReady(true), CheckJobs(true)) - w := waiter{ - c: checker, - log: c.Log, - timeout: timeout, - } - return w.waitForResources(resources) -} - -// WaitForDelete wait up to the given timeout for the specified resources to be deleted. -func (c *Client) WaitForDelete(resources ResourceList, timeout time.Duration) error { - w := waiter{ - log: c.Log, - timeout: timeout, - } - return w.waitForDeletedResources(resources) -} - func (c *Client) namespace() string { if c.Namespace != "" { return c.Namespace diff --git a/pkg/kube/ready.go b/pkg/kube/ready.go index b9a80208f..74578b858 100644 --- a/pkg/kube/ready.go +++ b/pkg/kube/ready.go @@ -287,17 +287,17 @@ func (c *ReadyChecker) deploymentReady(rs *appsv1.ReplicaSet, dep *appsv1.Deploy } // Verify the generation observed by the deployment controller matches the spec generation if dep.Status.ObservedGeneration != dep.ObjectMeta.Generation { - slog.Debug("Deployment is not ready: observedGeneration does not match spec generation", + slog.Debug("Deployment generation does not match spec", "namespace", dep.Namespace, "name", dep.Name, - "observedGeneration", dep.Status.ObservedGeneration, - "specGeneration", dep.ObjectMeta.Generation) + "actualGeneration", dep.Status.ObservedGeneration, + "expectedGeneration", dep.ObjectMeta.Generation) return false } expectedReady := *dep.Spec.Replicas - deploymentutil.MaxUnavailable(*dep) if !(rs.Status.ReadyReplicas >= expectedReady) { - slog.Debug("Deployment is not ready: not enough pods ready", + slog.Debug("Deployment does not have enough pods ready", "namespace", dep.Namespace, "name", dep.Name, "readyReplicas", rs.Status.ReadyReplicas, @@ -310,11 +310,11 @@ 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 { - slog.Debug("DaemonSet is not ready: observedGeneration does not match spec generation", + slog.Debug("DaemonSet generation does not match spec", "namespace", ds.Namespace, "name", ds.Name, - "observedGeneration", ds.Status.ObservedGeneration, - "specGeneration", ds.ObjectMeta.Generation) + "actualGeneration", ds.Status.ObservedGeneration, + "expectedReplicas", ds.ObjectMeta.Generation) return false } @@ -325,11 +325,11 @@ func (c *ReadyChecker) daemonSetReady(ds *appsv1.DaemonSet) bool { // Make sure all the updated pods have been scheduled if ds.Status.UpdatedNumberScheduled != ds.Status.DesiredNumberScheduled { - slog.Debug("DaemonSet is not ready: not enough pods scheduled", + slog.Debug("DaemonSet does not have enough pods scheduled", "namespace", ds.Namespace, "name", ds.Name, "updatedScheduled", ds.Status.UpdatedNumberScheduled, - "desiredScheduled", ds.Status.DesiredNumberScheduled) + "expectedScheduled", ds.Status.DesiredNumberScheduled) return false } maxUnavailable, err := intstr.GetScaledValueFromIntOrPercent(ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable, int(ds.Status.DesiredNumberScheduled), true) @@ -342,7 +342,7 @@ func (c *ReadyChecker) daemonSetReady(ds *appsv1.DaemonSet) bool { expectedReady := int(ds.Status.DesiredNumberScheduled) - maxUnavailable if !(int(ds.Status.NumberReady) >= expectedReady) { - slog.Debug("DaemonSet is not ready: not enough pods ready", + slog.Debug("DaemonSet does not have enough pods ready", "namespace", ds.Namespace, "name", ds.Name, "readyPods", ds.Status.NumberReady, @@ -398,11 +398,11 @@ 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 { - slog.Debug("StatefulSet is not ready: observedGeneration does not match spec generation", + slog.Debug("StatefulSet does not match spec generation", "namespace", sts.Namespace, "name", sts.Name, - "observedGeneration", sts.Status.ObservedGeneration, - "specGeneration", sts.ObjectMeta.Generation) + "actualGeneration", sts.Status.ObservedGeneration, + "expectedGeneration", sts.ObjectMeta.Generation) return false } @@ -437,35 +437,35 @@ func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool { // Make sure all the updated pods have been scheduled if int(sts.Status.UpdatedReplicas) < expectedReplicas { - slog.Debug("StatefulSet is not ready: not enough pods scheduled", + slog.Debug("StatefulSet does not have enough pods scheduled", "namespace", sts.Namespace, "name", sts.Name, - "updatedReplicas", sts.Status.UpdatedReplicas, - "expectedReplicas", expectedReplicas) + "updatedScheduled", sts.Status.UpdatedReplicas, + "expectedScheduled", expectedReplicas) return false } if int(sts.Status.ReadyReplicas) != replicas { - slog.Debug("StatefulSet is not ready: not enough pods ready", + slog.Debug("StatefulSet does not have enough pods ready", "namespace", sts.Namespace, "name", sts.Name, - "readyReplicas", sts.Status.ReadyReplicas, - "expectedReplicas", replicas) + "readyPods", sts.Status.ReadyReplicas, + "expectedPods", replicas) return false } // This check only makes sense when all partitions are being upgraded otherwise during a // partitioned rolling upgrade, this condition will never evaluate to true, leading to // error. if partition == 0 && sts.Status.CurrentRevision != sts.Status.UpdateRevision { - slog.Debug("StatefulSet is not ready: currentRevision does not match updateRevision", + slog.Debug("StatefulSet does not have the expected revision", "namespace", sts.Namespace, "name", sts.Name, "currentRevision", sts.Status.CurrentRevision, - "updateRevision", sts.Status.UpdateRevision) + "expectedRevision", sts.Status.UpdateRevision) return false } - slog.Debug("StatefulSet is ready", + slog.Debug("StatefulSet does not have enough pods ready", "namespace", sts.Namespace, "name", sts.Name, "readyReplicas", sts.Status.ReadyReplicas, @@ -476,11 +476,11 @@ func (c *ReadyChecker) statefulSetReady(sts *appsv1.StatefulSet) bool { 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 { - slog.Debug("ReplicationController is not ready: observedGeneration does not match spec generation", + slog.Debug("ReplicationController does not match spec generation", "namespace", rc.Namespace, "name", rc.Name, - "observedGeneration", rc.Status.ObservedGeneration, - "specGeneration", rc.ObjectMeta.Generation) + "actualGeneration", rc.Status.ObservedGeneration, + "expectedGeneration", rc.ObjectMeta.Generation) return false } return true @@ -489,11 +489,11 @@ func (c *ReadyChecker) replicationControllerReady(rc *corev1.ReplicationControll 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 { - slog.Debug("ReplicaSet is not ready: observedGeneration does not match spec generation", + slog.Debug("ReplicaSet does not match spec generation", "namespace", rs.Namespace, "name", rs.Name, - "observedGeneration", rs.Status.ObservedGeneration, - "specGeneration", rs.ObjectMeta.Generation) + "actualGeneration", rs.Status.ObservedGeneration, + "expectedGeneration", rs.ObjectMeta.Generation) return false } return true diff --git a/pkg/kube/ready_test.go b/pkg/kube/ready_test.go index c1a25c38c..9680f71e3 100644 --- a/pkg/kube/ready_test.go +++ b/pkg/kube/ready_test.go @@ -56,7 +56,6 @@ func Test_ReadyChecker_IsReady_Pod(t *testing.T) { name: "IsReady Pod", fields: fields{ client: fake.NewSimpleClientset(), - log: func(string, ...interface{}) {}, checkJobs: true, pausedAsReady: false, }, @@ -72,7 +71,6 @@ func Test_ReadyChecker_IsReady_Pod(t *testing.T) { name: "IsReady Pod returns error", fields: fields{ client: fake.NewSimpleClientset(), - log: func(string, ...interface{}) {}, checkJobs: true, pausedAsReady: false, }, @@ -130,7 +128,6 @@ func Test_ReadyChecker_IsReady_Job(t *testing.T) { name: "IsReady Job error while getting job", fields: fields{ client: fake.NewSimpleClientset(), - log: func(string, ...interface{}) {}, checkJobs: true, pausedAsReady: false, }, @@ -146,7 +143,6 @@ func Test_ReadyChecker_IsReady_Job(t *testing.T) { name: "IsReady Job", fields: fields{ client: fake.NewSimpleClientset(), - log: func(string, ...interface{}) {}, checkJobs: true, pausedAsReady: false, }, @@ -204,7 +200,6 @@ func Test_ReadyChecker_IsReady_Deployment(t *testing.T) { name: "IsReady Deployments error while getting current Deployment", fields: fields{ client: fake.NewSimpleClientset(), - log: func(string, ...interface{}) {}, checkJobs: true, pausedAsReady: false, }, @@ -221,7 +216,6 @@ func Test_ReadyChecker_IsReady_Deployment(t *testing.T) { name: "IsReady Deployments", //TODO fix this one fields: fields{ client: fake.NewSimpleClientset(), - log: func(string, ...interface{}) {}, checkJobs: true, pausedAsReady: false, }, diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go index 79a2df8cc..ecc4ea611 100644 --- a/pkg/kube/wait.go +++ b/pkg/kube/wait.go @@ -55,12 +55,12 @@ type legacyWaiter struct { } func (hw *legacyWaiter) Wait(resources ResourceList, timeout time.Duration) error { - hw.c = NewReadyChecker(hw.kubeClient, hw.log, PausedAsReady(true)) + hw.c = NewReadyChecker(hw.kubeClient, PausedAsReady(true)) return hw.waitForResources(resources, timeout) } func (hw *legacyWaiter) WaitWithJobs(resources ResourceList, timeout time.Duration) error { - hw.c = NewReadyChecker(hw.kubeClient, hw.log, PausedAsReady(true), CheckJobs(true)) + hw.c = NewReadyChecker(hw.kubeClient, PausedAsReady(true), CheckJobs(true)) return hw.waitForResources(resources, timeout) }