Skip to content

Commit

Permalink
Merge pull request #798 from openshift-cherrypick-robot/cherry-pick-7…
Browse files Browse the repository at this point in the history 
…91-to-release-4.10

[release-4.10] Bug 2108292: pkg/cvo: retain initial completed update history entry
  • Loading branch information
@openshift-ci
openshift-ci[bot] committed Jul 19, 2022
2 parents b09fb92 + 79cdca2 commit 288bd4d
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 12 deletions.
25 changes: 14 additions & 11 deletions pkg/cvo/status.go
View file
Expand Up @@ -29,6 +29,10 @@ const (
// cannot reach the desired state. It is considered more serious than Degraded
// and indicates the cluster is not healthy.
ClusterStatusFailing = configv1.ClusterStatusConditionType("Failing")

// MaxHistory is the maximum size of ClusterVersion history. Once exceeded
// ClusterVersion history will be pruned.
MaxHistory = 50
)

func mergeEqualVersions(current *configv1.UpdateHistory, desired configv1.Release) bool {
Expand Down Expand Up @@ -128,26 +132,25 @@ func mergeOperatorHistory(config *configv1.ClusterVersion, desired configv1.Rele
}

// TODO: prune Z versions over transitions to Y versions, keep initial installed version
pruneStatusHistory(config, 50)
pruneStatusHistory(config, MaxHistory)

config.Status.Desired = desired
}

// pruneStatusHistory maintains history size at MaxHistory by removing entry at index MaxHistory
// unless that entry is a completed update in which case entry at MaxHistory-1 is removed thereby
// retaining the initial completed version.
func pruneStatusHistory(config *configv1.ClusterVersion, maxHistory int) {
if len(config.Status.History) <= maxHistory {
return
}
for i, item := range config.Status.History {
if item.State != configv1.CompletedUpdate {
continue
}
// guarantee the last position in the history is always a completed item
if i >= maxHistory {
config.Status.History[maxHistory-1] = item
}
break
if config.Status.History[maxHistory].State == configv1.CompletedUpdate {
item := config.Status.History[maxHistory]
config.Status.History = config.Status.History[0 : maxHistory-1]
config.Status.History = append(config.Status.History, item)
} else {
config.Status.History = config.Status.History[:maxHistory]
}
config.Status.History = config.Status.History[:maxHistory]
}

// ClusterVersionInvalid indicates that the cluster version has an error that prevents the server from
Expand Down
71 changes: 70 additions & 1 deletion pkg/cvo/status_test.go
View file
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"reflect"
"testing"
"time"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/diff"
Expand Down Expand Up @@ -80,14 +81,16 @@ func Test_pruneStatusHistory(t *testing.T) {
want []configv1.UpdateHistory
}{
{
name: "max history 2",
config: obj.DeepCopy(),
maxHistory: 2,
want: []configv1.UpdateHistory{
{State: configv1.PartialUpdate, Version: "0.0.10"},
{State: configv1.CompletedUpdate, Version: "0.0.7"},
{State: configv1.PartialUpdate, Version: "0.0.9"},
},
},
{
name: "max history 3",
config: obj.DeepCopy(),
maxHistory: 3,
want: []configv1.UpdateHistory{
Expand All @@ -97,6 +100,7 @@ func Test_pruneStatusHistory(t *testing.T) {
},
},
{
name: "max history 4",
config: obj.DeepCopy(),
maxHistory: 4,
want: []configv1.UpdateHistory{
Expand All @@ -107,6 +111,7 @@ func Test_pruneStatusHistory(t *testing.T) {
},
},
{
name: "max history 5",
config: obj.DeepCopy(),
maxHistory: 5,
want: []configv1.UpdateHistory{
Expand All @@ -118,6 +123,7 @@ func Test_pruneStatusHistory(t *testing.T) {
},
},
{
name: "max history 6",
config: obj.DeepCopy(),
maxHistory: 6,
want: []configv1.UpdateHistory{
Expand All @@ -134,12 +140,75 @@ func Test_pruneStatusHistory(t *testing.T) {
config := tt.config.DeepCopy()
pruneStatusHistory(config, tt.maxHistory)
if !reflect.DeepEqual(tt.want, config.Status.History) {
t.Logf("%v", config.Status.History)
t.Fatalf("%s", diff.ObjectReflectDiff(tt.want, config.Status.History))
}
})
}
}

func Test_mergeOperatorHistory(t *testing.T) {
obj := &configv1.ClusterVersion{}
var nowT time.Time
now := metav1.NewTime(nowT)
tests := []struct {
name string
config *configv1.ClusterVersion
first configv1.UpdateHistory
second configv1.UpdateHistory
want configv1.UpdateHistory
}{
{
name: "drop partial",
config: obj.DeepCopy(),
first: configv1.UpdateHistory{State: configv1.PartialUpdate, CompletionTime: &now, Version: "0.0.7", Image: "test:0"},
second: configv1.UpdateHistory{State: configv1.CompletedUpdate, CompletionTime: &now, Version: "0.0.6", Image: "test:1"},
want: configv1.UpdateHistory{State: configv1.CompletedUpdate, CompletionTime: &now, Version: "0.0.6", Image: "test:1"},
},
{
name: "keep completed",
config: obj.DeepCopy(),
first: configv1.UpdateHistory{State: configv1.CompletedUpdate, CompletionTime: &now, Version: "0.0.7", Image: "test:0"},
second: configv1.UpdateHistory{State: configv1.PartialUpdate, CompletionTime: &now, Version: "0.0.6", Image: "test:1"},
want: configv1.UpdateHistory{State: configv1.CompletedUpdate, CompletionTime: &now, Version: "0.0.7", Image: "test:0"},
},
{
name: "neither completed",
config: obj.DeepCopy(),
first: configv1.UpdateHistory{State: configv1.PartialUpdate, CompletionTime: &now, Version: "0.0.7", Image: "test:0"},
second: configv1.UpdateHistory{State: configv1.PartialUpdate, CompletionTime: &now, Version: "0.0.6", Image: "test:1"},
want: configv1.UpdateHistory{State: configv1.PartialUpdate, CompletionTime: &now, Version: "0.0.6", Image: "test:1"},
},
{
name: "keep first completed",
config: obj.DeepCopy(),
first: configv1.UpdateHistory{State: configv1.CompletedUpdate, CompletionTime: &now, Version: "0.0.7", Image: "test:0"},
second: configv1.UpdateHistory{State: configv1.CompletedUpdate, CompletionTime: &now, Version: "0.0.6", Image: "test:1"},
want: configv1.UpdateHistory{State: configv1.CompletedUpdate, CompletionTime: &now, Version: "0.0.7", Image: "test:0"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
for i := 0; i <= MaxHistory; i++ {
image := fmt.Sprintf("test:%d", i)
if i == 0 {
mergeOperatorHistory(tt.config, configv1.Release{Image: image, Version: tt.first.Version}, false, now,
(tt.first.State == configv1.CompletedUpdate))
} else if i == 1 {
mergeOperatorHistory(tt.config, configv1.Release{Image: image, Version: tt.second.Version}, false, now,
(tt.second.State == configv1.CompletedUpdate))
} else {
version := fmt.Sprintf("%d", i)
mergeOperatorHistory(tt.config, configv1.Release{Image: image, Version: version}, false, now, true)
}
}
if !reflect.DeepEqual(tt.want, tt.config.Status.History[MaxHistory-1]) {
t.Fatalf("%s", diff.ObjectReflectDiff(tt.want, tt.config.Status.History[MaxHistory-1]))
}
})
}
}

func TestOperator_syncFailingStatus(t *testing.T) {
ctx := context.Background()
tests := []struct {
Expand Down

0 comments on commit 288bd4d

Please sign in to comment.