Skip to content

Commit

Permalink
Don't monitor health if single node rolling upgrade in E2E tests (#1804)
Browse files Browse the repository at this point in the history
* Don't monitor health if single node rolling upgrade

Disable continuous health check monitoring if we're doing a rolling
upgrade of a 1-node cluster.

This test was flappy, since a 1-node cluster upgrade will temporarily be
Red while shards are initializing after the upgrade.

The PR also introduces an optional "Skip" field in the TestStep. If
defined, a function returning True will disable the associated test.

* Check one-data-node clusters instead of one-node clusters

* Log skipped tests
  • Loading branch information
sebgl authored Sep 27, 2019
1 parent 339075b commit a93f475
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
30 changes: 27 additions & 3 deletions test/e2e/test/elasticsearch/steps_mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,15 @@ func (b Builder) MutationTestSteps(k *test.K8sClient) test.StepList {
},
test.Step{
Name: "Start querying Elasticsearch cluster health while mutation is going on",
Skip: func() bool {
// Don't monitor cluster health if we're doing a rolling upgrade of a one data node cluster.
// The cluster will become unavailable at some point, then its health will be red
// after the upgrade while shards are initializing.
return IsOneDataNodeRollingUpgrade(b)
},
Test: func(t *testing.T) {
var err error
continuousHealthChecks, err = NewContinuousHealthCheck(b.Elasticsearch, k)
continuousHealthChecks, err = NewContinuousHealthCheck(b, k)
require.NoError(t, err)
continuousHealthChecks.Start()
},
Expand Down Expand Up @@ -80,6 +86,9 @@ func (b Builder) MutationTestSteps(k *test.K8sClient) test.StepList {
},
test.Step{
Name: "Elasticsearch cluster health should not have been red during mutation process",
Skip: func() bool {
return IsOneDataNodeRollingUpgrade(b)
},
Test: func(t *testing.T) {
continuousHealthChecks.Stop()
assert.Equal(t, 0, continuousHealthChecks.FailureCount)
Expand All @@ -97,6 +106,21 @@ func (b Builder) MutationTestSteps(k *test.K8sClient) test.StepList {
})
}

func IsOneDataNodeRollingUpgrade(b Builder) bool {
if b.MutatedFrom == nil {
return false
}
initial := b.MutatedFrom.Elasticsearch
mutated := b.Elasticsearch
// consider we're in the 1-node rolling upgrade scenario if we mutate
// from one data node to one data node with the same name
if MustNumDataNodes(initial) == 1 && MustNumDataNodes(mutated) == 1 &&
initial.Spec.Nodes[0].Name == mutated.Spec.Nodes[0].Name {
return true
}
return false
}

// ContinuousHealthCheckFailure represents an health check failure
type ContinuousHealthCheckFailure struct {
err error
Expand All @@ -114,8 +138,8 @@ type ContinuousHealthCheck struct {
}

// NewContinuousHealthCheck sets up a ContinuousHealthCheck struct
func NewContinuousHealthCheck(es estype.Elasticsearch, k *test.K8sClient) (*ContinuousHealthCheck, error) {
esClient, err := NewElasticsearchClient(es, k)
func NewContinuousHealthCheck(b Builder, k *test.K8sClient) (*ContinuousHealthCheck, error) {
esClient, err := NewElasticsearchClient(b.Elasticsearch, k)
if err != nil {
return nil, err
}
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/test/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
type Step struct {
Name string
Test func(t *testing.T)
Skip func() bool // returns true if the test should be skipped
}

// StepList defines a list of Step
Expand All @@ -33,6 +34,10 @@ func (l StepList) WithStep(testStep Step) StepList {
// RunSequential runs the StepList sequentially, and fails fast on first error.
func (l StepList) RunSequential(t *testing.T) {
for _, ts := range l {
if ts.Skip != nil && ts.Skip() {
log.Info("Skipping test", "name", ts.Name)
continue
}
if !t.Run(ts.Name, ts.Test) {
logf.Log.Error(errors.New("test failure"), "stopping early")
t.FailNow()
Expand Down

0 comments on commit a93f475

Please sign in to comment.