Skip to content

Commit

Permalink
fix: various test fixes and refactors
Browse files Browse the repository at this point in the history 
Mostly this is replacing deprecated functions and attempting to reduce
timing dependent behavior by waiting for expected resources to show up.

Signed-off-by: Jan Fajerski <jfajersk@redhat.com>
  • Loading branch information
@jan--f
jan--f committed Feb 19, 2024
1 parent e4d9b94 commit 2e0cfac
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 20 deletions.
46 changes: 39 additions & 7 deletions test/e2e/framework/assertions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ func WithPollInterval(d time.Duration) OptionFn {
}
}

// AssertResourceNeverExists asserts that a statefulset is never created for the duration of DefaultTestTimeout
// AssertResourceNeverExists asserts that a resource is never created for the
// duration of the timeout
func (f *Framework) AssertResourceNeverExists(name, namespace string, resource client.Object, fns ...OptionFn) func(t *testing.T) {
option := AssertOption{
PollInterval: 5 * time.Second,
Expand All @@ -60,7 +61,7 @@ func (f *Framework) AssertResourceNeverExists(name, namespace string, resource c

return func(t *testing.T) {
//nolint
if err := wait.Poll(option.PollInterval, option.WaitTimeout, func() (bool, error) {
if err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (bool, error) {
key := types.NamespacedName{
Name: name,
Namespace: namespace,
Expand All @@ -76,6 +77,35 @@ func (f *Framework) AssertResourceNeverExists(name, namespace string, resource c
}
}

// AssertResourceAbsent asserts that a resource is not present or, if present, is deleted
// within the timeout
func (f *Framework) AssertResourceAbsent(name, namespace string, resource client.Object, fns ...OptionFn) func(t *testing.T) {
option := AssertOption{
PollInterval: 5 * time.Second,
WaitTimeout: DefaultTestTimeout,
}
for _, fn := range fns {
fn(&option)
}

return func(t *testing.T) {
//nolint
if err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (bool, error) {
key := types.NamespacedName{
Name: name,
Namespace: namespace,
}
if err := f.K8sClient.Get(context.Background(), key, resource); errors.IsNotFound(err) {
return true, nil
}

return false, fmt.Errorf("resource %s/%s should not be present", namespace, name)
}); wait.Interrupted(err) {
t.Fatal(err)
}
}
}

// AssertResourceEventuallyExists asserts that a resource is created duration a time period of customForeverTestTimeout
func (f *Framework) AssertResourceEventuallyExists(name, namespace string, resource client.Object, fns ...OptionFn) func(t *testing.T) {
option := AssertOption{
Expand Down Expand Up @@ -113,8 +143,7 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option
}
return func(t *testing.T) {
key := types.NamespacedName{Name: name, Namespace: namespace}
//nolint
if err := wait.Poll(5*time.Second, option.WaitTimeout, func() (bool, error) {
if err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (bool, error) {
pod := &appsv1.StatefulSet{}
err := f.K8sClient.Get(context.Background(), key, pod)
return err == nil && pod.Status.ReadyReplicas == *pod.Spec.Replicas, nil
Expand All @@ -125,8 +154,11 @@ func (f *Framework) AssertStatefulsetReady(name, namespace string, fns ...Option
}

func (f *Framework) GetResourceWithRetry(t *testing.T, name, namespace string, obj client.Object) {
//nolint
err := wait.Poll(5*time.Second, DefaultTestTimeout, func() (bool, error) {
option := AssertOption{
PollInterval: 5 * time.Second,
WaitTimeout: DefaultTestTimeout,
}
err := wait.PollUntilContextTimeout(context.Background(), option.PollInterval, option.WaitTimeout, true, func(ctx context.Context) (bool, error) {
key := types.NamespacedName{Name: name, Namespace: namespace}

if err := f.K8sClient.Get(context.Background(), key, obj); errors.IsNotFound(err) {
Expand Down Expand Up @@ -238,7 +270,7 @@ func (f *Framework) GetStackWhenAvailable(t *testing.T, name, namespace string)
}
var lastErr error

err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, DefaultTestTimeout+10*time.Second, true, func(ctx context.Context) (bool, error) {
err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, DefaultTestTimeout+120*time.Second, true, func(ctx context.Context) (bool, error) {
lastErr = nil
err := f.K8sClient.Get(context.Background(), key, &ms)
if err != nil {
Expand Down
13 changes: 7 additions & 6 deletions test/e2e/framework/framework.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (
"strings"
"testing"

policyv1beta1 "k8s.io/api/policy/v1beta1"
"github.com/pkg/errors"
policyv1 "k8s.io/api/policy/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"

Expand Down Expand Up @@ -37,7 +38,7 @@ type Framework struct {
func (f *Framework) StartPortForward(podName string, ns string, port string, stopChan chan struct{}) error {
roundTripper, upgrader, err := spdy.RoundTripperFor(f.Config)
if err != nil {
return err
return errors.Wrap(err, "error creating RoundTripper")
}

path := fmt.Sprintf("/api/v1/namespaces/%s/pods/%s/portforward", ns, podName)
Expand All @@ -49,7 +50,7 @@ func (f *Framework) StartPortForward(podName string, ns string, port string, sto
out, errOut := new(bytes.Buffer), new(bytes.Buffer)
forwarder, err := portforward.New(dialer, []string{port}, stopChan, readyChan, out, errOut)
if err != nil {
return err
return errors.Wrap(err, "failed to create portforward")
}

go func() {
Expand Down Expand Up @@ -131,9 +132,9 @@ func (f *Framework) Evict(pod *corev1.Pod, gracePeriodSeconds int64) error {
GracePeriodSeconds: &gracePeriodSeconds,
}

eviction := &policyv1beta1.Eviction{
eviction := &policyv1.Eviction{
TypeMeta: metav1.TypeMeta{
APIVersion: policyv1beta1.SchemeGroupVersion.String(),
APIVersion: policyv1.SchemeGroupVersion.String(),
Kind: "Eviction",
},
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -147,7 +148,7 @@ func (f *Framework) Evict(pod *corev1.Pod, gracePeriodSeconds int64) error {
if err != nil {
return err
}
return c.PolicyV1beta1().Evictions(pod.Namespace).Evict(context.Background(), eviction)
return c.PolicyV1().Evictions(pod.Namespace).Evict(context.Background(), eviction)
}

func (f *Framework) CleanUp(t *testing.T, cleanupFunc func()) {
Expand Down
21 changes: 19 additions & 2 deletions test/e2e/monitoring_stack_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ func TestMonitoringStackController(t *testing.T) {
if err != nil {
t.Fatal(err)
}
assertPDBExpectedPodsAreHealthy(t, stackName+"-alertmanager", e2eTestNamespace)
assertAlertmanagersAreOnDifferentNodes(t, pods)
assertAlertmanagersAreResilientToDisruption(t, pods)
},
Expand Down Expand Up @@ -402,7 +403,7 @@ func singlePrometheusReplicaHasNoPDB(t *testing.T) {
assert.NilError(t, err, "failed to update monitoring stack")

// ensure there is no pdb
f.AssertResourceNeverExists(pdbName, ms.Namespace, &pdb)(t)
f.AssertResourceAbsent(pdbName, ms.Namespace, &pdb)(t)
}

func assertPrometheusScrapesItself(t *testing.T) {
Expand Down Expand Up @@ -516,6 +517,21 @@ func assertAlertmanagersAreResilientToDisruption(t *testing.T, pods []corev1.Pod
}
}

func assertPDBExpectedPodsAreHealthy(t *testing.T, name, namespace string) {
if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.DefaultTestTimeout, true, func(ctx context.Context) (bool, error) {
pdb := &policyv1.PodDisruptionBudget{}
key := types.NamespacedName{Name: name, Namespace: namespace}
err := f.K8sClient.Get(context.Background(), key, pdb)
if err != nil {
return false, nil
}
return pdb.Status.CurrentHealthy == pdb.Status.ExpectedPods, nil

}); err != nil {
t.Fatal(err)
}
}

func assertAlertmanagerReceivesAlerts(t *testing.T) {
ms := newMonitoringStack(t, "alerting")
if err := f.K8sClient.Create(context.Background(), ms); err != nil {
Expand Down Expand Up @@ -851,11 +867,12 @@ func namespaceSelectorTest(t *testing.T) {
err := deployDemoApp(t, ns, nsLabels, resourceLabels)
assert.NilError(t, err, "%s: deploying demo app failed", ns)
}
f.AssertStatefulsetReady("prometheus-"+stackName, e2eTestNamespace, framework.WithTimeout(5*time.Minute))(t)

stopChan := make(chan struct{})
defer close(stopChan)
//nolint
if pollErr := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.DefaultTestTimeout, true, func(ctx context.Context) (bool, error) {
if pollErr := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, framework.DefaultTestTimeout*2, true, func(ctx context.Context) (bool, error) {
err := f.StartServicePortForward(ms.Name+"-prometheus", e2eTestNamespace, "9090", stopChan)
return err == nil, nil
}); pollErr != nil {
Expand Down
11 changes: 6 additions & 5 deletions test/e2e/thanos_querier_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,14 @@ func singleStackWithSidecar(t *testing.T) {
err = f.StartServicePortForward(name, e2eTestNamespace, "10902", stopChan)
return err == nil, nil
}); wait.Interrupted(err) {
t.Fatal(err)
t.Fatal("timeout waiting for port-forward")
}

promClient := framework.NewPrometheusClient("http://localhost:9090")
promClient := framework.NewPrometheusClient("http://localhost:10902")
expectedResults := map[string]int{
"prometheus_build_info": 2, // must return from both prometheus pods
}
var lastErr error
if err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) {
correct := 0
for query, value := range expectedResults {
Expand All @@ -99,8 +100,8 @@ func singleStackWithSidecar(t *testing.T) {
}

if len(result.Data.Result) > value {
resultErr := fmt.Errorf("invalid result for query %s, got %d, want %d", query, len(result.Data.Result), value)
return true, resultErr
lastErr = fmt.Errorf("invalid result for query %s, got %d, want %d", query, len(result.Data.Result), value)
return true, lastErr
}

if len(result.Data.Result) != value {
Expand All @@ -112,7 +113,7 @@ func singleStackWithSidecar(t *testing.T) {

return correct == len(expectedResults), nil
}); wait.Interrupted(err) {
t.Fatal(err)
t.Fatal(fmt.Errorf("querying thanos did not yield expected results: %w", lastErr))
}
}

Expand Down

0 comments on commit 2e0cfac

Please sign in to comment.