Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

status: catch a few more Progressing cases #143

Merged
merged 1 commit into from Apr 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 13 additions & 1 deletion pkg/controller/statusmanager/status_manager.go
Expand Up @@ -193,8 +193,12 @@ func (status *StatusManager) SetFromPods() {

if ds.Status.NumberUnavailable > 0 {
progressing = append(progressing, fmt.Sprintf("DaemonSet %q is not available (awaiting %d nodes)", dsName.String(), ds.Status.NumberUnavailable))
} else if ds.Status.NumberAvailable == 0 {
} else if ds.Status.NumberAvailable == 0 { // NOTE: update this if we ever expect empty (unscheduled) daemonsets ~cdc
progressing = append(progressing, fmt.Sprintf("DaemonSet %q is not yet scheduled on any nodes", dsName.String()))
} else if ds.Status.UpdatedNumberScheduled < ds.Status.DesiredNumberScheduled {
progressing = append(progressing, fmt.Sprintf("DaemonSet %q update is rolling out (%d out of %d updated)", dsName.String(), ds.Status.UpdatedNumberScheduled, ds.Status.DesiredNumberScheduled))
} else if ds.Generation > ds.Status.ObservedGeneration {
squeed marked this conversation as resolved.
Show resolved Hide resolved
progressing = append(progressing, fmt.Sprintf("DaemonSet %q update is being processed (generation %d, observed generation %d)", dsName.String(), ds.Generation, ds.Status.ObservedGeneration))
}

if !(ds.Generation <= ds.Status.ObservedGeneration && ds.Status.UpdatedNumberScheduled == ds.Status.DesiredNumberScheduled && ds.Status.NumberUnavailable == 0 && ds.Annotations["release.openshift.io/version"] == targetLevel) {
Expand Down Expand Up @@ -231,6 +235,14 @@ func (status *StatusManager) SetFromPods() {
progressing = append(progressing, fmt.Sprintf("Deployment %q is not available (awaiting %d nodes)", depName.String(), dep.Status.UnavailableReplicas))
} else if dep.Status.AvailableReplicas == 0 {
progressing = append(progressing, fmt.Sprintf("Deployment %q is not yet scheduled on any nodes", depName.String()))
} else if dep.Status.ObservedGeneration < dep.Generation {
progressing = append(progressing, fmt.Sprintf("Deployment %q update is being processed (generation %d, observed generation %d)", depName.String(), dep.Generation, dep.Status.ObservedGeneration))
}

for _, cond := range dep.Status.Conditions {
if cond.Type == appsv1.DeploymentProgressing && cond.Status == corev1.ConditionTrue {
progressing = append(progressing, fmt.Sprintf("Deployment %q is progressing (%q)", depName.String(), cond.Reason))
}
}

if !(dep.Generation <= dep.Status.ObservedGeneration && dep.Status.UpdatedReplicas == dep.Status.Replicas && dep.Status.AvailableReplicas > 0 && dep.Annotations["release.openshift.io/version"] == targetLevel) {
Expand Down
33 changes: 33 additions & 0 deletions pkg/controller/statusmanager/status_manager_test.go
Expand Up @@ -3,6 +3,7 @@ package statusmanager
import (
"context"
"testing"
"time"

configv1 "github.com/openshift/api/config/v1"
"github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers"
Expand Down Expand Up @@ -307,6 +308,17 @@ func TestStatusManagerSetFromDaemonSets(t *testing.T) {
t.Fatalf("unexpected Status.Conditions: %#v", co.Status.Conditions)
}

progressingTS := metav1.Now()
if cond := v1helpers.FindStatusCondition(co.Status.Conditions, configv1.OperatorProgressing); cond != nil {
if cond.LastTransitionTime.IsZero() {
t.Fatalf("progressing transition time was zero")
}
progressingTS = cond.LastTransitionTime
} else {
// unreachable
t.Fatalf("Progressing condition unexpectedly missing")
}

// Update to report expected deployment size
dsANodes := int32(1)
dsBNodes := int32(3)
Expand Down Expand Up @@ -342,6 +354,16 @@ func TestStatusManagerSetFromDaemonSets(t *testing.T) {
t.Fatalf("unexpected Status.Conditions: %#v", co.Status.Conditions)
}

// Validate that the transition time was not bumped
if cond := v1helpers.FindStatusCondition(co.Status.Conditions, configv1.OperatorProgressing); cond != nil {
if !progressingTS.Equal(&cond.LastTransitionTime) {
t.Fatalf("Progressing LastTransitionTime changed unnecessarily")
}
} else {
// unreachable
t.Fatalf("Progressing condition unexpectedly missing")
}

if dsA.Status.NumberUnavailable > 0 {
dsA.Status.NumberUnavailable--
dsA.Status.NumberAvailable++
Expand All @@ -365,6 +387,7 @@ func TestStatusManagerSetFromDaemonSets(t *testing.T) {
if err != nil {
t.Fatalf("error updating DaemonSet: %v", err)
}
time.Sleep(1 * time.Second) // minimum transition time fidelity
status.SetFromPods()

co, err = getCO(client, "testing")
Expand All @@ -387,6 +410,16 @@ func TestStatusManagerSetFromDaemonSets(t *testing.T) {
}) {
t.Fatalf("unexpected Status.Conditions: %#v", co.Status.Conditions)
}

// Validate that the transition time was bumped
if cond := v1helpers.FindStatusCondition(co.Status.Conditions, configv1.OperatorProgressing); cond != nil {
if progressingTS.Equal(&cond.LastTransitionTime) {
t.Fatalf("Progressing LastTransitionTime didn't change when Progressing -> false")
}
} else {
// unreachable
t.Fatalf("Progressing condition unexpectedly missing")
}
}

func TestStatusManagerSetFromPods(t *testing.T) {
Expand Down