Skip to content

Commit

Permalink
test/e2e: updates tests for child objects
Browse files Browse the repository at this point in the history
Attemp to make tests less flaky by ignoring concurrent reconcile errors.

 Also, reduce tests concurrency to 1 for now.

Signed-off-by: f41gh7 <[email protected]>
  • Loading branch information
f41gh7 committed Dec 17, 2024
1 parent 17df84a commit e2e8ab4
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 33 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ test: manifests generate fmt vet envtest ## Run tests.
.PHONY: test-e2e # Run the e2e tests against a Kind k8s instance that is spun up.
test-e2e: load-kind ginkgo-install
$(GINKGO_BIN) -procs=$(E2E_TESTS_CONCURRENCY) -timeout=20m ./test/e2e/
$(GINKGO_BIN) -procs=$(E2E_TESTS_CONCURRENCY) -timeout=20m ./test/e2e/childobjects/
$(GINKGO_BIN) -procs=1 -timeout=20m ./test/e2e/childobjects/
$(GINKGO_BIN) -procs=$(E2E_TESTS_CONCURRENCY) -timeout=20m ./test/e2e/watchnamespace/
$(GINKGO_BIN) -procs=$(E2E_TESTS_CONCURRENCY) -timeout=20m ./test/e2e/deploy/

Expand Down
3 changes: 3 additions & 0 deletions internal/controller/operator/factory/finalize/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ func patchReplaceFinalizers(ctx context.Context, rclient client.Client, instance
return fmt.Errorf("cannot marshal finalizers patch for=%q: %w", instance.GetName(), err)
}
if err := rclient.Patch(ctx, instance, client.RawPatch(types.JSONPatchType, patchData)); err != nil {
if errors.IsNotFound(err) {
return nil
}
return fmt.Errorf("cannot patch finalizers for object=%q with name=%q: %w", instance.GetObjectKind().GroupVersionKind(), instance.GetName(), err)
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/operator/factory/reconcile/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func StatusForChildObjects[T any, PT interface {
} else {
currCound.Status = "False"
currCound.Message = st.CurrentSyncError
errors = append(errors, fmt.Sprintf("parent=%s config=namespace/name=%s/%s error text: %s", parentObjectName, childObject.GetNamespace(), childObject.GetName(), st.Reason))
errors = append(errors, fmt.Sprintf("parent=%s config=namespace/name=%s/%s error text: %s", parentObjectName, childObject.GetNamespace(), childObject.GetName(), st.CurrentSyncError))
}
if err := updateChildStatusConditions[T](ctx, rclient, childObject, currCound); err != nil {
return err
Expand Down
3 changes: 3 additions & 0 deletions internal/controller/operator/factory/vmauth/vmusers_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ func createVMUserSecrets(ctx context.Context, rclient client.Client, secrets []*
for i := range secrets {
secret := secrets[i]
if err := rclient.Create(ctx, secret); err != nil {
if errors.IsAlreadyExists(err) {
continue
}
return err
}
}
Expand Down
18 changes: 17 additions & 1 deletion test/e2e/childobjects/suite_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package childobjects

import (
"fmt"
"strings"
"testing"

"github.com/VictoriaMetrics/operator/test/e2e/suite"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"sigs.k8s.io/controller-runtime/pkg/client"

v1beta1vm "github.com/VictoriaMetrics/operator/api/operator/v1beta1"
"github.com/VictoriaMetrics/operator/test/e2e/suite"
)

const eventualDeletionTimeout = 20
Expand Down Expand Up @@ -34,3 +38,15 @@ var _ = SynchronizedAfterSuite(
func() {
suite.ShutdownOperatorProcess()
})

func expectConditionOkFor(conds []v1beta1vm.Condition, typeCondtains string) error {
for _, cond := range conds {
if strings.Contains(cond.Type, typeCondtains) {
if cond.Status == "True" {
return nil
}
return fmt.Errorf("unexpected status=%q for type=%q with message=%q", cond.Status, cond.Type, cond.Message)
}
}
return fmt.Errorf("expected condition not found")
}
29 changes: 11 additions & 18 deletions test/e2e/childobjects/vmalertmanagerconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,29 +314,22 @@ route:
receiver: blackhole
`
Expect(k8sClient.Update(ctx, &toUpdate)).To(Succeed())
Eventually(func() error {
return suite.ExpectObjectStatus(ctx, k8sClient, &v1beta1vm.VMAlertmanager{}, nsn, v1beta1vm.UpdateStatusOperational)
}, eventualReadyTimeout).Should(Succeed())

},
verify: func() {
for _, nsn := range []types.NamespacedName{
{Name: "partiallly-ok", Namespace: namespace},
} {
var amcfg v1beta1vm.VMAlertmanagerConfig
Expect(k8sClient.Get(ctx, nsn, &amcfg)).To(Succeed())
if len(amcfg.Status.Conditions) == 2 {
Expect(amcfg.Status.UpdateStatus).To(Equal(v1beta1vm.UpdateStatusOperational))
Expect(amcfg.Status.Reason).To(BeEmpty())
}
for _, cond := range amcfg.Status.Conditions {
switch {
case strings.HasPrefix(cond.Type, "parsing-test."):
case strings.HasPrefix(cond.Type, "parsing-test-with-global-option."):
default:
continue
}
Expect(cond.Status).To(Equal(metav1.ConditionTrue), "reason=%q,type=%q,rule=%q", cond.Reason, cond.Type, amcfg.Name)
}
Eventually(func() error {
var amcfg v1beta1vm.VMAlertmanagerConfig
Expect(k8sClient.Get(ctx, nsn, &amcfg)).To(Succeed())
return expectConditionOkFor(amcfg.Status.Conditions, "parsing-test.")
}, eventualReadyTimeout).Should(Succeed())
Eventually(func() error {
var amcfg v1beta1vm.VMAlertmanagerConfig
Expect(k8sClient.Get(ctx, nsn, &amcfg)).To(Succeed())
return expectConditionOkFor(amcfg.Status.Conditions, "parsing-test-with-global-option.")
}, eventualReadyTimeout).Should(Succeed())
}

},
Expand Down
18 changes: 7 additions & 11 deletions test/e2e/childobjects/vmuser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ var _ = Describe("test vmuser Controller", func() {
},
Spec: v1beta1vm.VMUserSpec{
UserName: ptr.To("user"),
Password: ptr.To("password"),
Password: ptr.To("password-1"),
TargetRefs: []v1beta1vm.TargetRef{
{
Static: &v1beta1vm.StaticRef{
Expand Down Expand Up @@ -189,7 +189,7 @@ var _ = Describe("test vmuser Controller", func() {
Name: "backend-e2e-access-1",
Namespace: namespace,
},
StringData: map[string]string{"password": "password"},
StringData: map[string]string{"password": "password-2"},
}
Expect(k8sClient.Create(ctx, &passwordSecret)).To(Succeed())
DeferCleanup(func() {
Expand All @@ -202,20 +202,16 @@ var _ = Describe("test vmuser Controller", func() {
Expect(k8sClient.Get(ctx, nsn, &toUpdate)).To(Succeed())
toUpdate.Spec.PasswordRef.Name = "backend-e2e-access-1"
Expect(k8sClient.Update(ctx, &toUpdate)).To(Succeed())
Eventually(func() error {
return suite.ExpectObjectStatus(ctx, k8sClient, &v1beta1vm.VMUser{}, nsn, v1beta1vm.UpdateStatusOperational)
}, eventualReadyTimeout).Should(Succeed())
},
verify: func() {
for _, nsn := range []types.NamespacedName{
{Name: "missing-ref-1", Namespace: namespace},
} {
var vmuser v1beta1vm.VMUser
Expect(k8sClient.Get(ctx, nsn, &vmuser)).To(Succeed())
Expect(vmuser.Status.UpdateStatus).To(Equal(v1beta1vm.UpdateStatusOperational))
for _, cond := range vmuser.Status.Conditions {
Expect(cond.Status).To(Equal(metav1.ConditionTrue), "expect condition to be True, reason=%q,type=%q", cond.Reason, cond.Type)
}
Eventually(func() error {
var toUpdate v1beta1vm.VMUser
Expect(k8sClient.Get(ctx, nsn, &toUpdate)).To(Succeed())
return expectConditionOkFor(toUpdate.Status.Conditions, "multiple-1.")
}, eventualReadyTimeout, 2).Should(Succeed())
}
},
},
Expand Down
7 changes: 6 additions & 1 deletion test/e2e/suite/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"strings"

"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -39,7 +40,11 @@ func ExpectObjectStatus(ctx context.Context,
return fmt.Errorf("expected generation: %d be greater than: %d", obs.Status.ObservedGeneration, object.GetGeneration())
}
if obs.Status.UpdateStatus != status {
return fmt.Errorf("not expected object status=%q", obs.Status.UpdateStatus)
var conds []string
for _, cond := range obs.Status.Conditions {
conds = append(conds, fmt.Sprintf("type=%s,message=%q,generation=%d,status=%q", cond.Type, cond.Message, cond.ObservedGeneration, cond.Status))
}
return fmt.Errorf("not expected object status=%q, reason=%q,conditions=%s", obs.Status.UpdateStatus, obs.Status.Reason, strings.Join(conds, ","))
}

return nil
Expand Down

0 comments on commit e2e8ab4

Please sign in to comment.