From 353deb443278e321c383890e9d607cd5faccddc3 Mon Sep 17 00:00:00 2001 From: Andrii Chubatiuk Date: Thu, 19 Feb 2026 10:41:18 +0200 Subject: [PATCH 1/2] tests: simplify test client --- go.mod | 1 - go.sum | 2 - .../operator/converter/v1alpha1/apis_test.go | 10 +- .../operator/factory/build/container_test.go | 13 +- .../operator/factory/build/service_test.go | 13 +- .../factory/k8stools/selectors_test.go | 29 +- .../operator/factory/k8stools/test_helpers.go | 233 +------- .../factory/reconcile/configmap_test.go | 67 ++- .../operator/factory/reconcile/deploy_test.go | 73 ++- .../operator/factory/reconcile/diff_test.go | 12 +- .../factory/reconcile/service_test.go | 128 ++--- .../reconcile/statefulset_pvc_expand_test.go | 41 +- .../factory/reconcile/statefulset_test.go | 81 ++- .../operator/factory/vlagent/vlagent_test.go | 527 ++++++++---------- .../factory/vlcluster/vlcluster_test.go | 156 ++---- .../factory/vmagent/staticscrape_test.go | 13 +- .../vmagent/vmagent_scrapeconfig_test.go | 35 +- .../operator/factory/vmagent/vmagent_test.go | 328 +++-------- .../operator/factory/vmalert/vmalert_test.go | 110 ++-- .../vmalertmanager/alertmanager_test.go | 140 ++--- .../factory/vmalertmanager/config_test.go | 53 +- .../factory/vmanomaly/config/config_test.go | 11 +- .../factory/vmanomaly/statefulset_test.go | 126 ++--- .../operator/factory/vmauth/vmauth_test.go | 39 +- .../factory/vmcluster/vmcluster_test.go | 154 ++--- .../vmdistributed/vmdistributed_test.go | 318 +++++++---- .../factory/vtcluster/cluster_test.go | 156 ++---- 27 files changed, 1029 insertions(+), 1840 deletions(-) diff --git a/go.mod b/go.mod index 93bd21914..c5ee4f80e 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,6 @@ require ( github.com/cespare/xxhash/v2 v2.3.0 github.com/fsnotify/fsnotify v1.9.0 github.com/go-logr/logr v1.4.3 - github.com/go-test/deep v1.1.1 github.com/google/go-cmp v0.7.0 github.com/google/uuid v1.6.0 github.com/onsi/ginkgo/v2 v2.28.1 diff --git a/go.sum b/go.sum index 7994aca0d..1ed8676aa 100644 --- a/go.sum +++ b/go.sum @@ -106,8 +106,6 @@ github.com/go-openapi/testify/v2 v2.0.2 h1:X999g3jeLcoY8qctY/c/Z8iBHTbwLz7R2WXd6 github.com/go-openapi/testify/v2 v2.0.2/go.mod h1:HCPmvFFnheKK2BuwSA0TbbdxJ3I16pjwMkYkP4Ywn54= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= -github.com/go-test/deep v1.1.1 h1:0r/53hagsehfO4bzD2Pgr/+RgHqhmf+k1Bpse2cTu1U= -github.com/go-test/deep v1.1.1/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/goccy/go-yaml v1.18.0 h1:8W7wMFS12Pcas7KU+VVkaiCng+kG8QiFeFwzFb+rwuw= github.com/goccy/go-yaml v1.18.0/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA= github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q= diff --git a/internal/controller/operator/converter/v1alpha1/apis_test.go b/internal/controller/operator/converter/v1alpha1/apis_test.go index d1d5af99b..8d81ecc01 100644 --- a/internal/controller/operator/converter/v1alpha1/apis_test.go +++ b/internal/controller/operator/converter/v1alpha1/apis_test.go @@ -3,7 +3,6 @@ package v1alpha1 import ( "testing" - "github.com/google/go-cmp/cmp" promv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" promv1alpha1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1alpha1" "github.com/stretchr/testify/assert" @@ -23,9 +22,7 @@ func TestConvertAlertmanagerConfig(t *testing.T) { f := func(o opts) { t.Helper() converted, err := ConvertAlertmanagerConfig(o.promCfg, &config.BaseOperatorConf{}) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } + assert.NoError(t, err) o.validate(converted) } @@ -97,10 +94,7 @@ func TestConvertScrapeConfig(t *testing.T) { f := func(opts opts) { t.Helper() got := ConvertScrapeConfig(opts.scrapeConfig, &config.BaseOperatorConf{EnabledPrometheusConverterOwnerReferences: opts.ownerRef}) - if !cmp.Equal(*got, opts.want) { - diff := cmp.Diff(*got, opts.want) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, *got, opts.want) } // with static config diff --git a/internal/controller/operator/factory/build/container_test.go b/internal/controller/operator/factory/build/container_test.go index bf2fcc9c2..4eddb9f46 100644 --- a/internal/controller/operator/factory/build/container_test.go +++ b/internal/controller/operator/factory/build/container_test.go @@ -48,12 +48,12 @@ func Test_buildProbe(t *testing.T) { type opts struct { container corev1.Container cr testBuildProbeCR - validate func(corev1.Container) error + validate func(corev1.Container) } f := func(o opts) { t.Helper() got := Probe(o.container, o.cr) - assert.NoError(t, o.validate(got)) + o.validate(got) } // build default probe with empty ep @@ -67,11 +67,10 @@ func Test_buildProbe(t *testing.T) { scheme: "HTTP", }, container: corev1.Container{}, - validate: func(container corev1.Container) error { + validate: func(container corev1.Container) { assert.NotNil(t, container.LivenessProbe) assert.NotNil(t, container.ReadinessProbe) assert.Equal(t, corev1.URIScheme("HTTP"), container.ReadinessProbe.HTTPGet.Scheme) - return nil }, }) @@ -86,12 +85,11 @@ func Test_buildProbe(t *testing.T) { scheme: "HTTPS", }, container: corev1.Container{}, - validate: func(container corev1.Container) error { + validate: func(container corev1.Container) { assert.NotNil(t, container.LivenessProbe) assert.Equal(t, corev1.URIScheme("HTTPS"), container.LivenessProbe.HTTPGet.Scheme) assert.NotNil(t, container.ReadinessProbe) assert.Equal(t, corev1.URIScheme("HTTPS"), container.ReadinessProbe.HTTPGet.Scheme) - return nil }, }) @@ -130,7 +128,7 @@ func Test_buildProbe(t *testing.T) { }, }, container: corev1.Container{}, - validate: func(container corev1.Container) error { + validate: func(container corev1.Container) { assert.NotNil(t, container.LivenessProbe) assert.Equal(t, "/live1", container.LivenessProbe.HTTPGet.Path) assert.Equal(t, int32(20), container.LivenessProbe.InitialDelaySeconds) @@ -139,7 +137,6 @@ func Test_buildProbe(t *testing.T) { assert.Len(t, container.ReadinessProbe.Exec.Command, 2) assert.NotNil(t, container.StartupProbe) assert.Equal(t, "some", container.StartupProbe.HTTPGet.Host) - return nil }, }) } diff --git a/internal/controller/operator/factory/build/service_test.go b/internal/controller/operator/factory/build/service_test.go index ed2422f86..6246cb37c 100644 --- a/internal/controller/operator/factory/build/service_test.go +++ b/internal/controller/operator/factory/build/service_test.go @@ -14,13 +14,13 @@ func Test_mergeServiceSpec(t *testing.T) { type opts struct { svc *corev1.Service svcSpec *vmv1beta1.AdditionalServiceSpec - validate func(svc *corev1.Service) error + validate func(svc *corev1.Service) } f := func(o opts) { t.Helper() additionalSvc := AdditionalServiceFromDefault(o.svc, o.svcSpec) - assert.NoError(t, o.validate(additionalSvc)) + o.validate(additionalSvc) } // override ports @@ -42,11 +42,10 @@ func Test_mergeServiceSpec(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { + validate: func(svc *corev1.Service) { assert.Equal(t, "some-name-additional-service", svc.Name) assert.Len(t, svc.Spec.Ports, 1) assert.Equal(t, "metrics", svc.Spec.Ports[0].Name) - return nil }, }) @@ -67,11 +66,10 @@ func Test_mergeServiceSpec(t *testing.T) { Type: corev1.ServiceTypeNodePort, }, }, - validate: func(svc *corev1.Service) error { + validate: func(svc *corev1.Service) { assert.Equal(t, corev1.ServiceTypeNodePort, svc.Spec.Type) assert.Len(t, svc.Spec.Ports, 1) assert.Equal(t, "metrics", svc.Spec.Ports[0].Name) - return nil }, }) @@ -99,11 +97,10 @@ func Test_mergeServiceSpec(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { + validate: func(svc *corev1.Service) { assert.Equal(t, map[string]string{"app-2": "value-3"}, svc.Spec.Selector) assert.Len(t, svc.Spec.Ports, 1) assert.Equal(t, "metrics", svc.Spec.Ports[0].Name) - return nil }, }) } diff --git a/internal/controller/operator/factory/k8stools/selectors_test.go b/internal/controller/operator/factory/k8stools/selectors_test.go index f11c3b1d8..a11dcce1a 100644 --- a/internal/controller/operator/factory/k8stools/selectors_test.go +++ b/internal/controller/operator/factory/k8stools/selectors_test.go @@ -4,8 +4,7 @@ import ( "context" "testing" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -23,12 +22,8 @@ func Test_discoverNamespacesOk(t *testing.T) { t.Helper() fclient := GetTestClientWithObjects(opts.predefinedObjects) got, err := discoverNamespaces(context.TODO(), fclient, &opts.selectorOpts) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - if d := cmp.Diff(got, opts.want, cmp.AllowUnexported(discoverNamespacesResponse{})); len(d) > 0 { - t.Fatalf("unexpected diff: %s", d) - } + assert.NoError(t, err) + assert.Equal(t, got, opts.want) } // match nothing on namespace selector mismatch @@ -101,8 +96,6 @@ func TestVisitSelected(t *testing.T) { }, } } - ignoreDiffOpts := cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion") - f := func(opts opts) { cfg := config.MustGetBaseConfig() if len(opts.watchNamespaces) > 0 { @@ -116,15 +109,13 @@ func TestVisitSelected(t *testing.T) { ctx := context.Background() fclient := GetTestClientWithObjects(opts.predefinedObjects) var gotPods []corev1.Pod - err := VisitSelected(ctx, fclient, opts.so, func(pl *corev1.PodList) { - gotPods = append(gotPods, pl.Items...) - }) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - if d := cmp.Diff(opts.wantPods, gotPods, ignoreDiffOpts); len(d) > 0 { - t.Fatalf("unexpected diff: %s", d) - } + assert.NoError(t, VisitSelected(ctx, fclient, opts.so, func(pl *corev1.PodList) { + for _, p := range pl.Items { + p.ResourceVersion = "" + gotPods = append(gotPods, p) + } + })) + assert.Equal(t, opts.wantPods, gotPods) } // empty objects o := opts{ diff --git a/internal/controller/operator/factory/k8stools/test_helpers.go b/internal/controller/operator/factory/k8stools/test_helpers.go index d9c737536..0f92ec57c 100644 --- a/internal/controller/operator/factory/k8stools/test_helpers.go +++ b/internal/controller/operator/factory/k8stools/test_helpers.go @@ -1,22 +1,18 @@ package k8stools import ( - "context" - "fmt" - "sync" "testing" - "github.com/go-test/deep" + "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" vpav1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" vmv1 "github.com/VictoriaMetrics/operator/api/operator/v1" @@ -94,18 +90,18 @@ func testGetScheme() *runtime.Scheme { return s } +// GetTestClientWithObjectsAndInterceptors returns testing client with optional predefined objects and interceptors +func GetTestClientWithObjectsAndInterceptors(predefinedObjects []runtime.Object, fns interceptor.Funcs) client.Client { + return getTestClient(predefinedObjects, &fns) +} + // GetTestClientWithObjects returns testing client with optional predefined objects -func GetTestClientWithObjects(predefinedObjects []runtime.Object) *TestClientWithStatsTrack { - obj := make([]client.Object, 0, len(predefinedObjects)) - for _, o := range predefinedObjects { - obj = append(obj, o.(client.Object)) - } - return GetTestClientWithClientObjects(obj) +func GetTestClientWithObjects(predefinedObjects []runtime.Object) client.Client { + return getTestClient(predefinedObjects, nil) } -// GetTestClientWithClientObjects returns testing client with optional predefined objects -func GetTestClientWithClientObjects(predefinedObjects []client.Object) *TestClientWithStatsTrack { - fclient := fake.NewClientBuilder(). +func getTestClient(predefinedObjects []runtime.Object, fns *interceptor.Funcs) client.Client { + builder := fake.NewClientBuilder(). WithScheme(testGetScheme()). WithStatusSubresource( &vmv1beta1.VMRule{}, @@ -134,28 +130,19 @@ func GetTestClientWithClientObjects(predefinedObjects []client.Object) *TestClie &gwapiv1.HTTPRoute{}, &vpav1.VerticalPodAutoscaler{}, ). - WithObjects(predefinedObjects...).Build() - withStats := TestClientWithStatsTrack{ - origin: fclient, + WithRuntimeObjects(predefinedObjects...) + if fns != nil { + builder = builder.WithInterceptorFuncs(*fns) } - return &withStats + return builder.Build() } // CompareObjectMeta compares metadata objects func CompareObjectMeta(t *testing.T, got, want metav1.ObjectMeta) { - if diff := deep.Equal(got.Labels, want.Labels); len(diff) > 0 { - t.Fatalf("objects not match, labels diff: %v", diff) - } - if diff := deep.Equal(got.Annotations, want.Annotations); len(diff) > 0 { - t.Fatalf("objects not match, annotations diff: %v", diff) - } - - if diff := deep.Equal(got.Name, want.Name); len(diff) > 0 { - t.Fatalf("objects not match, Name diff: %v", diff) - } - if diff := deep.Equal(got.Namespace, want.Namespace); len(diff) > 0 { - t.Fatalf("objects not match, namespace diff: %v", diff) - } + assert.Equal(t, got.Labels, want.Labels) + assert.Equal(t, got.Annotations, want.Annotations) + assert.Equal(t, got.Name, want.Name) + assert.Equal(t, got.Namespace, want.Namespace) } // NewReadyDeployment returns a new deployment with ready status condition @@ -179,185 +166,3 @@ func NewReadyDeployment(name, namespace string) *appsv1.Deployment { }, } } - -type calls struct { - objects []client.Object - mu sync.Mutex -} - -func getKey(obj client.Object) string { - return fmt.Sprintf("%T/%s/%s", obj, obj.GetNamespace(), obj.GetName()) -} - -func (c *calls) add(obj client.Object) { - c.mu.Lock() - defer c.mu.Unlock() - c.objects = append(c.objects, obj) -} - -// Count returns calls count for given obj -func (c *calls) Count(obj client.Object) int { - c.mu.Lock() - defer c.mu.Unlock() - if obj == nil { - return len(c.objects) - } - var count int - key := getKey(obj) - for _, o := range c.objects { - if getKey(o) == key { - count++ - } - } - return count -} - -// First returns first call, that matches given obj -func (c *calls) First(obj client.Object) client.Object { - c.mu.Lock() - defer c.mu.Unlock() - if obj == nil { - if len(c.objects) > 0 { - return c.objects[0] - } - return nil - } - key := getKey(obj) - for _, o := range c.objects { - if getKey(o) == key { - return o - } - } - return nil -} - -type callType int - -const ( - GetCallType callType = iota - DeleteCallType - CreateCallType - UpdateCallType - PatchCallType -) - -func (d callType) String() string { - return [...]string{"Get", "Delete", "Create", "Update", "Patch"}[d] -} - -type action struct { - obj client.Object - call callType - opts interface{} -} - -func (a action) GetVerb() string { - return a.call.String() -} - -func (a action) GetObject() client.Object { - return a.obj -} - -func (a action) GetOpts() interface{} { - return a.opts -} - -// TestClientWithStatsTrack helps to track actual requests to the api server -type TestClientWithStatsTrack struct { - origin client.Client - GetCalls calls - DeleteCalls calls - CreateCalls calls - UpdateCalls calls - PatchCalls calls - mu sync.Mutex - Actions []action -} - -func (tcs *TestClientWithStatsTrack) TotalCallsCount(obj client.Object) int { - var count int - count += tcs.GetCalls.Count(obj) - count += tcs.DeleteCalls.Count(obj) - count += tcs.CreateCalls.Count(obj) - count += tcs.UpdateCalls.Count(obj) - count += tcs.PatchCalls.Count(obj) - return count -} - -func (tcs *TestClientWithStatsTrack) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { - tcs.GetCalls.add(obj) - tcs.mu.Lock() - tcs.Actions = append(tcs.Actions, action{obj: obj, call: GetCallType, opts: opts}) - tcs.mu.Unlock() - return tcs.origin.Get(ctx, key, obj, opts...) -} - -func (tcs *TestClientWithStatsTrack) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { - tcs.CreateCalls.add(obj) - tcs.mu.Lock() - tcs.Actions = append(tcs.Actions, action{obj: obj, call: CreateCallType, opts: opts}) - tcs.mu.Unlock() - return tcs.origin.Create(ctx, obj, opts...) -} - -func (tcs *TestClientWithStatsTrack) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { - tcs.DeleteCalls.add(obj) - tcs.mu.Lock() - tcs.Actions = append(tcs.Actions, action{obj: obj, call: DeleteCallType, opts: opts}) - tcs.mu.Unlock() - return tcs.origin.Delete(ctx, obj, opts...) -} - -func (tcs *TestClientWithStatsTrack) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - tcs.UpdateCalls.add(obj) - tcs.mu.Lock() - tcs.Actions = append(tcs.Actions, action{obj: obj, call: UpdateCallType, opts: opts}) - tcs.mu.Unlock() - return tcs.origin.Update(ctx, obj, opts...) -} - -func (tcs *TestClientWithStatsTrack) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { - tcs.PatchCalls.add(obj) - tcs.mu.Lock() - tcs.Actions = append(tcs.Actions, action{obj: obj, call: PatchCallType, opts: opts}) - tcs.mu.Unlock() - return tcs.origin.Patch(ctx, obj, patch, opts...) -} - -func (tcs *TestClientWithStatsTrack) Apply(ctx context.Context, obj runtime.ApplyConfiguration, opts ...client.ApplyOption) error { - return tcs.origin.Apply(ctx, obj, opts...) -} - -func (tcs *TestClientWithStatsTrack) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - return tcs.origin.List(ctx, list, opts...) -} - -// DeleteAllOf deletes all objects of the given type matching the given options. -func (tcs *TestClientWithStatsTrack) DeleteAllOf(ctx context.Context, obj client.Object, opts ...client.DeleteAllOfOption) error { - return tcs.origin.DeleteAllOf(ctx, obj, opts...) -} - -func (tcs *TestClientWithStatsTrack) Status() client.SubResourceWriter { - return tcs.origin.Status() -} - -func (tcs *TestClientWithStatsTrack) SubResource(subResource string) client.SubResourceClient { - return tcs.origin.SubResource(subResource) -} - -func (tcs *TestClientWithStatsTrack) Scheme() *runtime.Scheme { - return tcs.origin.Scheme() -} - -func (tcs *TestClientWithStatsTrack) RESTMapper() meta.RESTMapper { - return tcs.origin.RESTMapper() -} - -func (tcs *TestClientWithStatsTrack) GroupVersionKindFor(obj runtime.Object) (schema.GroupVersionKind, error) { - return tcs.origin.GroupVersionKindFor(obj) -} - -func (tcs *TestClientWithStatsTrack) IsObjectNamespaced(obj runtime.Object) (bool, error) { - return tcs.origin.IsObjectNamespaced(obj) -} diff --git a/internal/controller/operator/factory/reconcile/configmap_test.go b/internal/controller/operator/factory/reconcile/configmap_test.go index 704a19dcd..1708740b4 100644 --- a/internal/controller/operator/factory/reconcile/configmap_test.go +++ b/internal/controller/operator/factory/reconcile/configmap_test.go @@ -9,6 +9,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools" @@ -20,22 +22,44 @@ func TestConfigMapReconcile(t *testing.T) { prevMeta *metav1.ObjectMeta owner *metav1.OwnerReference predefinedObjects []runtime.Object - validate func(*k8stools.TestClientWithStatsTrack, *corev1.ConfigMap) + actions []string + validate func(*corev1.ConfigMap) } f := func(o opts) { t.Helper() ctx := context.Background() - rclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) + var actions []string + rclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + if o.new.Name == obj.GetName() && o.new.Namespace == obj.GetNamespace() { + actions = append(actions, "Create") + } + return cl.Create(ctx, obj, opts...) + }, + Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if o.new.Name == key.Name && o.new.Namespace == key.Namespace { + actions = append(actions, "Get") + } + return cl.Get(ctx, key, obj, opts...) + }, + Update: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + if o.new.Name == obj.GetName() && o.new.Namespace == obj.GetNamespace() { + actions = append(actions, "Update") + } + return cl.Update(ctx, obj, opts...) + }, + }) _, err := ConfigMap(ctx, rclient, o.new, o.prevMeta, o.owner) assert.NoError(t, err) - var got corev1.ConfigMap - nsn := types.NamespacedName{ - Name: o.new.Name, - Namespace: o.new.Namespace, - } - assert.NoError(t, rclient.Get(ctx, nsn, &got)) + assert.Equal(t, o.actions, actions) if o.validate != nil { - o.validate(rclient, &got) + var got corev1.ConfigMap + nsn := types.NamespacedName{ + Name: o.new.Name, + Namespace: o.new.Namespace, + } + assert.NoError(t, rclient.Get(ctx, nsn, &got)) + o.validate(&got) } } @@ -50,9 +74,8 @@ func TestConfigMapReconcile(t *testing.T) { "data": "test", }, }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, c *corev1.ConfigMap) { - assert.Equal(t, 1, rclient.CreateCalls.Count(c)) - assert.Equal(t, 0, rclient.UpdateCalls.Count(c)) + actions: []string{"Get", "Create"}, + validate: func(c *corev1.ConfigMap) { assert.Equal(t, "test", c.Data["data"]) }, }) @@ -84,9 +107,8 @@ func TestConfigMapReconcile(t *testing.T) { }, }, }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, c *corev1.ConfigMap) { - assert.Equal(t, 0, rclient.CreateCalls.Count(c)) - assert.Equal(t, 0, rclient.UpdateCalls.Count(c)) + actions: []string{"Get"}, + validate: func(c *corev1.ConfigMap) { assert.Equal(t, "test", c.Data["data"]) }, }) @@ -120,9 +142,8 @@ func TestConfigMapReconcile(t *testing.T) { }, }, }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, c *corev1.ConfigMap) { - assert.Equal(t, 0, rclient.CreateCalls.Count(c)) - assert.Equal(t, 1, rclient.UpdateCalls.Count(c)) + actions: []string{"Get", "Update"}, + validate: func(c *corev1.ConfigMap) { assert.Equal(t, "value", c.Annotations["key"]) }, }) @@ -153,9 +174,8 @@ func TestConfigMapReconcile(t *testing.T) { }, }, }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, c *corev1.ConfigMap) { - assert.Equal(t, 0, rclient.CreateCalls.Count(c)) - assert.Equal(t, 1, rclient.UpdateCalls.Count(c)) + actions: []string{"Get", "Update"}, + validate: func(c *corev1.ConfigMap) { assert.Equal(t, "after", c.Data["data"]) }, }) @@ -197,9 +217,8 @@ func TestConfigMapReconcile(t *testing.T) { }, }, }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, c *corev1.ConfigMap) { - assert.Equal(t, 0, rclient.CreateCalls.Count(c)) - assert.Equal(t, 0, rclient.UpdateCalls.Count(c)) + actions: []string{"Get"}, + validate: func(c *corev1.ConfigMap) { assert.Equal(t, "test", c.Data["data"]) }, }) diff --git a/internal/controller/operator/factory/reconcile/deploy_test.go b/internal/controller/operator/factory/reconcile/deploy_test.go index 91fb6f463..3e51ee903 100644 --- a/internal/controller/operator/factory/reconcile/deploy_test.go +++ b/internal/controller/operator/factory/reconcile/deploy_test.go @@ -9,7 +9,8 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools" @@ -19,8 +20,7 @@ func TestDeployReconcile(t *testing.T) { type opts struct { new, prev *appsv1.Deployment predefinedObjects []runtime.Object - validate func(*k8stools.TestClientWithStatsTrack, *appsv1.Deployment) - wantErr bool + actions []string } getDeploy := func(fns ...func(d *appsv1.Deployment)) *appsv1.Deployment { d := &appsv1.Deployment{ @@ -70,37 +70,36 @@ func TestDeployReconcile(t *testing.T) { } f := func(o opts) { t.Helper() + var actions []string ctx := context.Background() - rclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) - err := Deployment(ctx, rclient, o.new, o.prev, false, nil) - if err != nil { - t.Fatalf("failed to wait deployment created: %s", err) - } - if o.wantErr { - assert.Error(t, err) - return - } else { - assert.NoError(t, err) - } - nsn := types.NamespacedName{ - Name: o.new.Name, - Namespace: o.new.Namespace, - } - var got appsv1.Deployment - assert.NoError(t, rclient.Get(ctx, nsn, &got)) - if o.validate != nil { - o.validate(rclient, &got) - } + rclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + if o.new.Name == obj.GetName() && o.new.Namespace == obj.GetNamespace() { + actions = append(actions, "Create") + } + return cl.Create(ctx, obj, opts...) + }, + Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if o.new.Name == key.Name && o.new.Namespace == key.Namespace { + actions = append(actions, "Get") + } + return cl.Get(ctx, key, obj, opts...) + }, + Update: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + if o.new.Name == obj.GetName() && o.new.Namespace == obj.GetNamespace() { + actions = append(actions, "Update") + } + return cl.Update(ctx, obj, opts...) + }, + }) + assert.NoError(t, Deployment(ctx, rclient, o.new, o.prev, false, nil)) + assert.Equal(t, o.actions, actions) } // create deployment f(opts{ - new: getDeploy(), - validate: func(rclient *k8stools.TestClientWithStatsTrack, d *appsv1.Deployment) { - assert.Equal(t, 2, rclient.GetCalls.Count(d)) - assert.Equal(t, 1, rclient.CreateCalls.Count(d)) - assert.Equal(t, 0, rclient.UpdateCalls.Count(d)) - }, + new: getDeploy(), + actions: []string{"Get", "Create", "Get"}, }) // no updates @@ -112,11 +111,7 @@ func TestDeployReconcile(t *testing.T) { d.Finalizers = []string{vmv1beta1.FinalizerName} }), }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, d *appsv1.Deployment) { - assert.Equal(t, 3, rclient.GetCalls.Count(d)) - assert.Equal(t, 0, rclient.CreateCalls.Count(d)) - assert.Equal(t, 0, rclient.UpdateCalls.Count(d)) - }, + actions: []string{"Get", "Get"}, }) // update spec @@ -128,10 +123,7 @@ func TestDeployReconcile(t *testing.T) { predefinedObjects: []runtime.Object{ getDeploy(), }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, d *appsv1.Deployment) { - assert.Equal(t, 0, rclient.CreateCalls.Count(d)) - assert.Equal(t, 1, rclient.UpdateCalls.Count(d)) - }, + actions: []string{"Get", "Update", "Get"}, }) // remove template annotations @@ -143,9 +135,6 @@ func TestDeployReconcile(t *testing.T) { d.Spec.Template.Annotations = map[string]string{"new-annotation": "value"} }), }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, d *appsv1.Deployment) { - assert.Equal(t, 0, rclient.CreateCalls.Count(d)) - assert.Equal(t, 1, rclient.UpdateCalls.Count(d)) - }, + actions: []string{"Get", "Update", "Get"}, }) } diff --git a/internal/controller/operator/factory/reconcile/diff_test.go b/internal/controller/operator/factory/reconcile/diff_test.go index 3cadc60a8..735d9e38a 100644 --- a/internal/controller/operator/factory/reconcile/diff_test.go +++ b/internal/controller/operator/factory/reconcile/diff_test.go @@ -3,16 +3,14 @@ package reconcile import ( "testing" - "k8s.io/apimachinery/pkg/api/equality" + "github.com/stretchr/testify/assert" ) func TestDiffDeepOk(t *testing.T) { f := func(oldObj, newObj any, expected string) { t.Helper() got := diffDeep(oldObj, newObj) - if got != expected { - t.Fatalf("unexpected diff, \ngot:\n%s\nwant:\n%s\n", got, expected) - } + assert.Equal(t, got, expected) } var a1Slice []string a2SliceNonNil := make([]string, 0) @@ -47,12 +45,8 @@ func TestDiffDeepOk(t *testing.T) { func TestDiffDeepDerivativeOk(t *testing.T) { f := func(oldObj, newObj any, expected string) { t.Helper() - sym := equality.Semantic.DeepDerivative(oldObj, newObj) - got := diffDeepDerivative(oldObj, newObj) - if got != expected { - t.Fatalf("unexpected diff, \ngot:\n%s\nwant:\n%s,\nsym=%v", got, expected, sym) - } + assert.Equal(t, got, expected) } var oldS []string newS := make([]string, 0) diff --git a/internal/controller/operator/factory/reconcile/service_test.go b/internal/controller/operator/factory/reconcile/service_test.go index 363a78ce3..2cba20692 100644 --- a/internal/controller/operator/factory/reconcile/service_test.go +++ b/internal/controller/operator/factory/reconcile/service_test.go @@ -2,9 +2,9 @@ package reconcile import ( "context" - "fmt" "testing" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -20,24 +20,17 @@ func Test_reconcileServiceForCRD(t *testing.T) { newService *corev1.Service prevService *corev1.Service predefinedObjects []runtime.Object - validate func(svc *corev1.Service) error + validate func(svc *corev1.Service) } f := func(opts opts) { t.Helper() cl := k8stools.GetTestClientWithObjects(opts.predefinedObjects) ctx := context.Background() - err := Service(ctx, cl, opts.newService, opts.prevService, nil) - if err != nil { - t.Fatalf("unexpected reconcileServiceForCRD() error = %s", err) - } + assert.NoError(t, Service(ctx, cl, opts.newService, opts.prevService, nil)) var gotSvc corev1.Service - if err := cl.Get(ctx, types.NamespacedName{Namespace: opts.newService.Namespace, Name: opts.newService.Name}, &gotSvc); err != nil { - t.Fatalf("unexpected error: %v", err) - } - if err := opts.validate(&gotSvc); err != nil { - t.Errorf("unexpected result service error: %s", err) - } + assert.NoError(t, cl.Get(ctx, types.NamespacedName{Namespace: opts.newService.Namespace, Name: opts.newService.Name}, &gotSvc)) + opts.validate(&gotSvc) } f(opts{ @@ -50,11 +43,8 @@ func Test_reconcileServiceForCRD(t *testing.T) { Type: corev1.ServiceTypeNodePort, }, }, - validate: func(svc *corev1.Service) error { - if svc.Name != "prefixed-1" { - return fmt.Errorf("unexpected name, got: %v, want: prefixed-1", svc.Name) - } - return nil + validate: func(svc *corev1.Service) { + assert.Equal(t, svc.Name, "prefixed-1") }, }) @@ -81,11 +71,8 @@ func Test_reconcileServiceForCRD(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { - if *svc.Spec.LoadBalancerClass != "some-class" { - return fmt.Errorf("unexpected LoadBalancerClass: %s, want: some-classs", *svc.Spec.LoadBalancerClass) - } - return nil + validate: func(svc *corev1.Service) { + assert.Equal(t, *svc.Spec.LoadBalancerClass, "some-class") }, }) @@ -123,18 +110,11 @@ func Test_reconcileServiceForCRD(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { - if *svc.Spec.LoadBalancerClass != "some-class" { - return fmt.Errorf("unexpected LoadBalancerClass: %s, want: some-classs", *svc.Spec.LoadBalancerClass) - } + validate: func(svc *corev1.Service) { + assert.Equal(t, *svc.Spec.LoadBalancerClass, "some-class") l, ok := svc.Labels["custom"] - if !ok { - return fmt.Errorf("missing 'custom' label on svc") - } - if l != "label" { - return fmt.Errorf("unexpected value of 'custom' label on svc, got: %v, want: 'value'", l) - } - return nil + assert.True(t, ok) + assert.Equal(t, l, "label") }, }) @@ -161,14 +141,9 @@ func Test_reconcileServiceForCRD(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { - if svc.Name != "prefixed-1" { - return fmt.Errorf("unexpected name, got: %v, want: prefixed-1", svc.Name) - } - if svc.Spec.ClusterIP == "None" { - return fmt.Errorf("unexpected value for clusterIP, want ip, got: %v", svc.Spec.ClusterIP) - } - return nil + validate: func(svc *corev1.Service) { + assert.Equal(t, svc.Name, "prefixed-1") + assert.Empty(t, svc.Spec.ClusterIP) }, }) @@ -196,14 +171,9 @@ func Test_reconcileServiceForCRD(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { - if svc.Name != "prefixed-1" { - return fmt.Errorf("unexpected name, got: %v, want: prefixed-1", svc.Name) - } - if svc.Spec.ClusterIP != "None" { - return fmt.Errorf("unexpected value for clusterIP, want ip, got: %v", svc.Spec.ClusterIP) - } - return nil + validate: func(svc *corev1.Service) { + assert.Equal(t, svc.Name, "prefixed-1") + assert.Equal(t, svc.Spec.ClusterIP, "None") }, }) @@ -231,14 +201,9 @@ func Test_reconcileServiceForCRD(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { - if svc.Name != "prefixed-1" { - return fmt.Errorf("unexpected name, got: %v, want: prefixed-1", svc.Name) - } - if svc.Spec.ClusterIP != "192.168.1.5" { - return fmt.Errorf("unexpected value for clusterIP, want ip, got: %v", svc.Spec.ClusterIP) - } - return nil + validate: func(svc *corev1.Service) { + assert.Equal(t, svc.Name, "prefixed-1") + assert.Equal(t, svc.Spec.ClusterIP, "192.168.1.5") }, }) @@ -266,17 +231,10 @@ func Test_reconcileServiceForCRD(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { - if svc.Name != "prefixed-1" { - return fmt.Errorf("unexpected name, got: %v, want: prefixed-1", svc.Name) - } - if svc.Spec.Type != corev1.ServiceTypeClusterIP { - return fmt.Errorf("unexpected type: %v", svc.Spec.Type) - } - if svc.Spec.ClusterIP != "192.168.1.5" { - return fmt.Errorf("unexpected value for clusterIP, want ip, got: %v", svc.Spec.ClusterIP) - } - return nil + validate: func(svc *corev1.Service) { + assert.Equal(t, svc.Name, "prefixed-1") + assert.Equal(t, svc.Spec.Type, corev1.ServiceTypeClusterIP) + assert.Equal(t, svc.Spec.ClusterIP, "192.168.1.5") }, }) @@ -317,20 +275,11 @@ func Test_reconcileServiceForCRD(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { - if svc.Name != "prefixed-1" { - return fmt.Errorf("unexpected name, got: %v, want: prefixed-1", svc.Name) - } - if svc.Spec.Type != corev1.ServiceTypeNodePort { - return fmt.Errorf("unexpected type: %v", svc.Spec.Type) - } - if svc.Spec.ClusterIP != "192.168.1.5" { - return fmt.Errorf("unexpected value for clusterIP, want ip, got: %v", svc.Spec.ClusterIP) - } - if svc.Spec.Ports[0].NodePort != 331 { - return fmt.Errorf("unexpected value for node port: %v", svc.Spec.Ports[0]) - } - return nil + validate: func(svc *corev1.Service) { + assert.Equal(t, svc.Name, "prefixed-1") + assert.Equal(t, svc.Spec.Type, corev1.ServiceTypeNodePort) + assert.Equal(t, svc.Spec.ClusterIP, "192.168.1.5") + assert.Equal(t, svc.Spec.Ports[0].NodePort, int32(331)) }, }) @@ -358,18 +307,11 @@ func Test_reconcileServiceForCRD(t *testing.T) { }, }, }, - validate: func(svc *corev1.Service) error { - if svc.Name != "prefixed-1" { - return fmt.Errorf("unexpected name, got: %v, want: prefixed-1", svc.Name) - } + validate: func(svc *corev1.Service) { + assert.Equal(t, svc.Name, "prefixed-1") l, ok := svc.Labels["custom"] - if !ok { - return fmt.Errorf("missing 'custom' label on svc") - } - if l != "label" { - return fmt.Errorf("unexpected value of 'custom' label on svc, got: %v, want: 'value'", l) - } - return nil + assert.True(t, ok) + assert.Equal(t, l, "label") }, }) } diff --git a/internal/controller/operator/factory/reconcile/statefulset_pvc_expand_test.go b/internal/controller/operator/factory/reconcile/statefulset_pvc_expand_test.go index fe3bd00a7..da4cc9547 100644 --- a/internal/controller/operator/factory/reconcile/statefulset_pvc_expand_test.go +++ b/internal/controller/operator/factory/reconcile/statefulset_pvc_expand_test.go @@ -2,7 +2,6 @@ package reconcile import ( "context" - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -25,7 +24,7 @@ func Test_reCreateSTS(t *testing.T) { type opts struct { newSTS *appsv1.StatefulSet oldSTS *appsv1.StatefulSet - validate func(sts *appsv1.StatefulSet) error + validate func(sts *appsv1.StatefulSet) mustRecreateSTS bool mustRecreatePod bool } @@ -41,7 +40,7 @@ func Test_reCreateSTS(t *testing.T) { var updatedSTS appsv1.StatefulSet nsn := types.NamespacedName{Namespace: o.newSTS.Namespace, Name: o.newSTS.Name} assert.NoError(t, cl.Get(ctx, nsn, &updatedSTS)) - assert.NoError(t, o.validate(&updatedSTS)) + o.validate(&updatedSTS) assert.Equal(t, mustRecreateSTS, o.mustRecreateSTS) assert.Equal(t, mustRecreatePod, o.mustRecreatePod) } @@ -68,11 +67,8 @@ func Test_reCreateSTS(t *testing.T) { }, }}, }, - validate: func(sts *appsv1.StatefulSet) error { - if len(sts.Spec.VolumeClaimTemplates) != 1 { - return fmt.Errorf("unexpected configuration for volumeclaim at sts: %v, want at least one, got: %v", sts.Name, sts.Spec.VolumeClaimTemplates) - } - return nil + validate: func(sts *appsv1.StatefulSet) { + assert.Len(t, sts.Spec.VolumeClaimTemplates, 1) }, mustRecreateSTS: true, mustRecreatePod: true, @@ -116,15 +112,10 @@ func Test_reCreateSTS(t *testing.T) { }, }}, }, - validate: func(sts *appsv1.StatefulSet) error { - if len(sts.Spec.VolumeClaimTemplates) != 1 { - return fmt.Errorf("unexpected configuration for volumeclaim at sts: %v, want at least one, got: %v", sts.Name, sts.Spec.VolumeClaimTemplates) - } + validate: func(sts *appsv1.StatefulSet) { + assert.Len(t, sts.Spec.VolumeClaimTemplates, 1) sz := sts.Spec.VolumeClaimTemplates[0].Spec.Resources.Requests.Storage().String() - if sz != "15Gi" { - return fmt.Errorf("unexpected sts size, got: %v, want: %v", sz, "15Gi") - } - return nil + assert.Equal(t, sz, "15Gi") }, mustRecreateSTS: true, mustRecreatePod: false, @@ -170,15 +161,10 @@ func Test_reCreateSTS(t *testing.T) { }, }}, }, - validate: func(sts *appsv1.StatefulSet) error { - if len(sts.Spec.VolumeClaimTemplates) != 1 { - return fmt.Errorf("unexpected configuration for volumeclaim at sts: %v, want at least one, got: %v", sts.Name, sts.Spec.VolumeClaimTemplates) - } + validate: func(sts *appsv1.StatefulSet) { + assert.Len(t, sts.Spec.VolumeClaimTemplates, 1) name := *sts.Spec.VolumeClaimTemplates[0].Spec.StorageClassName - if name != "new-sc" { - return fmt.Errorf("unexpected sts storageClass name, got: %v, want: %v", name, "new-sc") - } - return nil + assert.Equal(t, name, "new-sc") }, mustRecreateSTS: true, mustRecreatePod: false, @@ -204,11 +190,8 @@ func Test_reCreateSTS(t *testing.T) { ServiceName: "new-service", }, }, - validate: func(sts *appsv1.StatefulSet) error { - if sts.Spec.ServiceName != "new-service" { - return fmt.Errorf("unexpected serviceName at sts: %s, want: %s", sts.Spec.ServiceName, "new-service") - } - return nil + validate: func(sts *appsv1.StatefulSet) { + assert.Equal(t, sts.Spec.ServiceName, "new-service") }, mustRecreateSTS: true, mustRecreatePod: true, diff --git a/internal/controller/operator/factory/reconcile/statefulset_test.go b/internal/controller/operator/factory/reconcile/statefulset_test.go index 5112287d4..8c69ba357 100644 --- a/internal/controller/operator/factory/reconcile/statefulset_test.go +++ b/internal/controller/operator/factory/reconcile/statefulset_test.go @@ -12,6 +12,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/k8stools" @@ -357,13 +359,9 @@ func Test_performRollingUpdateOnSts(t *testing.T) { func TestSortPodsByID(t *testing.T) { f := func(unorderedPods []corev1.Pod, expectedOrder []corev1.Pod) { t.Helper() - if err := sortStsPodsByID(unorderedPods); err != nil { - t.Fatalf("unexpected error during pod sorting: %s", err) - } + assert.NoError(t, sortStsPodsByID(unorderedPods)) for idx, pod := range expectedOrder { - if pod.Name != unorderedPods[idx].Name { - t.Fatalf("order mismatch want pod: %s at idx: %d, got: %s", pod.Name, idx, unorderedPods[idx].Name) - } + assert.Equal(t, pod.Name, unorderedPods[idx].Name) } } podsFromNames := func(podNames []string) []corev1.Pod { @@ -387,7 +385,8 @@ func TestStatefulsetReconcile(t *testing.T) { type opts struct { new, prev *appsv1.StatefulSet predefinedObjects []runtime.Object - validate func(*k8stools.TestClientWithStatsTrack, *appsv1.StatefulSet) + validate func(*appsv1.StatefulSet) + actions []string wantErr bool } getSts := func(fns ...func(s *appsv1.StatefulSet)) *appsv1.StatefulSet { @@ -435,7 +434,27 @@ func TestStatefulsetReconcile(t *testing.T) { f := func(o opts) { t.Helper() ctx := context.Background() - rclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) + var actions []string + rclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + if o.new.Name == obj.GetName() && o.new.Namespace == obj.GetNamespace() { + actions = append(actions, "Create") + } + return cl.Create(ctx, obj, opts...) + }, + Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if o.new.Name == key.Name && o.new.Namespace == key.Namespace { + actions = append(actions, "Get") + } + return cl.Get(ctx, key, obj, opts...) + }, + Update: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + if o.new.Name == obj.GetName() && o.new.Namespace == obj.GetNamespace() { + actions = append(actions, "Update") + } + return cl.Update(ctx, obj, opts...) + }, + }) var emptyOpts STSOptions err := StatefulSet(ctx, rclient, emptyOpts, o.new, o.prev, nil) if o.wantErr { @@ -448,29 +467,18 @@ func TestStatefulsetReconcile(t *testing.T) { Name: o.new.Name, Namespace: o.new.Namespace, } - var got appsv1.StatefulSet - assert.NoError(t, rclient.Get(ctx, nsn, &got)) + assert.Equal(t, o.actions, actions) if o.validate != nil { - o.validate(rclient, &got) + var got appsv1.StatefulSet + assert.NoError(t, rclient.Get(ctx, nsn, &got)) + o.validate(&got) } } // create statefulset f(opts{ - new: getSts(), - validate: func(rclient *k8stools.TestClientWithStatsTrack, s *appsv1.StatefulSet) { - assert.Equal(t, 2, rclient.GetCalls.Count(s)) - assert.Equal(t, 1, rclient.CreateCalls.Count(s)) - assert.Equal(t, 0, rclient.UpdateCalls.Count(s)) - - assert.Len(t, rclient.Actions, 4) - action := rclient.Actions[0] - assert.Equal(t, action.GetVerb(), "Get") - action = rclient.Actions[1] - assert.Equal(t, action.GetVerb(), "Create") - action = rclient.Actions[2] - assert.Equal(t, action.GetVerb(), "Get") - }, + new: getSts(), + actions: []string{"Get", "Create", "Get"}, }) // no updates @@ -482,11 +490,7 @@ func TestStatefulsetReconcile(t *testing.T) { s.Finalizers = []string{vmv1beta1.FinalizerName} }), }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, s *appsv1.StatefulSet) { - assert.Equal(t, 3, rclient.GetCalls.Count(s)) - assert.Equal(t, 0, rclient.CreateCalls.Count(s)) - assert.Equal(t, 0, rclient.UpdateCalls.Count(s)) - }, + actions: []string{"Get", "Get"}, }) // add annotations @@ -498,9 +502,8 @@ func TestStatefulsetReconcile(t *testing.T) { predefinedObjects: []runtime.Object{ getSts(), }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, s *appsv1.StatefulSet) { - assert.Equal(t, 0, rclient.CreateCalls.Count(s)) - assert.Equal(t, 1, rclient.UpdateCalls.Count(s)) + actions: []string{"Get", "Update", "Get"}, + validate: func(s *appsv1.StatefulSet) { assert.Equal(t, "value", s.Spec.Template.Annotations["new-annotation"]) }, }) @@ -516,8 +519,8 @@ func TestStatefulsetReconcile(t *testing.T) { s.Spec.Template.Annotations = map[string]string{"new-annotation": "value"} }), }, - validate: func(rclient *k8stools.TestClientWithStatsTrack, s *appsv1.StatefulSet) { - assert.Equal(t, 1, rclient.UpdateCalls.Count(s)) + actions: []string{"Get", "Update", "Get"}, + validate: func(s *appsv1.StatefulSet) { assert.Empty(t, s.Spec.Template.Annotations["new-annotation"]) }, }) @@ -540,9 +543,7 @@ func TestStatefulsetReconcile(t *testing.T) { func TestValidateStatefulSetFail(t *testing.T) { f := func(sts appsv1.StatefulSet) { t.Helper() - if err := validateStatefulSet(&sts); err == nil { - t.Fatalf("expected non-empty error") - } + assert.Error(t, validateStatefulSet(&sts)) } // missing volume name f(appsv1.StatefulSet{ @@ -636,9 +637,7 @@ func TestValidateStatefulSetFail(t *testing.T) { func TestValidateStatefulSetOk(t *testing.T) { f := func(sts appsv1.StatefulSet) { t.Helper() - if err := validateStatefulSet(&sts); err != nil { - t.Fatalf("unexpected error: %s", err) - } + assert.NoError(t, validateStatefulSet(&sts)) } // empty case f(appsv1.StatefulSet{ diff --git a/internal/controller/operator/factory/vlagent/vlagent_test.go b/internal/controller/operator/factory/vlagent/vlagent_test.go index 2ea1bf8c9..65c2ffad5 100644 --- a/internal/controller/operator/factory/vlagent/vlagent_test.go +++ b/internal/controller/operator/factory/vlagent/vlagent_test.go @@ -2,14 +2,10 @@ package vlagent import ( "context" - "encoding/json" - "fmt" "sort" "strings" "testing" - "time" - "github.com/go-test/deep" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" appsv1 "k8s.io/api/apps/v1" @@ -18,8 +14,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1 "github.com/VictoriaMetrics/operator/api/operator/v1" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" @@ -28,326 +25,305 @@ import ( ) func TestCreateOrUpdate(t *testing.T) { - f := func(cr *vmv1.VLAgent, mustAddPrevSpec, wantErr bool, validate func(set *appsv1.StatefulSet) error, predefinedObjects ...runtime.Object) { + type opts struct { + cr *vmv1.VLAgent + validate func(set *appsv1.StatefulSet) + predefinedObjects []runtime.Object + } + f := func(o opts) { t.Helper() - fclient := k8stools.GetTestClientWithObjects(predefinedObjects) + fclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if obj.GetNamespace() != o.cr.Namespace { + return nil + } + switch v := obj.(type) { + case *appsv1.StatefulSet: + v.Status.ObservedGeneration = v.Generation + v.Status.ReadyReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) + v.Status.UpdatedReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) + v.Status.CurrentReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) + case *appsv1.Deployment: + v.Status.Conditions = append(v.Status.Conditions, appsv1.DeploymentCondition{ + Type: appsv1.DeploymentProgressing, + Reason: "NewReplicaSetAvailable", + Status: "True", + }) + v.Status.UpdatedReplicas = *v.Spec.Replicas + v.Status.AvailableReplicas = v.Status.UpdatedReplicas + } + assert.NoError(t, cl.Create(ctx, obj)) + return nil + }, + }) ctx := context.TODO() - if mustAddPrevSpec { - jsonSpec, err := json.Marshal(cr.Spec) - if err != nil { - t.Fatalf("cannot set last applied spec: %s", err) - } - if cr.Annotations == nil { - cr.Annotations = make(map[string]string) - } - cr.Annotations["operator.victoriametrics/last-applied-spec"] = string(jsonSpec) - } - errC := make(chan error, 1) build.AddDefaults(fclient.Scheme()) - fclient.Scheme().Default(cr) - go func() { - err := CreateOrUpdate(ctx, cr, fclient) - errC <- err - }() - err := wait.PollUntilContextTimeout(context.Background(), 20*time.Millisecond, time.Second, true, func(ctx context.Context) (done bool, err error) { - var sts appsv1.StatefulSet - if err := fclient.Get(ctx, types.NamespacedName{Namespace: "default", Name: fmt.Sprintf("vlagent-%s", cr.Name)}, &sts); err != nil { - return false, nil - } - sts.Status.ReadyReplicas = ptr.Deref(cr.Spec.ReplicaCount, 0) - sts.Status.UpdatedReplicas = ptr.Deref(cr.Spec.ReplicaCount, 0) - sts.Status.CurrentReplicas = ptr.Deref(cr.Spec.ReplicaCount, 0) - sts.Status.ObservedGeneration = sts.GetGeneration() - err = fclient.Status().Update(ctx, &sts) - if err != nil { - return false, err - } - return true, nil - }) - if err != nil { - t.Errorf("cannot wait sts ready: %s", err) - } - err = <-errC - if (err != nil) != wantErr { - t.Errorf("CreateOrUpdate() error = %v, wantErr %v", err, wantErr) - return - } - var got appsv1.StatefulSet - if err := fclient.Get(context.Background(), types.NamespacedName{Namespace: cr.Namespace, Name: cr.PrefixedName()}, &got); (err != nil) != wantErr { - t.Fatalf("CreateOrUpdate() error = %v, wantErr %v", err, wantErr) - } - if err := validate(&got); err != nil { - t.Fatalf("unexpected error: %v", err) + fclient.Scheme().Default(o.cr) + assert.NoError(t, CreateOrUpdate(ctx, o.cr, fclient)) + if o.validate != nil { + var got appsv1.StatefulSet + assert.NoError(t, fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got)) + o.validate(&got) } } // generate vlagent statefulset with storage - f(&vmv1.VLAgent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "example-agent", - Namespace: "default", - }, - Spec: vmv1.VLAgentSpec{ - RemoteWrite: []vmv1.VLAgentRemoteWriteSpec{ - {URL: "http://remote-write"}, + f(opts{ + cr: &vmv1.VLAgent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-agent", + Namespace: "default", }, - CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ - ReplicaCount: ptr.To(int32(0)), - }, - CommonDefaultableParams: vmv1beta1.CommonDefaultableParams{}, - Storage: &vmv1beta1.StorageSpec{ - VolumeClaimTemplate: vmv1beta1.EmbeddedPersistentVolumeClaim{ - Spec: corev1.PersistentVolumeClaimSpec{ - StorageClassName: ptr.To("embed-sc"), - Resources: corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("10Gi"), + Spec: vmv1.VLAgentSpec{ + RemoteWrite: []vmv1.VLAgentRemoteWriteSpec{ + {URL: "http://remote-write"}, + }, + CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ + ReplicaCount: ptr.To(int32(0)), + }, + CommonDefaultableParams: vmv1beta1.CommonDefaultableParams{}, + Storage: &vmv1beta1.StorageSpec{ + VolumeClaimTemplate: vmv1beta1.EmbeddedPersistentVolumeClaim{ + Spec: corev1.PersistentVolumeClaimSpec{ + StorageClassName: ptr.To("embed-sc"), + Resources: corev1.VolumeResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, }, }, }, }, - }, - ClaimTemplates: []corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "extraTemplate", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - StorageClassName: ptr.To("default"), - Resources: corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("2Gi"), + ClaimTemplates: []corev1.PersistentVolumeClaim{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "extraTemplate", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + StorageClassName: ptr.To("default"), + Resources: corev1.VolumeResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("2Gi"), + }, }, }, }, }, }, }, - }, false, false, func(got *appsv1.StatefulSet) error { - if len(got.Spec.Template.Spec.Containers) != 1 { - return fmt.Errorf("unexpected count of container, got: %d, want: %d", len(got.Spec.Template.Spec.Containers), 1) - } - if len(got.Spec.VolumeClaimTemplates) != 2 { - return fmt.Errorf("unexpected count of VolumeClaimTemplates, got: %d, want: %d", len(got.Spec.VolumeClaimTemplates), 2) - } - if *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName != "embed-sc" { - return fmt.Errorf("unexpected embed VolumeClaimTemplates name, got: %s, want: %s", *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName, "embed-sc") - } - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("10Gi"), - }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected embed VolumeClaimTemplates resources, diff: %v", diff) - } - if *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName != "default" { - return fmt.Errorf("unexpected extra VolumeClaimTemplates, got: %s, want: %s", *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName, "default") - } - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("2Gi"), - }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected extra VolumeClaimTemplates resources, diff: %v", diff) - } - return nil + validate: func(got *appsv1.StatefulSet) { + assert.Len(t, got.Spec.Template.Spec.Containers, 1) + assert.Len(t, got.Spec.VolumeClaimTemplates, 2) + assert.Equal(t, *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName, "embed-sc") + assert.Equal(t, got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + }) + assert.Equal(t, *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName, "default") + assert.Equal(t, got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("2Gi"), + }, + }) + }, }) // generate vlagent with tls-secret - f(&vmv1.VLAgent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "example-agent-tls", - Namespace: "default", - }, - Spec: vmv1.VLAgentSpec{ - CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ - ReplicaCount: ptr.To(int32(0)), + f(opts{ + cr: &vmv1.VLAgent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-agent-tls", + Namespace: "default", }, - RemoteWrite: []vmv1.VLAgentRemoteWriteSpec{ - {URL: "http://remote-write"}, - { - URL: "http://remote-write2", - TLSConfig: &vmv1.TLSConfig{ - CASecret: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "remote2-secret", + Spec: vmv1.VLAgentSpec{ + CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ + ReplicaCount: ptr.To(int32(0)), + }, + RemoteWrite: []vmv1.VLAgentRemoteWriteSpec{ + {URL: "http://remote-write"}, + { + URL: "http://remote-write2", + TLSConfig: &vmv1.TLSConfig{ + CASecret: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "remote2-secret", + }, + Key: "ca", }, - Key: "ca", - }, - CertSecret: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "remote2-secret", + CertSecret: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "remote2-secret", + }, + Key: "ca", }, - Key: "ca", - }, - KeySecret: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "remote2-secret", + KeySecret: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "remote2-secret", + }, + Key: "key", }, - Key: "key", }, }, - }, - { - URL: "http://remote-write3", - TLSConfig: &vmv1.TLSConfig{ - KeySecret: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "remote3-secret", + { + URL: "http://remote-write3", + TLSConfig: &vmv1.TLSConfig{ + KeySecret: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "remote3-secret", + }, + Key: "key", }, - Key: "key", }, }, + { + URL: "http://remote-write4", + TLSConfig: &vmv1.TLSConfig{CertFile: "/tmp/cert1", KeyFile: "/tmp/key1", CAFile: "/tmp/ca"}, + }, }, - { - URL: "http://remote-write4", - TLSConfig: &vmv1.TLSConfig{CertFile: "/tmp/cert1", KeyFile: "/tmp/key1", CAFile: "/tmp/ca"}, - }, }, }, - }, false, false, func(set *appsv1.StatefulSet) error { return nil }, &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}, - }, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: "tls-scrape", Namespace: "default"}, - Data: map[string][]byte{"cert": []byte(`cert-data`), "ca": []byte(`ca-data`), "key": []byte(`key-data`)}, - }, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: "remote2-secret", Namespace: "default"}, - Data: map[string][]byte{"cert": []byte(`cert-data`), "ca": []byte(`ca-data`), "key": []byte(`key-data`)}, - }, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: "remote3-secret", Namespace: "default"}, - Data: map[string][]byte{"key": []byte(`key-data`)}, - }, &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: "remote3-cm", Namespace: "default"}, - Data: map[string]string{"ca": "ca-data", "cert": "cert-data"}, + predefinedObjects: []runtime.Object{ + &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: "default"}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "tls-scrape", Namespace: "default"}, + Data: map[string][]byte{"cert": []byte(`cert-data`), "ca": []byte(`ca-data`), "key": []byte(`key-data`)}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "remote2-secret", Namespace: "default"}, + Data: map[string][]byte{"cert": []byte(`cert-data`), "ca": []byte(`ca-data`), "key": []byte(`key-data`)}, + }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "remote3-secret", Namespace: "default"}, + Data: map[string][]byte{"key": []byte(`key-data`)}, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "remote3-cm", Namespace: "default"}, + Data: map[string]string{"ca": "ca-data", "cert": "cert-data"}, + }, + }, }) // generate vlagent with prevSpec - f(&vmv1.VLAgent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "example-agent", - Namespace: "default", - }, - Spec: vmv1.VLAgentSpec{ - RemoteWrite: []vmv1.VLAgentRemoteWriteSpec{ - {URL: "http://remote-write"}, - }, - CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ - ReplicaCount: ptr.To(int32(1)), + f(opts{ + cr: &vmv1.VLAgent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-agent", + Namespace: "default", }, - Storage: &vmv1beta1.StorageSpec{ - VolumeClaimTemplate: vmv1beta1.EmbeddedPersistentVolumeClaim{ - Spec: corev1.PersistentVolumeClaimSpec{ - StorageClassName: ptr.To("embed-sc"), - Resources: corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("10Gi"), + Spec: vmv1.VLAgentSpec{ + RemoteWrite: []vmv1.VLAgentRemoteWriteSpec{ + {URL: "http://remote-write"}, + }, + CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ + ReplicaCount: ptr.To(int32(1)), + }, + Storage: &vmv1beta1.StorageSpec{ + VolumeClaimTemplate: vmv1beta1.EmbeddedPersistentVolumeClaim{ + Spec: corev1.PersistentVolumeClaimSpec{ + StorageClassName: ptr.To("embed-sc"), + Resources: corev1.VolumeResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, }, }, }, }, - }, - ClaimTemplates: []corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "extraTemplate", - }, - Spec: corev1.PersistentVolumeClaimSpec{ - StorageClassName: ptr.To("default"), - Resources: corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("2Gi"), + ClaimTemplates: []corev1.PersistentVolumeClaim{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "extraTemplate", + }, + Spec: corev1.PersistentVolumeClaimSpec{ + StorageClassName: ptr.To("default"), + Resources: corev1.VolumeResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("2Gi"), + }, }, }, }, }, }, }, - }, true, false, func(got *appsv1.StatefulSet) error { - if len(got.Spec.Template.Spec.Containers) != 1 { - return fmt.Errorf("unexpected count of container, got: %d, want: %d", len(got.Spec.Template.Spec.Containers), 1) - } - if len(got.Spec.VolumeClaimTemplates) != 2 { - return fmt.Errorf("unexpected count of VolumeClaimTemplates, got: %d, want: %d", len(got.Spec.VolumeClaimTemplates), 2) - } - if *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName != "embed-sc" { - return fmt.Errorf("unexpected embed VolumeClaimTemplates name, got: %s, want: %s", *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName, "embed-sc") - } - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("10Gi"), - }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected embed VolumeClaimTemplates resources, diff: %v", diff) - } - if *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName != "default" { - return fmt.Errorf("unexpected extra VolumeClaimTemplates, got: %s, want: %s", *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName, "default") - } - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ - Requests: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceStorage: resource.MustParse("2Gi"), - }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected extra VolumeClaimTemplates resources, diff: %v", diff) - } - return nil + validate: func(got *appsv1.StatefulSet) { + assert.Len(t, got.Spec.Template.Spec.Containers, 1) + assert.Len(t, got.Spec.VolumeClaimTemplates, 2) + assert.Equal(t, *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName, "embed-sc") + assert.Equal(t, got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("10Gi"), + }, + }) + assert.Equal(t, *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName, "default") + assert.Equal(t, got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: resource.MustParse("2Gi"), + }, + }) + }, }) // with oauth2 rw - f(&vmv1.VLAgent{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oauth2", - Namespace: "default", - }, - Spec: vmv1.VLAgentSpec{ - CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ - ReplicaCount: ptr.To(int32(0)), + f(opts{ + cr: &vmv1.VLAgent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oauth2", + Namespace: "default", }, - RemoteWrite: []vmv1.VLAgentRemoteWriteSpec{ - { - URL: "http://some-url", - OAuth2: &vmv1.OAuth2{ - TokenURL: "http://oauth2-svc/auth", - ClientIDSecret: &corev1.SecretKeySelector{ - Key: "client-id", - LocalObjectReference: corev1.LocalObjectReference{ - Name: "oauth2-access", + Spec: vmv1.VLAgentSpec{ + CommonApplicationDeploymentParams: vmv1beta1.CommonApplicationDeploymentParams{ + ReplicaCount: ptr.To(int32(0)), + }, + RemoteWrite: []vmv1.VLAgentRemoteWriteSpec{ + { + URL: "http://some-url", + OAuth2: &vmv1.OAuth2{ + TokenURL: "http://oauth2-svc/auth", + ClientIDSecret: &corev1.SecretKeySelector{ + Key: "client-id", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "oauth2-access", + }, }, - }, - ClientSecret: &corev1.SecretKeySelector{ - Key: "client-secret", - LocalObjectReference: corev1.LocalObjectReference{ - Name: "oauth2-access", + ClientSecret: &corev1.SecretKeySelector{ + Key: "client-secret", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "oauth2-access", + }, }, }, + TLSConfig: &vmv1.TLSConfig{}, }, - TLSConfig: &vmv1.TLSConfig{}, }, }, }, - }, false, false, func(set *appsv1.StatefulSet) error { - cnt := set.Spec.Template.Spec.Containers[0] - if cnt.Name != "vlagent" { - return fmt.Errorf("unexpected container name: %q, want: vlagent", cnt.Name) - } - hasClientSecretArg := false - for _, arg := range cnt.Args { - if strings.Contains(arg, "remoteWrite.oauth2.clientSecretFile") { - hasClientSecretArg = true - break + validate: func(set *appsv1.StatefulSet) { + cnt := set.Spec.Template.Spec.Containers[0] + assert.Equal(t, cnt.Name, "vlagent") + hasClientSecretArg := false + for _, arg := range cnt.Args { + if strings.Contains(arg, "remoteWrite.oauth2.clientSecretFile") { + hasClientSecretArg = true + break + } } - } - if !hasClientSecretArg { - return fmt.Errorf("container must have remoteWrite.oauth2.clientSecretFile flag, has only: %s", strings.Join(cnt.Args, ":,:")) - } - return nil - }, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "oauth2-access", - Namespace: "default", + assert.True(t, hasClientSecretArg) }, - Data: map[string][]byte{ - "client-secret": []byte(`some-secret-value`), - "client-id": []byte(`some-id-value`), + predefinedObjects: []runtime.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oauth2-access", + Namespace: "default", + }, + Data: map[string][]byte{ + "client-secret": []byte(`some-secret-value`), + "client-id": []byte(`some-id-value`), + }, + }, }, }) } @@ -357,9 +333,7 @@ func TestBuildRemoteWriteArgs(t *testing.T) { t.Helper() sort.Strings(want) got, err := buildRemoteWriteArgs(cr) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } + assert.NoError(t, err) sort.Strings(got) assert.Equal(t, want, got) } @@ -798,22 +772,13 @@ func TestMakeSpecForAgentOk(t *testing.T) { scheme.Default(cr) // this trick allows to omit empty fields for yaml var wantSpec corev1.PodSpec - if err := yaml.Unmarshal([]byte(wantYaml), &wantSpec); err != nil { - t.Fatalf("not expected wantYaml: %q: \n%q", wantYaml, err) - } + assert.NoError(t, yaml.Unmarshal([]byte(wantYaml), &wantSpec)) wantYAMLForCompare, err := yaml.Marshal(wantSpec) - if err != nil { - t.Fatalf("BUG: cannot parse as yaml: %q", err) - } + assert.NoError(t, err) got, err := newPodSpec(cr) - if err != nil { - t.Fatalf("not expected error=%q", err) - } + assert.NoError(t, err) gotYAML, err := yaml.Marshal(got) - if err != nil { - t.Fatalf("cannot parse got as yaml: %q", err) - } - + assert.NoError(t, err) assert.Equal(t, string(wantYAMLForCompare), string(gotYAML)) } f(&vmv1.VLAgent{ diff --git a/internal/controller/operator/factory/vlcluster/vlcluster_test.go b/internal/controller/operator/factory/vlcluster/vlcluster_test.go index c5517cc83..bfb07567f 100644 --- a/internal/controller/operator/factory/vlcluster/vlcluster_test.go +++ b/internal/controller/operator/factory/vlcluster/vlcluster_test.go @@ -2,11 +2,8 @@ package vlcluster import ( "context" - "sync" "testing" - "time" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" @@ -19,6 +16,7 @@ import ( vpav1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1 "github.com/VictoriaMetrics/operator/api/operator/v1" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" @@ -45,114 +43,38 @@ func TestCreateOrUpdate(t *testing.T) { *config.MustGetBaseConfig() = defaultCfg }() } - fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) - build.AddDefaults(fclient.Scheme()) - fclient.Scheme().Default(o.cr) - ctx, cancel := context.WithCancel(context.Background()) - var wg sync.WaitGroup - defer func() { - cancel() - wg.Wait() - }() - eventuallyUpdateStatusToOk := func(cb func() error) { - wg.Add(1) - go func() { - defer wg.Done() - tc := time.NewTicker(time.Millisecond * 100) - for { - select { - case <-ctx.Done(): - return - case <-tc.C: - if err := cb(); err != nil { - if k8serrors.IsNotFound(err) { - continue - } - t.Errorf("callback error: %s", err) - return - } - return - } - } - }() - } - if o.cr.Spec.VLStorage != nil { - var vlst appsv1.StatefulSet - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: o.cr.PrefixedName(vmv1beta1.ClusterComponentStorage), Namespace: o.cr.Namespace}, &vlst); err != nil { - return err - } - vlst.Status.ReadyReplicas = *o.cr.Spec.VLStorage.ReplicaCount - vlst.Status.UpdatedReplicas = *o.cr.Spec.VLStorage.ReplicaCount - if err := fclient.Status().Update(ctx, &vlst); err != nil { - return err - } - - return nil - }) - } - if o.cr.Spec.VLSelect != nil { - var vls appsv1.Deployment - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: o.cr.PrefixedName(vmv1beta1.ClusterComponentSelect), Namespace: o.cr.Namespace}, &vls); err != nil { - return err - } - vls.Status.Conditions = append(vls.Status.Conditions, appsv1.DeploymentCondition{ - Type: appsv1.DeploymentProgressing, - Reason: "NewReplicaSetAvailable", - Status: "True", - }) - vls.Status.UpdatedReplicas = *vls.Spec.Replicas - vls.Status.AvailableReplicas = vls.Status.UpdatedReplicas - if err := fclient.Status().Update(ctx, &vls); err != nil { - return err - } - - return nil - }) - - } - if o.cr.Spec.VLInsert != nil { - var vli appsv1.Deployment - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: o.cr.PrefixedName(vmv1beta1.ClusterComponentInsert), Namespace: o.cr.Namespace}, &vli); err != nil { - return err + fclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if obj.GetNamespace() != o.cr.Namespace { + return nil } - vli.Status.Conditions = append(vli.Status.Conditions, appsv1.DeploymentCondition{ - Type: appsv1.DeploymentProgressing, - Reason: "NewReplicaSetAvailable", - Status: "True", - }) - vli.Status.UpdatedReplicas = *vli.Spec.Replicas - vli.Status.AvailableReplicas = vli.Status.UpdatedReplicas - if err := fclient.Status().Update(ctx, &vli); err != nil { - return err - } - return nil - }) - - } - if o.cr.Spec.RequestsLoadBalancer.Enabled { - var vmauthLB appsv1.Deployment - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: o.cr.PrefixedName(vmv1beta1.ClusterComponentBalancer), Namespace: o.cr.Namespace}, &vmauthLB); err != nil { - return err - } - vmauthLB.Status.Conditions = append(vmauthLB.Status.Conditions, appsv1.DeploymentCondition{ - Type: appsv1.DeploymentProgressing, - Reason: "NewReplicaSetAvailable", - Status: "True", - }) - if err := fclient.Status().Update(ctx, &vmauthLB); err != nil { - return err + switch v := obj.(type) { + case *appsv1.StatefulSet: + if v.Name == o.cr.PrefixedName(vmv1beta1.ClusterComponentStorage) { + v.Status.ReadyReplicas = *o.cr.Spec.VLStorage.ReplicaCount + v.Status.UpdatedReplicas = *o.cr.Spec.VLStorage.ReplicaCount + } + case *appsv1.Deployment: + v.Status.Conditions = append(v.Status.Conditions, appsv1.DeploymentCondition{ + Type: appsv1.DeploymentProgressing, + Reason: "NewReplicaSetAvailable", + Status: "True", + }) + v.Status.UpdatedReplicas = *v.Spec.Replicas + v.Status.AvailableReplicas = v.Status.UpdatedReplicas } - + assert.NoError(t, cl.Create(ctx, obj)) return nil - }) - } + }, + }) + build.AddDefaults(fclient.Scheme()) + ctx := context.Background() + fclient.Scheme().Default(o.cr) err := CreateOrUpdate(ctx, fclient, o.cr.DeepCopy()) - if (err != nil) != o.wantErr { - t.Fatalf("unexpected error: %s", err) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } if o.validate != nil { o.validate(ctx, fclient, o.cr) @@ -383,10 +305,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -455,10 +374,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -517,10 +433,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -602,10 +515,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) diff --git a/internal/controller/operator/factory/vmagent/staticscrape_test.go b/internal/controller/operator/factory/vmagent/staticscrape_test.go index 2974318bd..01f0218e6 100644 --- a/internal/controller/operator/factory/vmagent/staticscrape_test.go +++ b/internal/controller/operator/factory/vmagent/staticscrape_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" corev1 "k8s.io/api/core/v1" @@ -30,16 +29,10 @@ func Test_generateStaticScrapeConfig(t *testing.T) { ac := getAssetsCache(ctx, fclient, o.cr) sp := &o.cr.Spec.CommonScrapeParams got, err := generateStaticScrapeConfig(ctx, sp, o.sc, o.sc.Spec.TargetEndpoints[0], 0, ac) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } + assert.NoError(t, err) gotBytes, err := yaml.Marshal(got) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !assert.Equal(t, o.want, string(gotBytes)) { - t.Errorf("generateStaticScrapeConfig(): %s", cmp.Diff(string(gotBytes), o.want)) - } + assert.NoError(t, err) + assert.Equal(t, o.want, string(gotBytes)) } // basic cfg diff --git a/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go b/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go index bb499dc5f..3df440a82 100644 --- a/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go +++ b/internal/controller/operator/factory/vmagent/vmagent_scrapeconfig_test.go @@ -24,19 +24,12 @@ func Test_generateRelabelConfig(t *testing.T) { f := func(rc *vmv1beta1.RelabelConfig, want string) { // related fields only filled during json unmarshal j, err := json.Marshal(rc) - if err != nil { - t.Fatalf("cannot serialize relabelConfig: %s", err) - } + assert.NoError(t, err) var rlbCfg vmv1beta1.RelabelConfig - if err := json.Unmarshal(j, &rlbCfg); err != nil { - t.Fatalf("cannot parse relabelConfig: %s", err) - } + assert.NoError(t, json.Unmarshal(j, &rlbCfg)) got := generateRelabelConfig(&rlbCfg) gotBytes, err := yaml.Marshal(got) - if err != nil { - t.Errorf("cannot marshal generateRelabelConfig to yaml: %e", err) - return - } + assert.NoError(t, err) assert.Equal(t, want, string(gotBytes)) } @@ -127,14 +120,10 @@ func TestCreateOrUpdateScrapeConfig(t *testing.T) { t.Errorf("CreateOrUpdateConfigurationSecret() error = %v", err) } var expectSecret corev1.Secret - if err := testClient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &expectSecret); err != nil { - t.Fatalf("cannot get vmagent config secret: %s", err) - } + assert.NoError(t, testClient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &expectSecret)) gotCfg := expectSecret.Data[scrapeGzippedFilename] data, err := build.GunzipConfig(gotCfg) - if err != nil { - t.Fatalf("cannot gunzip vmagent config: %s", err) - } + assert.NoError(t, err) assert.Equal(t, o.wantConfig, string(data)) } @@ -2213,7 +2202,7 @@ func TestScrapeObjectFailedStatus(t *testing.T) { scrape_configs: [] ` ctx := context.TODO() - testClient := k8stools.GetTestClientWithClientObjects([]client.Object{so}) + testClient := k8stools.GetTestClientWithObjects([]runtime.Object{so}) build.AddDefaults(testClient.Scheme()) cr := &vmv1beta1.VMAgent{ @@ -2237,20 +2226,14 @@ scrape_configs: [] t.Errorf("CreateOrUpdateConfigurationSecret() error = %s", err) } var configSecret corev1.Secret - if err := testClient.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: cr.PrefixedName()}, &configSecret); err != nil { - t.Fatalf("cannot get vmagent config secret: %s", err) - } + assert.NoError(t, testClient.Get(ctx, types.NamespacedName{Namespace: cr.Namespace, Name: cr.PrefixedName()}, &configSecret)) gotCfg := configSecret.Data[scrapeGzippedFilename] data, err := build.GunzipConfig(gotCfg) - if err != nil { - t.Fatalf("cannot gunzip vmagent config: %s", err) - } + assert.NoError(t, err) assert.Equal(t, expectedConfig, string(data)) - if err := testClient.Get(ctx, types.NamespacedName{Name: so.GetName(), Namespace: so.GetNamespace()}, so); err != nil { - t.Fatalf("cannot reload object: %s", err) - } + assert.NoError(t, testClient.Get(ctx, types.NamespacedName{Name: so.GetName(), Namespace: so.GetNamespace()}, so)) status := so.(getStatusMeta).GetStatusMetadata() assert.Equal(t, vmv1beta1.UpdateStatusFailed, status.UpdateStatus) assert.NotEmpty(t, status.Reason) diff --git a/internal/controller/operator/factory/vmagent/vmagent_test.go b/internal/controller/operator/factory/vmagent/vmagent_test.go index 14ded09c3..4b3bd9766 100644 --- a/internal/controller/operator/factory/vmagent/vmagent_test.go +++ b/internal/controller/operator/factory/vmagent/vmagent_test.go @@ -2,27 +2,23 @@ package vmagent import ( "context" - "encoding/json" "fmt" "sort" "strings" "testing" - "time" - "github.com/go-test/deep" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - policyv1 "k8s.io/api/policy/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/build" @@ -32,8 +28,7 @@ import ( func TestCreateOrUpdate(t *testing.T) { type opts struct { cr *vmv1beta1.VMAgent - mustAddPrevSpec bool - validate func(set *appsv1.StatefulSet) error + validate func(set *appsv1.StatefulSet) statefulsetMode bool wantErr bool predefinedObjects []runtime.Object @@ -41,101 +36,43 @@ func TestCreateOrUpdate(t *testing.T) { f := func(o opts) { t.Helper() - fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) + fclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if obj.GetNamespace() != o.cr.Namespace { + return nil + } + switch v := obj.(type) { + case *appsv1.StatefulSet: + v.Status.ObservedGeneration = v.Generation + v.Status.ReadyReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) + v.Status.UpdatedReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) + v.Status.CurrentReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) + case *appsv1.Deployment: + v.Status.Conditions = append(v.Status.Conditions, appsv1.DeploymentCondition{ + Type: appsv1.DeploymentProgressing, + Reason: "NewReplicaSetAvailable", + Status: "True", + }) + v.Status.UpdatedReplicas = *v.Spec.Replicas + v.Status.AvailableReplicas = v.Status.UpdatedReplicas + } + assert.NoError(t, cl.Create(ctx, obj)) + return nil + }, + }) ctx := context.TODO() - if o.mustAddPrevSpec { - jsonSpec, err := json.Marshal(o.cr.Spec) - if err != nil { - t.Fatalf("cannot set last applied spec: %s", err) - } - if o.cr.Annotations == nil { - o.cr.Annotations = make(map[string]string) - } - o.cr.Annotations["operator.victoriametrics/last-applied-spec"] = string(jsonSpec) - } - errC := make(chan error, 1) build.AddDefaults(fclient.Scheme()) fclient.Scheme().Default(o.cr) - go func() { - err := CreateOrUpdate(ctx, o.cr, fclient) - errC <- err - }() - - if o.statefulsetMode { - if o.cr.Spec.ShardCount != nil { - for i := 0; i < *o.cr.Spec.ShardCount; i++ { - err := wait.PollUntilContextTimeout(context.Background(), 20*time.Millisecond, time.Second, true, func(ctx context.Context) (done bool, err error) { - var sts appsv1.StatefulSet - if err := fclient.Get(ctx, types.NamespacedName{ - Namespace: "default", - Name: fmt.Sprintf("vmagent-%s-%d", o.cr.Name, i), - }, &sts); err != nil { - - return false, nil - } - sts.Status.ObservedGeneration = sts.Generation - sts.Status.ReadyReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) - sts.Status.UpdatedReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) - sts.Status.CurrentReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) - if err := fclient.Status().Update(ctx, &sts); err != nil { - return false, err - } - - return true, nil - }) - if err != nil { - t.Errorf("cannot wait sts ready: %s", err) - } - if o.cr.Spec.PodDisruptionBudget != nil { - err = wait.PollUntilContextTimeout(context.Background(), 20*time.Millisecond, time.Second, true, func(ctx context.Context) (done bool, err error) { - var pdb policyv1.PodDisruptionBudget - if err := fclient.Get(ctx, types.NamespacedName{ - Namespace: "default", - Name: fmt.Sprintf("vmagent-%s-%d", o.cr.Name, i), - }, &pdb); err != nil { - - return false, nil - } - return true, nil - }) - if err != nil { - t.Errorf("pdb vmagent-%s-%d didn't get created: %s", o.cr.Name, i, err) - } - } - } - } else { - err := wait.PollUntilContextTimeout(context.Background(), 20*time.Millisecond, time.Second, true, func(ctx context.Context) (done bool, err error) { - var sts appsv1.StatefulSet - if err := fclient.Get(ctx, types.NamespacedName{Namespace: "default", Name: fmt.Sprintf("vmagent-%s", o.cr.Name)}, &sts); err != nil { - return false, nil - } - sts.Status.ReadyReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) - sts.Status.UpdatedReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) - sts.Status.CurrentReplicas = ptr.Deref(o.cr.Spec.ReplicaCount, 0) - err = fclient.Status().Update(ctx, &sts) - if err != nil { - return false, err - } - return true, nil - }) - if err != nil { - t.Errorf("cannot wait sts ready: %s", err) - } - } - } - err := <-errC - if (err != nil) != o.wantErr { - t.Errorf("CreateOrUpdate() error: %s", cmp.Diff(err, o.wantErr)) - return + err := CreateOrUpdate(ctx, o.cr, fclient) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } if o.statefulsetMode && o.cr.Spec.ShardCount == nil { var got appsv1.StatefulSet - if err := fclient.Get(context.Background(), types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got); (err != nil) != o.wantErr { - t.Fatalf("CreateOrUpdate() statefulMode error: %s", cmp.Diff(err, o.wantErr)) - } - if err := o.validate(&got); err != nil { - t.Fatalf("unexpected error: %v", err) - } + assert.NoError(t, fclient.Get(context.Background(), types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got)) + o.validate(&got) } } @@ -187,26 +124,21 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - validate: func(got *appsv1.StatefulSet) error { + validate: func(got *appsv1.StatefulSet) { assert.Equal(t, 1, len(got.Spec.Template.Spec.Containers)) assert.Equal(t, 2, len(got.Spec.VolumeClaimTemplates)) assert.Equal(t, "embed-sc", *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName) - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ + assert.Equal(t, got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("10Gi"), }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected embed VolumeClaimTemplates resources, diff: %v", diff) - } + }) assert.Equal(t, "default", *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName) - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ + assert.Equal(t, got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("2Gi"), }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected extra VolumeClaimTemplates resources, diff: %v", diff) - } - return nil + }) }, statefulsetMode: true, predefinedObjects: []runtime.Object{ @@ -521,11 +453,8 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - validate: func(got *appsv1.StatefulSet) error { - if got.Spec.ServiceName != "my-headless-additional-service" { - return fmt.Errorf("unexpected serviceName, got: %s, want: %s", got.Spec.ServiceName, "my-headless-additional-service") - } - return nil + validate: func(got *appsv1.StatefulSet) { + assert.Equal(t, got.Spec.ServiceName, "my-headless-additional-service") }, statefulsetMode: true, predefinedObjects: []runtime.Object{ @@ -535,7 +464,6 @@ func TestCreateOrUpdate(t *testing.T) { // generate vmagent sharded statefulset with prevSpec f(opts{ - mustAddPrevSpec: true, cr: &vmv1beta1.VMAgent{ ObjectMeta: metav1.ObjectMeta{ Name: "example-agent", @@ -586,41 +514,27 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - validate: func(got *appsv1.StatefulSet) error { - if len(got.Spec.Template.Spec.Containers) != 1 { - return fmt.Errorf("unexpected count of container, got: %d, want: %d", len(got.Spec.Template.Spec.Containers), 1) - } - if len(got.Spec.VolumeClaimTemplates) != 2 { - return fmt.Errorf("unexpected count of VolumeClaimTemplates, got: %d, want: %d", len(got.Spec.VolumeClaimTemplates), 2) - } - if *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName != "embed-sc" { - return fmt.Errorf("unexpected embed VolumeClaimTemplates name, got: %s, want: %s", *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName, "embed-sc") - } - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ + validate: func(got *appsv1.StatefulSet) { + assert.Len(t, got.Spec.Template.Spec.Containers, 1) + assert.Len(t, got.Spec.VolumeClaimTemplates, 2) + assert.Equal(t, *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName, "embed-sc") + assert.Equal(t, got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("10Gi"), }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected embed VolumeClaimTemplates resources, diff: %v", diff) - } - if *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName != "default" { - return fmt.Errorf("unexpected extra VolumeClaimTemplates, got: %s, want: %s", *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName, "default") - } - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ + }) + assert.Equal(t, *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName, "default") + assert.Equal(t, got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("2Gi"), }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected extra VolumeClaimTemplates resources, diff: %v", diff) - } - return nil + }) }, statefulsetMode: true, }) // generate vmagent statefulset with prevSpec f(opts{ - mustAddPrevSpec: true, cr: &vmv1beta1.VMAgent{ ObjectMeta: metav1.ObjectMeta{ Name: "example-agent", @@ -666,34 +580,21 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - validate: func(got *appsv1.StatefulSet) error { - if len(got.Spec.Template.Spec.Containers) != 1 { - return fmt.Errorf("unexpected count of container, got: %d, want: %d", len(got.Spec.Template.Spec.Containers), 1) - } - if len(got.Spec.VolumeClaimTemplates) != 2 { - return fmt.Errorf("unexpected count of VolumeClaimTemplates, got: %d, want: %d", len(got.Spec.VolumeClaimTemplates), 2) - } - if *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName != "embed-sc" { - return fmt.Errorf("unexpected embed VolumeClaimTemplates name, got: %s, want: %s", *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName, "embed-sc") - } - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ + validate: func(got *appsv1.StatefulSet) { + assert.Len(t, got.Spec.Template.Spec.Containers, 1) + assert.Len(t, got.Spec.VolumeClaimTemplates, 2) + assert.Equal(t, *got.Spec.VolumeClaimTemplates[0].Spec.StorageClassName, "embed-sc") + assert.Equal(t, got.Spec.VolumeClaimTemplates[0].Spec.Resources, corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("10Gi"), }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected embed VolumeClaimTemplates resources, diff: %v", diff) - } - if *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName != "default" { - return fmt.Errorf("unexpected extra VolumeClaimTemplates, got: %s, want: %s", *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName, "default") - } - if diff := deep.Equal(got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ + }) + assert.Equal(t, *got.Spec.VolumeClaimTemplates[1].Spec.StorageClassName, "default") + assert.Equal(t, got.Spec.VolumeClaimTemplates[1].Spec.Resources, corev1.VolumeResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceStorage: resource.MustParse("2Gi"), }, - }); len(diff) != 0 { - return fmt.Errorf("unexpected extra VolumeClaimTemplates resources, diff: %v", diff) - } - return nil + }) }, statefulsetMode: true, }) @@ -748,11 +649,9 @@ func TestCreateOrUpdate(t *testing.T) { }, }, statefulsetMode: true, - validate: func(set *appsv1.StatefulSet) error { + validate: func(set *appsv1.StatefulSet) { cnt := set.Spec.Template.Spec.Containers[0] - if cnt.Name != "vmagent" { - return fmt.Errorf("unexpected container name: %q, want: vmagent", cnt.Name) - } + assert.Equal(t, cnt.Name, "vmagent") hasClientSecretArg := false for _, arg := range cnt.Args { if strings.Contains(arg, "remoteWrite.oauth2.clientSecretFile") { @@ -760,10 +659,7 @@ func TestCreateOrUpdate(t *testing.T) { break } } - if !hasClientSecretArg { - return fmt.Errorf("container must have remoteWrite.oauth2.clientSecretFile flag, has only: %s", strings.Join(cnt.Args, ":,:")) - } - return nil + assert.True(t, hasClientSecretArg) }, }) } @@ -781,9 +677,7 @@ func TestBuildRemoteWriteArgs(t *testing.T) { ac := getAssetsCache(ctx, fclient, o.cr) sort.Strings(o.want) got, err := buildRemoteWriteArgs(o.cr, ac) - if err != nil { - t.Fatalf("unexpected error: %s", err) - } + assert.NoError(t, err) sort.Strings(got) assert.Equal(t, o.want, got) } @@ -1748,12 +1642,8 @@ func TestCreateOrUpdateService(t *testing.T) { } if o.wantAdditionalService != nil { var additionalSvc corev1.Service - if err := cl.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.Spec.ServiceSpec.NameOrDefault(o.cr.Name)}, &additionalSvc); err != nil { - t.Fatalf("unexpected error: %s", err) - } - if err := o.wantAdditionalService(&additionalSvc); err != nil { - t.Fatalf("CreateOrUpdateService validation failed for additional service: %s", err) - } + assert.NoError(t, cl.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.Spec.ServiceSpec.NameOrDefault(o.cr.Name)}, &additionalSvc)) + assert.NoError(t, o.wantAdditionalService(&additionalSvc)) } } @@ -1894,26 +1784,20 @@ func TestCreateOrUpdateRelabelConfigsAssets(t *testing.T) { type opts struct { cr *vmv1beta1.VMAgent predefinedObjects []runtime.Object - validate func(cm *corev1.ConfigMap) error + validate func(cm *corev1.ConfigMap) } f := func(o opts) { t.Helper() cl := k8stools.GetTestClientWithObjects(o.predefinedObjects) ctx := context.TODO() ac := build.NewAssetsCache(ctx, cl, nil) - if err := createOrUpdateRelabelConfigsAssets(ctx, cl, o.cr, nil, ac); err != nil { - t.Fatalf("CreateOrUpdateRelabelConfigsAssets() error = %v", err) - } + assert.NoError(t, createOrUpdateRelabelConfigsAssets(ctx, cl, o.cr, nil, ac)) var createdCM corev1.ConfigMap - if err := cl.Get(ctx, types.NamespacedName{ + assert.NoError(t, cl.Get(ctx, types.NamespacedName{ Namespace: o.cr.Namespace, Name: build.ResourceName(build.RelabelConfigResourceKind, o.cr), - }, &createdCM); err != nil { - t.Fatalf("cannot fetch created cm: %v", err) - } - if err := o.validate(&createdCM); err != nil { - t.Fatalf("cannot validate created cm: %v", err) - } + }, &createdCM)) + o.validate(&createdCM) } // simple relabelcfg @@ -1936,18 +1820,15 @@ func TestCreateOrUpdateRelabelConfigsAssets(t *testing.T) { }, }, }, - validate: func(cm *corev1.ConfigMap) error { + validate: func(cm *corev1.ConfigMap) { data, ok := cm.Data[globalRelabelingName] - if !ok { - return fmt.Errorf("key: %s, not exists at map: %v", "global_relabeling.yaml", cm.BinaryData) - } + assert.True(t, ok) wantGlobal := `- source_labels: - pod regex: .* action: DROP ` assert.Equal(t, wantGlobal, data) - return nil }, }) @@ -1974,11 +1855,9 @@ func TestCreateOrUpdateRelabelConfigsAssets(t *testing.T) { }, }, }, - validate: func(cm *corev1.ConfigMap) error { + validate: func(cm *corev1.ConfigMap) { data, ok := cm.Data[globalRelabelingName] - if !ok { - return fmt.Errorf("key: %s, not exists at map: %v", "global_relabeling.yaml", cm.BinaryData) - } + assert.True(t, ok) wantGlobal := strings.TrimSpace(` - source_labels: - pod @@ -1987,7 +1866,6 @@ func TestCreateOrUpdateRelabelConfigsAssets(t *testing.T) { - action: DROP source_labels: ["pod-1"]`) assert.Equal(t, wantGlobal, data) - return nil }, predefinedObjects: []runtime.Object{ &corev1.ConfigMap{ @@ -2006,7 +1884,7 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { type opts struct { cr *vmv1beta1.VMAgent predefinedObjects []runtime.Object - validate func(cm *corev1.ConfigMap) error + validate func(cm *corev1.ConfigMap) } f := func(o opts) { @@ -2014,21 +1892,15 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { cl := k8stools.GetTestClientWithObjects(o.predefinedObjects) ctx := context.TODO() ac := build.NewAssetsCache(ctx, cl, nil) - if err := createOrUpdateStreamAggrConfig(ctx, cl, o.cr, nil, ac); err != nil { - t.Fatalf("CreateOrUpdateStreamAggrConfig() error = %v", err) - } + assert.NoError(t, createOrUpdateStreamAggrConfig(ctx, cl, o.cr, nil, ac)) var createdCM corev1.ConfigMap - if err := cl.Get(ctx, + assert.NoError(t, cl.Get(ctx, types.NamespacedName{ Namespace: o.cr.Namespace, Name: build.ResourceName(build.StreamAggrConfigResourceKind, o.cr), }, &createdCM, - ); err != nil { - t.Fatalf("cannot fetch created cm: %v", err) - } - if err := o.validate(&createdCM); err != nil { - t.Fatalf("cannot validate created cm: %v", err) - } + )) + o.validate(&createdCM) } // simple stream aggr config @@ -2052,18 +1924,15 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { }, }, }, - validate: func(cm *corev1.ConfigMap) error { + validate: func(cm *corev1.ConfigMap) { data, ok := cm.Data["RWS_0-CM-STREAM-AGGR-CONF"] - if !ok { - return fmt.Errorf("key: %s, not exists at map: %v", "RWS_0-CM-STREAM-AGGR-CONFl", cm.BinaryData) - } + assert.True(t, ok) wantGlobal := `- interval: 1m outputs: - total - avg ` assert.Equal(t, wantGlobal, data) - return nil }, }) @@ -2106,11 +1975,9 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { }, }, }, - validate: func(cm *corev1.ConfigMap) error { + validate: func(cm *corev1.ConfigMap) { globalData, ok := cm.Data["global_aggregation.yaml"] - if !ok { - return fmt.Errorf("key: %s, not exists at map: %v", "global_aggregation.yaml", cm.BinaryData) - } + assert.True(t, ok) wantGlobal := `- match: test interval: 30s outputs: @@ -2123,9 +1990,7 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { ` assert.Equal(t, wantGlobal, globalData) remoteData, ok := cm.Data["RWS_0-CM-STREAM-AGGR-CONF"] - if !ok { - return fmt.Errorf("key: %s, not exists at map: %v", "RWS_0-CM-STREAM-AGGR-CONFl", cm.BinaryData) - } + assert.True(t, ok) wantRemote := `- match: - '{__name__="count1"}' - '{__name__="count2"}' @@ -2143,7 +2008,6 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { - regex: (.+):.+ ` assert.Equal(t, wantRemote, remoteData) - return nil }, }) @@ -2178,11 +2042,9 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { }, }, }, - validate: func(cm *corev1.ConfigMap) error { + validate: func(cm *corev1.ConfigMap) { data, ok := cm.Data["RWS_0-CM-STREAM-AGGR-CONF"] - if !ok { - return fmt.Errorf("key: %s, not exists at map: %v", "RWS_0-CM-STREAM-AGGR-CONFl", cm.BinaryData) - } + assert.True(t, ok) wantGlobal := `- match: - '{__name__="count1"}' - '{__name__="count2"}' @@ -2204,7 +2066,6 @@ func TestCreateOrUpdateStreamAggrConfig(t *testing.T) { - vmauth ` assert.Equal(t, wantGlobal, data) - return nil }, }) } @@ -2225,22 +2086,13 @@ func TestMakeSpecForAgentOk(t *testing.T) { scheme.Default(o.cr) // this trick allows to omit empty fields for yaml var wantSpec corev1.PodSpec - if err := yaml.Unmarshal([]byte(o.wantYaml), &wantSpec); err != nil { - t.Fatalf("unexpected error: %e", err) - } + assert.NoError(t, yaml.Unmarshal([]byte(o.wantYaml), &wantSpec)) wantYAMLForCompare, err := yaml.Marshal(wantSpec) - if err != nil { - t.Fatalf("BUG: cannot parse as yaml: %q", err) - } + assert.NoError(t, err) got, err := newPodSpec(o.cr, ac) - if err != nil { - t.Fatalf("not expected error=%q", err) - } + assert.NoError(t, err) gotYAML, err := yaml.Marshal(got) - if err != nil { - t.Fatalf("cannot parse got as yaml: %q", err) - } - + assert.NoError(t, err) assert.Equal(t, string(wantYAMLForCompare), string(gotYAML)) } f(opts{ diff --git a/internal/controller/operator/factory/vmalert/vmalert_test.go b/internal/controller/operator/factory/vmalert/vmalert_test.go index 3da987349..b9c04c3da 100644 --- a/internal/controller/operator/factory/vmalert/vmalert_test.go +++ b/internal/controller/operator/factory/vmalert/vmalert_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -27,7 +26,7 @@ func TestCreateOrUpdate(t *testing.T) { cr *vmv1beta1.VMAlert cmNames []string predefinedObjects []runtime.Object - validator func(*appsv1.Deployment, *corev1.Secret) error + validate func(*appsv1.Deployment, *corev1.Secret) } f := func(o opts) { t.Helper() @@ -39,26 +38,18 @@ func TestCreateOrUpdate(t *testing.T) { return } - if o.validator != nil { + if o.validate != nil { var generatedDeploment appsv1.Deployment - if err := fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &generatedDeploment); err != nil { - t.Fatalf("cannot find generated deployment=%q, err: %v", o.cr.PrefixedName(), err) - } + assert.NoError(t, fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &generatedDeploment)) var generatedTLSSecret corev1.Secret tlsSecretName := build.ResourceName(build.TLSAssetsResourceKind, o.cr) - if err := fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: tlsSecretName}, &generatedTLSSecret); err != nil { - if !k8serrors.IsNotFound(err) { - t.Fatalf("unexpected error during attempt to get tls secret=%q, err: %v", build.ResourceName(build.TLSAssetsResourceKind, o.cr), err) - } - } - if err := o.validator(&generatedDeploment, &generatedTLSSecret); err != nil { - t.Fatalf("unexpected error at deployment validation: %v", err) - } + assert.NoError(t, fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: tlsSecretName}, &generatedTLSSecret)) + o.validate(&generatedDeploment, &generatedTLSSecret) } } // base-spec-gen - o := opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -90,11 +81,10 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - } - f(o) + }) // base-spec-gen with externalLabels - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -137,7 +127,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - validator: func(d *appsv1.Deployment, s *corev1.Secret) error { + validate: func(d *appsv1.Deployment, s *corev1.Secret) { var foundOk bool for _, cnt := range d.Spec.Template.Spec.Containers { if cnt.Name == "vmalert" { @@ -146,23 +136,17 @@ func TestCreateOrUpdate(t *testing.T) { if strings.HasPrefix(arg, "-external.label") { foundOk = true kv := strings.ReplaceAll(arg, "-external.label=", "") - if kv != "label1=value1" && kv != "label2=value-2" { - return fmt.Errorf("unexpected value for external.label arg: %s", kv) - } + assert.True(t, kv == "label1=value1" || kv == "label2=value-2") } } } } - if !foundOk { - return fmt.Errorf("expected to found arg: -external.label at vmalert container") - } - return nil + assert.True(t, foundOk) }, - } - f(o) + }) // with-remote-tls - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -261,23 +245,15 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - validator: func(d *appsv1.Deployment, s *corev1.Secret) error { - if _, ok := s.Data["default_configmap_datasource-tls_ca"]; !ok { - return fmt.Errorf("failed to find expected TLS CA") - } - if _, ok := s.Data["default_configmap_datasource-tls_cert"]; !ok { - return fmt.Errorf("failed to find expected TLS cert") - } - if _, ok := s.Data["default_datasource-tls_key"]; !ok { - return fmt.Errorf("failed to find TLS key") - } - return nil + validate: func(d *appsv1.Deployment, s *corev1.Secret) { + assert.NotEmpty(t, s.Data["default_configmap_datasource-tls_ca"]) + assert.NotEmpty(t, s.Data["default_configmap_datasource-tls_cert"]) + assert.NotEmpty(t, s.Data["default_datasource-tls_key"]) }, - } - f(o) + }) // with-notifiers-tls - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -358,11 +334,10 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - } - f(o) + }) // with tlsconfig insecure true - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -437,11 +412,10 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - } - f(o) + }) // with notifier config - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -483,8 +457,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, }, - } - f(o) + }) } func TestBuildNotifiers(t *testing.T) { @@ -506,7 +479,7 @@ func TestBuildNotifiers(t *testing.T) { } // ok build args - o := opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -541,11 +514,10 @@ func TestBuildNotifiers(t *testing.T) { "-notifier.tlsCAFile=,/tmp/ca.cert,", "-notifier.tlsInsecureSkipVerify=false,true,false", }, - } - f(o) + }) // ok build args with config - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -562,11 +534,10 @@ func TestBuildNotifiers(t *testing.T) { }, want: []string{"-notifier.config=" + notifierConfigMountPath + "/cfg.yaml"}, predefinedObjects: []runtime.Object{}, - } - f(o) + }) // with headers and oauth2 - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -637,8 +608,7 @@ func TestBuildNotifiers(t *testing.T) { }, }, want: []string{"-notifier.url=http://1,http://2", "-notifier.headers=key=value^^key2=value2,key3=value3^^key4=value4", "-notifier.bearerTokenFile=,/etc/vmalert/remote_secrets/default_bearer_bearer", "-notifier.oauth2.clientSecretFile=/etc/vmalert/remote_secrets/default_oauth2_client-secret,", "-notifier.oauth2.clientID=some-id,", "-notifier.oauth2.scopes=1,2,", "-notifier.oauth2.tokenUrl=http://some-url,"}, - } - f(o) + }) } func TestCreateOrUpdateService(t *testing.T) { @@ -666,7 +636,7 @@ func TestCreateOrUpdateService(t *testing.T) { } // base test - o := opts{ + f(opts{ c: config.MustGetBaseConfig(), cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ @@ -680,8 +650,7 @@ func TestCreateOrUpdateService(t *testing.T) { } return nil }, - } - f(o) + }) } func Test_buildVMAlertArgs(t *testing.T) { @@ -708,7 +677,7 @@ func Test_buildVMAlertArgs(t *testing.T) { } // basic args - o := opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -727,11 +696,10 @@ func Test_buildVMAlertArgs(t *testing.T) { }, ruleConfigMapNames: []string{"first-rule-cm.yaml"}, want: []string{"-datasource.url=http://vmsingle-url", "-httpListenAddr=:", "-notifier.url=http://test", "-rule=\"/etc/vmalert/config/first-rule-cm.yaml/*.yaml\""}, - } - f(o) + }) // with tls args - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Namespace: "default", @@ -758,11 +726,10 @@ func Test_buildVMAlertArgs(t *testing.T) { }, ruleConfigMapNames: []string{"first-rule-cm.yaml"}, want: []string{"--datasource.headers=x-org-id:one^^x-org-tenant:5", "-datasource.tlsCAFile=/path/to/sa", "-datasource.tlsInsecureSkipVerify=true", "-datasource.tlsKeyFile=/path/to/key", "-datasource.url=http://vmsingle-url", "-httpListenAddr=:", "-notifier.url=http://test", "-rule=\"/etc/vmalert/config/first-rule-cm.yaml/*.yaml\""}, - } - f(o) + }) // with static and selector notifiers - o = opts{ + f(opts{ cr: &vmv1beta1.VMAlert{ ObjectMeta: metav1.ObjectMeta{ Name: "basic-vmalert", @@ -839,6 +806,5 @@ func Test_buildVMAlertArgs(t *testing.T) { "-notifier.tlsKeyFile=,/tmp/key.pem,,,", "-notifier.url=http://am-1,http://am-2,http://am-3,http://vmalertmanager-test-system-0.vmalertmanager-test-system.monitoring.svc:9093,http://vmalertmanager-test-am-0.vmalertmanager-test-am.monitoring.svc:9093", }, - } - f(o) + }) } diff --git a/internal/controller/operator/factory/vmalertmanager/alertmanager_test.go b/internal/controller/operator/factory/vmalertmanager/alertmanager_test.go index 04c4da6ad..d21d1acd4 100644 --- a/internal/controller/operator/factory/vmalertmanager/alertmanager_test.go +++ b/internal/controller/operator/factory/vmalertmanager/alertmanager_test.go @@ -2,12 +2,9 @@ package vmalertmanager import ( "context" - "fmt" "strings" "testing" - "time" - "github.com/go-test/deep" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -17,6 +14,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" "github.com/VictoriaMetrics/operator/internal/controller/operator/factory/build" @@ -26,54 +25,40 @@ import ( func TestCreateOrUpdateAlertManager(t *testing.T) { type opts struct { cr *vmv1beta1.VMAlertmanager - validate func(set *appsv1.StatefulSet) error + validate func(set *appsv1.StatefulSet) wantErr bool predefinedObjects []runtime.Object } f := func(o opts) { t.Helper() - fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) + fclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if obj.GetNamespace() != o.cr.Namespace { + return nil + } + sts, ok := obj.(*appsv1.StatefulSet) + if !ok { + return nil + } + sts.Status.ReadyReplicas = *o.cr.Spec.ReplicaCount + sts.Status.UpdatedReplicas = *o.cr.Spec.ReplicaCount + return cl.Create(ctx, obj) + }, + }) build.AddDefaults(fclient.Scheme()) fclient.Scheme().Default(o.cr) ctx := context.TODO() - ctx, cancel := context.WithTimeout(ctx, time.Second*20) - defer cancel() - - go func() { - tc := time.NewTicker(time.Millisecond * 100) - for { - select { - case <-ctx.Done(): - return - case <-tc.C: - var got appsv1.StatefulSet - if err := fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got); err != nil { - if !k8serrors.IsNotFound(err) { - t.Errorf("cannot get statefulset for alertmanager: %s", err) - return - } - continue - } - got.Status.ReadyReplicas = *o.cr.Spec.ReplicaCount - got.Status.UpdatedReplicas = *o.cr.Spec.ReplicaCount - - if err := fclient.Status().Update(ctx, &got); err != nil { - t.Errorf("cannot update status statefulset for alertmanager: %s", err) - } - return - } - } - }() err := CreateOrUpdateAlertManager(ctx, o.cr, fclient) - if (err != nil) != o.wantErr { - t.Fatalf("CreateOrUpdateAlertManager() error = %v, wantErr %v", err, o.wantErr) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } - // TODO add client.Default - var got appsv1.StatefulSet - if err := fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got); (err != nil) != o.wantErr { - t.Fatalf("CreateOrUpdateAlertManager() error = %v, wantErr %v", err, o.wantErr) + if o.validate != nil { + var got appsv1.StatefulSet + assert.NoError(t, fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got)) + o.validate(&got) } - assert.NoError(t, o.validate(&got)) } // simple alertmanager @@ -93,11 +78,9 @@ func TestCreateOrUpdateAlertManager(t *testing.T) { }, }, }, - validate: func(set *appsv1.StatefulSet) error { - if set.Name != "vmalertmanager-test-am" { - return fmt.Errorf("unexpected name, got: %s, want: %s", set.Name, "vmalertmanager-test-am") - } - if diff := deep.Equal(set.Spec.Template.Spec.Containers[0].Resources, corev1.ResourceRequirements{ + validate: func(set *appsv1.StatefulSet) { + assert.Equal(t, set.Name, "vmalertmanager-test-am") + assert.Equal(t, set.Spec.Template.Spec.Containers[0].Resources, corev1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("100m"), corev1.ResourceMemory: resource.MustParse("256Mi"), @@ -106,19 +89,14 @@ func TestCreateOrUpdateAlertManager(t *testing.T) { corev1.ResourceCPU: resource.MustParse("30m"), corev1.ResourceMemory: resource.MustParse("56Mi"), }, - }); len(diff) > 0 { - return fmt.Errorf("unexpected diff with resources: %v", diff) - } - if diff := deep.Equal(set.Labels, map[string]string{ + }) + assert.Equal(t, set.Labels, map[string]string{ "app.kubernetes.io/component": "monitoring", "app.kubernetes.io/instance": "test-am", "app.kubernetes.io/name": "vmalertmanager", "managed-by": "vm-operator", "main": "system", - }); len(diff) > 0 { - return fmt.Errorf("unexpected diff with labels: %v", diff) - } - return nil + }) }, }) @@ -142,14 +120,13 @@ func TestCreateOrUpdateAlertManager(t *testing.T) { }, }, }, - validate: func(set *appsv1.StatefulSet) error { + validate: func(set *appsv1.StatefulSet) { assert.Len(t, set.Spec.Template.Spec.Containers, 2) vmaContainer := set.Spec.Template.Spec.Containers[0] assert.Equal(t, vmaContainer.Name, "alertmanager") assert.Equal(t, vmaContainer.LivenessProbe.TimeoutSeconds, int32(20)) assert.Equal(t, vmaContainer.LivenessProbe.HTTPGet.Path, "/-/healthy") assert.Equal(t, vmaContainer.ReadinessProbe.HTTPGet.Path, "/-/healthy") - return nil }, }) @@ -181,51 +158,26 @@ func TestCreateOrUpdateAlertManager(t *testing.T) { }, }, }, - validate: func(set *appsv1.StatefulSet) error { - if set.Name != "vmalertmanager-test-am" { - return fmt.Errorf("unexpected name, got: %s, want: %s", set.Name, "vmalertmanager-test-am") - } - if len(set.Spec.Template.Spec.Volumes) != 4 { - return fmt.Errorf("unexpected count of volumes, got: %d, want: %d", len(set.Spec.Template.Spec.Volumes), 4) - } + validate: func(set *appsv1.StatefulSet) { + assert.Equal(t, set.Name, "vmalertmanager-test-am") + assert.Len(t, set.Spec.Template.Spec.Volumes, 4) templatesVolume := set.Spec.Template.Spec.Volumes[2] - if templatesVolume.Name != "templates-test-am" { - return fmt.Errorf("unexpected volume name, got: %s, want: %s", templatesVolume.Name, "templates-test-am") - } - if templatesVolume.ConfigMap.Name != "test-am" { - return fmt.Errorf("unexpected configmap name, got: %s, want: %s", templatesVolume.ConfigMap.Name, "test-am") - } - + assert.Equal(t, templatesVolume.Name, "templates-test-am") + assert.Equal(t, templatesVolume.ConfigMap.Name, "test-am") vmaContainer := set.Spec.Template.Spec.Containers[0] - if vmaContainer.Name != "alertmanager" { - return fmt.Errorf("unexpected container name, got: %s, want: %s", vmaContainer.Name, "alertmanager") - } - - if len(vmaContainer.VolumeMounts) != 4 { - return fmt.Errorf("unexpected count of volume mounts, got: %d, want: %d", len(vmaContainer.VolumeMounts), 4) - } + assert.Equal(t, vmaContainer.Name, "alertmanager") + assert.Len(t, vmaContainer.VolumeMounts, 4) templatesVolumeMount := vmaContainer.VolumeMounts[3] - if templatesVolumeMount.Name != "templates-test-am" { - return fmt.Errorf("unexpected volume name, got: %s, want: %s", templatesVolumeMount.Name, "templates-test-am") - } - if templatesVolumeMount.MountPath != "/etc/vm/templates/test-am" { - return fmt.Errorf("unexpected volume mount path, got: %s, want: %s", templatesVolumeMount.MountPath, "/etc/vm/templates/test-am") - } - if !templatesVolumeMount.ReadOnly { - return fmt.Errorf("unexpected volume mount read only, got: %t, want: %t", templatesVolumeMount.ReadOnly, true) - } - + assert.Equal(t, templatesVolumeMount.Name, "templates-test-am") + assert.Equal(t, templatesVolumeMount.MountPath, "/etc/vm/templates/test-am") + assert.True(t, templatesVolumeMount.ReadOnly) foundTemplatesDir := false for _, arg := range set.Spec.Template.Spec.Containers[1].Args { if arg == "--watched-dir=/etc/vm/templates/test-am" { foundTemplatesDir = true } } - if !foundTemplatesDir { - return fmt.Errorf("templates dir not found in args of config-reloader container") - } - - return nil + assert.True(t, foundTemplatesDir) }, }) @@ -244,7 +196,7 @@ func TestCreateOrUpdateAlertManager(t *testing.T) { }, }, }, - validate: func(set *appsv1.StatefulSet) error { + validate: func(set *appsv1.StatefulSet) { assert.Len(t, set.Spec.Template.Spec.Containers, 2) vmaContainer := set.Spec.Template.Spec.Containers[0] @@ -260,7 +212,6 @@ func TestCreateOrUpdateAlertManager(t *testing.T) { "--cluster.peer=vmalertmanager-test-am-1.vmalertmanager-test-am.monitoring:9094", "--cluster.peer=vmalertmanager-test-am-2.vmalertmanager-test-am.monitoring:9094", }, "unexpected cluster peer arguments found") - return nil }, }) @@ -280,7 +231,7 @@ func TestCreateOrUpdateAlertManager(t *testing.T) { }, }, }, - validate: func(set *appsv1.StatefulSet) error { + validate: func(set *appsv1.StatefulSet) { assert.Len(t, set.Spec.Template.Spec.Containers, 2) vmaContainer := set.Spec.Template.Spec.Containers[0] @@ -296,7 +247,6 @@ func TestCreateOrUpdateAlertManager(t *testing.T) { "--cluster.peer=vmalertmanager-test-am-1.vmalertmanager-test-am.monitoring.svc.example.com.:9094", "--cluster.peer=vmalertmanager-test-am-2.vmalertmanager-test-am.monitoring.svc.example.com.:9094", }, "unexpected cluster peer arguments found") - return nil }, }) } diff --git a/internal/controller/operator/factory/vmalertmanager/config_test.go b/internal/controller/operator/factory/vmalertmanager/config_test.go index 26360946a..eb60cd7d1 100644 --- a/internal/controller/operator/factory/vmalertmanager/config_test.go +++ b/internal/controller/operator/factory/vmalertmanager/config_test.go @@ -24,9 +24,7 @@ import ( func mustRouteToJSON(t *testing.T, r vmv1beta1.SubRoute) apiextensionsv1.JSON { t.Helper() data, err := json.Marshal(r) - if err != nil { - t.Fatalf("unexpected json marshal error: %s", err) - } + assert.NoError(t, err) return apiextensionsv1.JSON{Raw: data} } @@ -1879,8 +1877,11 @@ func Test_UpdateDefaultAMConfig(t *testing.T) { ctx := context.TODO() // Create secret with alert manager config - if err := CreateOrUpdateConfig(ctx, fclient, o.cr, nil); (err != nil) != o.wantErr { - t.Fatalf("CreateOrUpdateConfig() error = %v, wantErr %v", err, o.wantErr) + err := CreateOrUpdateConfig(ctx, fclient, o.cr, nil) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } var amCfgs []*vmv1beta1.VMAlertmanagerConfig opts := &k8stools.SelectorOpts{ @@ -1912,15 +1913,11 @@ func Test_UpdateDefaultAMConfig(t *testing.T) { // check secret config after creating d, ok := createdSecret.Data[alertmanagerSecretConfigKeyGz] - if !ok { - t.Fatalf("config for alertmanager not exist") - } + assert.True(t, ok) var secretConfig alertmanagerConfig - if data, err := build.GunzipConfig(d); err != nil { - t.Fatalf("failed to gunzip config: %s", err) - } else { - assert.NoError(t, yaml.Unmarshal(data, &secretConfig)) - } + data, err := build.GunzipConfig(d) + assert.NoError(t, err) + assert.NoError(t, yaml.Unmarshal(data, &secretConfig)) var amc vmv1beta1.VMAlertmanagerConfig assert.NoError(t, fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: "test-amc"}, &amc)) // we add blachole as first route by default @@ -1929,8 +1926,11 @@ func Test_UpdateDefaultAMConfig(t *testing.T) { assert.Len(t, secretConfig.Route.Routes, 1) assert.Len(t, secretConfig.Route.Routes[0], len(amc.Spec.Route.Routes)+2) // Update secret with alert manager config - if err := CreateOrUpdateConfig(ctx, fclient, o.cr, nil); (err != nil) != o.wantErr { - t.Fatalf("CreateOrUpdateConfig() error = %v, wantErr %v", err, o.wantErr) + err = CreateOrUpdateConfig(ctx, fclient, o.cr, nil) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } if err := fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: secretName}, &createdSecret); err != nil { @@ -1942,14 +1942,10 @@ func Test_UpdateDefaultAMConfig(t *testing.T) { // check secret config after updating d, ok = createdSecret.Data[alertmanagerSecretConfigKeyGz] - if !ok { - t.Fatalf("config for alertmanager not exist") - } - if data, err := build.GunzipConfig(d); err != nil { - t.Fatalf("failed to gunzip alertmanager config: %s", err) - } else { - assert.NoError(t, yaml.Unmarshal(data, &secretConfig)) - } + assert.True(t, ok) + data, err = build.GunzipConfig(d) + assert.NoError(t, err) + assert.NoError(t, yaml.Unmarshal(data, &secretConfig)) assert.Len(t, secretConfig.Receivers, len(amc.Spec.Receivers)+1) assert.Len(t, secretConfig.InhibitRules, len(amc.Spec.InhibitRules)) assert.Len(t, secretConfig.Route.Routes, 1) @@ -2034,8 +2030,10 @@ func TestBuildWebConfig(t *testing.T) { ctx := context.TODO() ac := getAssetsCache(ctx, fclient, o.cr) c, err := buildWebServerConfigYAML(o.cr, ac) - if (err != nil) != o.wantErr { - t.Fatalf("unexpected error: %q", err) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } assert.Equal(t, o.want, string(c)) } @@ -2184,7 +2182,6 @@ func TestBuildGossipConfig(t *testing.T) { cr *vmv1beta1.VMAlertmanager predefinedObjects []runtime.Object want string - wantErr bool } f := func(o opts) { @@ -2193,9 +2190,7 @@ func TestBuildGossipConfig(t *testing.T) { ctx := context.TODO() ac := getAssetsCache(ctx, fclient, o.cr) c, err := buildGossipConfigYAML(o.cr, ac) - if (err != nil) != o.wantErr { - t.Fatalf("unexpected error: %q", err) - } + assert.NoError(t, err) assert.Equal(t, o.want, string(c)) } diff --git a/internal/controller/operator/factory/vmanomaly/config/config_test.go b/internal/controller/operator/factory/vmanomaly/config/config_test.go index e21197810..c7973b241 100644 --- a/internal/controller/operator/factory/vmanomaly/config/config_test.go +++ b/internal/controller/operator/factory/vmanomaly/config/config_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -39,14 +40,14 @@ func TestLoad(t *testing.T) { } ac := build.NewAssetsCache(ctx, fclient, cfg) loaded, err := Load(o.cr, ac) - if (err != nil) != o.wantErr { - t.Fatalf("Load() error = %v, wantErr %v", err, o.wantErr) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } expected := strings.TrimSpace(o.expected) got := strings.TrimSpace(string(loaded)) - if got != expected { - t.Fatalf("unexpected config produced by Load(): \nexpected:\n%s\ngot:\n%s", expected, got) - } + assert.Equal(t, got, expected) } // no custom readers and writers diff --git a/internal/controller/operator/factory/vmanomaly/statefulset_test.go b/internal/controller/operator/factory/vmanomaly/statefulset_test.go index dbafc14bb..3d4d793c4 100644 --- a/internal/controller/operator/factory/vmanomaly/statefulset_test.go +++ b/internal/controller/operator/factory/vmanomaly/statefulset_test.go @@ -2,11 +2,8 @@ package vmanomaly import ( "context" - "fmt" "testing" - "time" - "github.com/go-test/deep" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -16,6 +13,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1 "github.com/VictoriaMetrics/operator/api/operator/v1" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" @@ -26,53 +25,41 @@ import ( func TestCreateOrUpdate(t *testing.T) { type opts struct { cr *vmv1.VMAnomaly - validate func(set *appsv1.StatefulSet) error + validate func(set *appsv1.StatefulSet) wantErr bool predefinedObjects []runtime.Object } f := func(o opts) { t.Helper() - fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) + ctx := context.TODO() + fclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if obj.GetNamespace() != o.cr.Namespace { + return nil + } + sts, ok := obj.(*appsv1.StatefulSet) + if !ok { + return nil + } + sts.Status.ReadyReplicas = *o.cr.Spec.ReplicaCount + sts.Status.UpdatedReplicas = *o.cr.Spec.ReplicaCount + return cl.Create(ctx, obj) + }, + }) + build.AddDefaults(fclient.Scheme()) fclient.Scheme().Default(o.cr) - ctx, cancel := context.WithTimeout(context.TODO(), time.Second*20) - defer cancel() - - go func() { - tc := time.NewTicker(time.Millisecond * 100) - for { - select { - case <-ctx.Done(): - return - case <-tc.C: - var got appsv1.StatefulSet - if err := fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got); err != nil { - if !k8serrors.IsNotFound(err) { - t.Errorf("cannot get statefulset for vmanomaly: %s", err) - return - } - continue - } - got.Status.ReadyReplicas = *o.cr.Spec.ReplicaCount - got.Status.UpdatedReplicas = *o.cr.Spec.ReplicaCount - - if err := fclient.Status().Update(ctx, &got); err != nil { - t.Errorf("cannot update status statefulset for vmanomaly: %s", err) - } - return - } - } - }() err := CreateOrUpdate(ctx, o.cr, fclient) - if (err != nil) != o.wantErr { - t.Fatalf("CreateOrUpdate() error = %v, wantErr %v", err, o.wantErr) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } - // TODO add client.Default - var got appsv1.StatefulSet - if err := fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got); (err != nil) != o.wantErr { - t.Fatalf("CreateOrUpdate() error = %v, wantErr %v", err, o.wantErr) + if o.validate != nil { + var got appsv1.StatefulSet + assert.NoError(t, fclient.Get(ctx, types.NamespacedName{Namespace: o.cr.Namespace, Name: o.cr.PrefixedName()}, &got)) + o.validate(&got) } - assert.NoError(t, o.validate(&got)) } // simple vmanomaly @@ -119,11 +106,9 @@ schedulers: }, }, }, - validate: func(set *appsv1.StatefulSet) error { - if set.Name != "vmanomaly-test-anomaly" { - return fmt.Errorf("unexpected name, got: %s, want: %s", set.Name, "vmanomaly-test-anomaly") - } - if diff := deep.Equal(set.Spec.Template.Spec.Containers[0].Resources, corev1.ResourceRequirements{ + validate: func(set *appsv1.StatefulSet) { + assert.Equal(t, set.Name, "vmanomaly-test-anomaly") + assert.Equal(t, set.Spec.Template.Spec.Containers[0].Resources, corev1.ResourceRequirements{ Limits: corev1.ResourceList{ corev1.ResourceCPU: resource.MustParse("200m"), corev1.ResourceMemory: resource.MustParse("500Mi"), @@ -132,18 +117,13 @@ schedulers: corev1.ResourceCPU: resource.MustParse("50m"), corev1.ResourceMemory: resource.MustParse("200Mi"), }, - }); len(diff) > 0 { - return fmt.Errorf("unexpected diff with resources: %v", diff) - } - if diff := deep.Equal(set.Labels, map[string]string{ + }) + assert.Equal(t, set.Labels, map[string]string{ "app.kubernetes.io/component": "monitoring", "app.kubernetes.io/instance": "test-anomaly", "app.kubernetes.io/name": "vmanomaly", "managed-by": "vm-operator", - }); len(diff) > 0 { - return fmt.Errorf("unexpected diff with labels: %v", diff) - } - return nil + }) }, }) @@ -194,25 +174,13 @@ schedulers: }, }, }, - validate: func(set *appsv1.StatefulSet) error { - if len(set.Spec.Template.Spec.Containers) != 1 { - return fmt.Errorf("unexpected count of container, got: %d, want: %d", len(set.Spec.Template.Spec.Containers), 2) - } + validate: func(set *appsv1.StatefulSet) { + assert.Len(t, set.Spec.Template.Spec.Containers, 1) container := set.Spec.Template.Spec.Containers[0] - if container.Name != "vmanomaly" { - return fmt.Errorf("unexpected container name, got: %s, want: %s", container.Name, "vmanomaly") - } - if container.LivenessProbe.TimeoutSeconds != 20 { - return fmt.Errorf("unexpected liveness probe config, want timeout: %d, got: %d", container.LivenessProbe.TimeoutSeconds, 20) - } - if container.LivenessProbe.HTTPGet.Path != "/health" { - return fmt.Errorf("unexpected path for probe, got: %s, want: %s", container.LivenessProbe.HTTPGet.Path, "/health") - } - if container.ReadinessProbe.HTTPGet.Path != "/health" { - return fmt.Errorf("unexpected path for probe, got: %s, want: %s", container.ReadinessProbe.HTTPGet.Path, "/health") - } - - return nil + assert.Equal(t, container.Name, "vmanomaly") + assert.Equal(t, container.LivenessProbe.TimeoutSeconds, int32(20)) + assert.Equal(t, container.LivenessProbe.HTTPGet.Path, "/health") + assert.Equal(t, container.ReadinessProbe.HTTPGet.Path, "/health") }, }) @@ -261,20 +229,12 @@ schedulers: }, }, }, - validate: func(set *appsv1.StatefulSet) error { - if len(set.Spec.Template.Spec.Containers) != 1 { - return fmt.Errorf("unexpected count of container, got: %d, want: %d", len(set.Spec.Template.Spec.Containers), 1) - } + validate: func(set *appsv1.StatefulSet) { + assert.Len(t, set.Spec.Template.Spec.Containers, 1) container := set.Spec.Template.Spec.Containers[0] expectedPath := "/custom-prefix/health" - if container.LivenessProbe.HTTPGet.Path != expectedPath { - return fmt.Errorf("unexpected liveness probe path, got: %s, want: %s", container.LivenessProbe.HTTPGet.Path, expectedPath) - } - if container.ReadinessProbe.HTTPGet.Path != expectedPath { - return fmt.Errorf("unexpected readiness probe path, got: %s, want: %s", container.ReadinessProbe.HTTPGet.Path, expectedPath) - } - - return nil + assert.Equal(t, container.LivenessProbe.HTTPGet.Path, expectedPath) + assert.Equal(t, container.ReadinessProbe.HTTPGet.Path, expectedPath) }, }) } diff --git a/internal/controller/operator/factory/vmauth/vmauth_test.go b/internal/controller/operator/factory/vmauth/vmauth_test.go index 12a54bb20..deb7f73ba 100644 --- a/internal/controller/operator/factory/vmauth/vmauth_test.go +++ b/internal/controller/operator/factory/vmauth/vmauth_test.go @@ -4,7 +4,6 @@ import ( "context" "testing" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" autoscalingv1 "k8s.io/api/autoscaling/v1" @@ -274,10 +273,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -353,10 +349,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -420,21 +413,13 @@ func TestMakeSpecForAuthOk(t *testing.T) { build.AddDefaults(scheme) scheme.Default(cr) var wantSpec corev1.PodSpec - if err := yaml.Unmarshal([]byte(wantYaml), &wantSpec); err != nil { - t.Fatalf("not expected wantYaml: %q: \n%q", wantYaml, err) - } + assert.NoError(t, yaml.Unmarshal([]byte(wantYaml), &wantSpec)) wantYAMLForCompare, err := yaml.Marshal(wantSpec) - if err != nil { - t.Fatalf("BUG: cannot parse as yaml: %q", err) - } + assert.NoError(t, err) got, err := makeSpecForVMAuth(cr) - if err != nil { - t.Fatalf("not expected error=%q", err) - } + assert.NoError(t, err) gotYAML, err := yaml.Marshal(got.Spec) - if err != nil { - t.Fatalf("cannot parse got as yaml: %q", err) - } + assert.NoError(t, err) assert.Equal(t, string(wantYAMLForCompare), string(gotYAML)) } @@ -672,18 +657,12 @@ func TestBuildIngressForAuthOk(t *testing.T) { build.AddDefaults(scheme) scheme.Default(cr) var wantSpec networkingv1.IngressSpec - if err := yaml.Unmarshal([]byte(wantYaml), &wantSpec); err != nil { - t.Fatalf("not expected wantYaml: %q: \n%q", wantYaml, err) - } + assert.NoError(t, yaml.Unmarshal([]byte(wantYaml), &wantSpec)) wantYAMLForCompare, err := yaml.Marshal(wantSpec) - if err != nil { - t.Fatalf("BUG: cannot parse as yaml: %q", err) - } + assert.NoError(t, err) got := buildIngressConfig(cr) gotYAML, err := yaml.Marshal(got.Spec) - if err != nil { - t.Fatalf("cannot parse got as yaml: %q", err) - } + assert.NoError(t, err) assert.Equal(t, string(wantYAMLForCompare), string(gotYAML)) } diff --git a/internal/controller/operator/factory/vmcluster/vmcluster_test.go b/internal/controller/operator/factory/vmcluster/vmcluster_test.go index 34c3042d7..2a4a798bc 100644 --- a/internal/controller/operator/factory/vmcluster/vmcluster_test.go +++ b/internal/controller/operator/factory/vmcluster/vmcluster_test.go @@ -2,11 +2,8 @@ package vmcluster import ( "context" - "sync" "testing" - "time" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" appsv1 "k8s.io/api/apps/v1" @@ -21,6 +18,7 @@ import ( vpav1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" "github.com/VictoriaMetrics/operator/internal/config" @@ -47,106 +45,40 @@ func TestCreateOrUpdate(t *testing.T) { *config.MustGetBaseConfig() = defaultCfg }() } - fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - var wg sync.WaitGroup - eventuallyUpdateStatusToOk := func(cb func() error) { - wg.Add(1) - go func() { - defer wg.Done() - tc := time.NewTicker(time.Millisecond * 100) - for { - select { - case <-ctx.Done(): - return - case <-tc.C: - if err := cb(); err != nil { - if k8serrors.IsNotFound(err) { - continue - } - t.Errorf("callback error: %s", err) - return - } - return - } - } - }() - } - if o.cr.Spec.RequestsLoadBalancer.Enabled { - var vmauthLB appsv1.Deployment - name := o.cr.PrefixedName(vmv1beta1.ClusterComponentBalancer) - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: name, Namespace: o.cr.Namespace}, &vmauthLB); err != nil { - return err - } - vmauthLB.Status.Conditions = append(vmauthLB.Status.Conditions, appsv1.DeploymentCondition{ - Type: appsv1.DeploymentProgressing, - Reason: "NewReplicaSetAvailable", - Status: "True", - }) - if err := fclient.Status().Update(ctx, &vmauthLB); err != nil { - return err - } - - return nil - }) - - } - if o.cr.Spec.VMInsert != nil { - var vminsert appsv1.Deployment - name := o.cr.PrefixedName(vmv1beta1.ClusterComponentInsert) - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: name, Namespace: o.cr.Namespace}, &vminsert); err != nil { - return err - } - vminsert.Status.Conditions = append(vminsert.Status.Conditions, appsv1.DeploymentCondition{ - Type: appsv1.DeploymentProgressing, - Reason: "NewReplicaSetAvailable", - Status: "True", - }) - if err := fclient.Status().Update(ctx, &vminsert); err != nil { - return err - } - - return nil - }) - } - if o.cr.Spec.VMSelect != nil { - var vmselect appsv1.StatefulSet - name := o.cr.PrefixedName(vmv1beta1.ClusterComponentSelect) - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: name, Namespace: o.cr.Namespace}, &vmselect); err != nil { - return err + fclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if obj.GetNamespace() != o.cr.Namespace { + return nil } - vmselect.Status.ReadyReplicas = *o.cr.Spec.VMSelect.ReplicaCount - vmselect.Status.UpdatedReplicas = *o.cr.Spec.VMSelect.ReplicaCount - if err := fclient.Status().Update(ctx, &vmselect); err != nil { - return err - } - return nil - }) - } - if o.cr.Spec.VMStorage != nil { - var vmstorage appsv1.StatefulSet - eventuallyUpdateStatusToOk(func() error { - name := o.cr.PrefixedName(vmv1beta1.ClusterComponentStorage) - if err := fclient.Get(ctx, types.NamespacedName{Name: name, Namespace: o.cr.Namespace}, &vmstorage); err != nil { - return err - } - vmstorage.Status.ReadyReplicas = *o.cr.Spec.VMStorage.ReplicaCount - vmstorage.Status.UpdatedReplicas = *o.cr.Spec.VMStorage.ReplicaCount - if err := fclient.Status().Update(ctx, &vmstorage); err != nil { - return err + switch v := obj.(type) { + case *appsv1.StatefulSet: + switch { + case v.Name == o.cr.PrefixedName(vmv1beta1.ClusterComponentStorage): + v.Status.ReadyReplicas = *o.cr.Spec.VMStorage.ReplicaCount + v.Status.UpdatedReplicas = *o.cr.Spec.VMStorage.ReplicaCount + case v.Name == o.cr.PrefixedName(vmv1beta1.ClusterComponentSelect): + v.Status.ReadyReplicas = *o.cr.Spec.VMSelect.ReplicaCount + v.Status.UpdatedReplicas = *o.cr.Spec.VMSelect.ReplicaCount + } + case *appsv1.Deployment: + v.Status.Conditions = append(v.Status.Conditions, appsv1.DeploymentCondition{ + Type: appsv1.DeploymentProgressing, + Reason: "NewReplicaSetAvailable", + Status: "True", + }) + v.Status.UpdatedReplicas = *v.Spec.Replicas + v.Status.AvailableReplicas = v.Status.UpdatedReplicas } - + assert.NoError(t, cl.Create(ctx, obj)) return nil - }) - } + }, + }) + ctx := context.TODO() err := CreateOrUpdate(ctx, o.cr, fclient) - if (err != nil) != o.wantErr { - t.Errorf("CreateOrUpdate() error = %v, wantErr %v", err, o.wantErr) - return + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } if o.validate != nil { o.validate(ctx, fclient, o.cr) @@ -541,10 +473,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -603,10 +532,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -688,10 +614,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -778,19 +701,14 @@ func TestCreatOrUpdateClusterServices(t *testing.T) { case vmv1beta1.ClusterComponentSelect: builderF = createOrUpdateVMSelectService svc = buildVMSelectService(cr) - default: t.Fatalf("BUG not expected component for test: %q", component) } assert.NoError(t, builderF(ctx, fclient, cr, nil)) var actualService corev1.Service - if err := fclient.Get(ctx, types.NamespacedName{Namespace: svc.Namespace, Name: svc.Name}, &actualService); err != nil { - t.Fatalf("create service not found: %q", err) - } + assert.NoError(t, fclient.Get(ctx, types.NamespacedName{Namespace: svc.Namespace, Name: svc.Name}, &actualService)) var wantService corev1.Service - if err := yaml.Unmarshal([]byte(wantSvcYAML), &wantService); err != nil { - t.Fatalf("BUG: expect service definition at yaml: %q", err) - } + assert.NoError(t, yaml.Unmarshal([]byte(wantSvcYAML), &wantService)) assert.Equal(t, wantService, actualService) } diff --git a/internal/controller/operator/factory/vmdistributed/vmdistributed_test.go b/internal/controller/operator/factory/vmdistributed/vmdistributed_test.go index 2dbd95b53..5fa4bbc29 100644 --- a/internal/controller/operator/factory/vmdistributed/vmdistributed_test.go +++ b/internal/controller/operator/factory/vmdistributed/vmdistributed_test.go @@ -8,7 +8,10 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1alpha1 "github.com/VictoriaMetrics/operator/api/operator/v1alpha1" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" @@ -74,8 +77,9 @@ func newVMCluster(name, namespace, version string) *vmv1beta1.VMCluster { } type opts struct { - prepare func(*testData) - verify func(context.Context, *k8stools.TestClientWithStatsTrack, *testData) + prepare func(*testData) + validate func(context.Context, client.Client, *testData) + actions func(*testData) []action } type testData struct { @@ -138,13 +142,45 @@ func beforeEach(o opts) *testData { return d } +type action struct { + verb string + key string +} + func TestCreateOrUpdate(t *testing.T) { f := func(o opts) { t.Helper() d := beforeEach(o) - rclient := k8stools.GetTestClientWithObjects(d.predefinedObjects) + var actions []action + rclient := k8stools.GetTestClientWithObjectsAndInterceptors(d.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + actions = append(actions, action{ + verb: "Create", + key: fmt.Sprintf("%T/%s/%s", obj, obj.GetNamespace(), obj.GetName()), + }) + return cl.Create(ctx, obj, opts...) + }, + Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + actions = append(actions, action{ + verb: "Get", + key: fmt.Sprintf("%T/%s/%s", obj, key.Namespace, key.Name), + }) + return cl.Get(ctx, key, obj, opts...) + }, + Update: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.UpdateOption) error { + actions = append(actions, action{ + verb: "Update", + key: fmt.Sprintf("%T/%s/%s", obj, obj.GetNamespace(), obj.GetName()), + }) + return cl.Update(ctx, obj, opts...) + }, + }) ctx := context.Background() - o.verify(ctx, rclient, d) + o.validate(ctx, rclient, d) + if o.actions != nil { + expectedActions := o.actions(d) + assert.Equal(t, expectedActions, actions) + } } // paused CR should do nothing @@ -152,9 +188,8 @@ func TestCreateOrUpdate(t *testing.T) { prepare: func(d *testData) { d.cr.Spec.Paused = true }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + validate: func(ctx context.Context, rclient client.Client, d *testData) { assert.NoError(t, CreateOrUpdate(ctx, d.cr, rclient)) - assert.Empty(t, rclient.TotalCallsCount(nil)) }, }) @@ -169,7 +204,7 @@ func TestCreateOrUpdate(t *testing.T) { } } }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + validate: func(ctx context.Context, rclient client.Client, d *testData) { zs, err := getZones(ctx, rclient, d.cr) assert.NoError(t, err) @@ -183,7 +218,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }) - // verify remote write urls is appended to vmagent in a valid order + // validate remote write urls is appended to vmagent in a valid order f(opts{ prepare: func(d *testData) { for _, vmAgent := range d.zones.vmagents { @@ -193,7 +228,7 @@ func TestCreateOrUpdate(t *testing.T) { } } }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + validate: func(ctx context.Context, rclient client.Client, d *testData) { zs, err := getZones(ctx, rclient, d.cr) assert.NoError(t, err) @@ -215,22 +250,29 @@ func TestCreateOrUpdate(t *testing.T) { prepare: func(d *testData) { d.cr.Spec.VMAuth.Name = "" }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + actions: func(d *testData) []action { + return []action{ + { + verb: "Get", + key: fmt.Sprintf("*v1beta1.VMAuth/%s/%s", d.cr.Namespace, d.cr.Name), + }, + { + verb: "Create", + key: fmt.Sprintf("*v1beta1.VMAuth/%s/%s", d.cr.Namespace, d.cr.Name), + }, + { + verb: "Get", + key: fmt.Sprintf("*v1beta1.VMAuth/%s/%s", d.cr.Namespace, d.cr.Name), + }, + } + }, + validate: func(ctx context.Context, rclient client.Client, d *testData) { vmAuth := buildVMAuthLB(d.cr, d.zones.vmagents, d.zones.vmclusters) vmAuth.Status.StatusMetadata = vmv1beta1.StatusMetadata{ UpdateStatus: vmv1beta1.UpdateStatusOperational, } owner := d.cr.AsOwner() assert.NoError(t, reconcile.VMAuth(ctx, rclient, vmAuth, nil, &owner)) - assert.Equal(t, 3, rclient.TotalCallsCount(nil)) - // Verify the VMAuth was created with the default name (cr.Name) - createdItem := rclient.CreateCalls.First(&vmv1beta1.VMAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: d.cr.Name, - Namespace: d.cr.Namespace, - }, - }) - assert.NotNil(t, createdItem, "VMAuth should be created with default name") }, }) @@ -254,7 +296,23 @@ func TestCreateOrUpdate(t *testing.T) { }, }) }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + actions: func(d *testData) []action { + return []action{ + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Update", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + } + }, + validate: func(ctx context.Context, rclient client.Client, d *testData) { d.cr.Spec.VMAuth.Spec = vmv1beta1.VMAuthSpec{ LogLevel: "INFO", } @@ -266,17 +324,6 @@ func TestCreateOrUpdate(t *testing.T) { } owner := d.cr.AsOwner() assert.NoError(t, reconcile.VMAuth(ctx, rclient, vmAuth, nil, &owner)) - - // Check for update call - item := rclient.UpdateCalls.First(&vmv1beta1.VMAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: "vmauth-lb", - Namespace: "default", - }, - }) - assert.NotNil(t, item, "VMAuth should be updated") - updatedVMAuth := item.(*vmv1beta1.VMAuth) - assert.Equal(t, "INFO", updatedVMAuth.Spec.LogLevel) }, }) @@ -310,7 +357,19 @@ func TestCreateOrUpdate(t *testing.T) { } d.predefinedObjects = append(d.predefinedObjects, lb) }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + actions: func(d *testData) []action { + return []action{ + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + } + }, + validate: func(ctx context.Context, rclient client.Client, d *testData) { clusters := []*vmv1beta1.VMCluster{d.zones.vmclusters[0]} vmAuth := buildVMAuthLB(d.cr, d.zones.vmagents, clusters) vmAuth.Status.StatusMetadata = vmv1beta1.StatusMetadata{ @@ -319,14 +378,6 @@ func TestCreateOrUpdate(t *testing.T) { } owner := d.cr.AsOwner() assert.NoError(t, reconcile.VMAuth(ctx, rclient, vmAuth, nil, &owner)) - - // Should contain no updates - assert.Nil(t, rclient.UpdateCalls.First(&vmv1beta1.VMAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: "vmauth-lb", - Namespace: "default", - }, - })) }, }) @@ -341,49 +392,62 @@ func TestCreateOrUpdate(t *testing.T) { LogLevel: "INFO", } }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + actions: func(d *testData) []action { + return []action{ + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Create", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + } + }, + validate: func(ctx context.Context, rclient client.Client, d *testData) { vmAuth := buildVMAuthLB(d.cr, d.zones.vmagents, d.zones.vmclusters) vmAuth.Status.StatusMetadata = vmv1beta1.StatusMetadata{ UpdateStatus: vmv1beta1.UpdateStatusOperational, } owner := d.cr.AsOwner() assert.NoError(t, reconcile.VMAuth(ctx, rclient, vmAuth, nil, &owner)) - createdItem := rclient.CreateCalls.First(&vmv1beta1.VMAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: "vmauth-lb", - Namespace: "default", - }, - }) - assert.NotNil(t, createdItem) - createdVMAuth := createdItem.(*vmv1beta1.VMAuth) - assert.Equal(t, "vmauth-lb", createdVMAuth.Name) - assert.Equal(t, "default", createdVMAuth.Namespace) - assert.Equal(t, "INFO", createdVMAuth.Spec.LogLevel) - assert.NotNil(t, createdVMAuth.Spec.UnauthorizedUserAccessSpec) - assert.Len(t, createdVMAuth.Spec.UnauthorizedUserAccessSpec.TargetRefs, 6) - - for i := range createdVMAuth.Spec.UnauthorizedUserAccessSpec.TargetRefs { - targetRef := &createdVMAuth.Spec.UnauthorizedUserAccessSpec.TargetRefs[i] - assert.Equal(t, ptr.To("first_available"), targetRef.LoadBalancingPolicy) - assert.Equal(t, []int{500, 502, 503}, targetRef.RetryStatusCodes) - assert.NotNil(t, targetRef.CRD) + targetRefs := make([]vmv1beta1.TargetRef, 6) + for i := range targetRefs { + targetRef := &targetRefs[i] + targetRef.LoadBalancingPolicy = ptr.To("first_available") + targetRef.RetryStatusCodes = []int{500, 502, 503} if i < len(d.zones.vmagents) { - assert.Equal(t, d.zones.vmagents[i].Name, targetRef.CRD.Name) - assert.Equal(t, d.zones.vmagents[i].Namespace, targetRef.CRD.Namespace) - assert.Equal(t, []string{"/insert/.+", "/api/v1/write"}, targetRef.Paths) - assert.Equal(t, "VMAgent", targetRef.CRD.Kind) + targetRef.CRD = &vmv1beta1.CRDRef{ + Name: d.zones.vmagents[i].Name, + Namespace: d.zones.vmagents[i].Namespace, + Kind: "VMAgent", + } + targetRef.Paths = []string{"/insert/.+", "/api/v1/write"} } else { idx := i - len(d.zones.vmagents) - assert.Equal(t, d.zones.vmclusters[idx].Name, targetRef.CRD.Name) - assert.Equal(t, d.zones.vmclusters[idx].Namespace, targetRef.CRD.Namespace) - assert.Equal(t, []string{"/select/.+", "/admin/tenants"}, targetRef.Paths) - assert.Equal(t, "VMCluster/vmselect", targetRef.CRD.Kind) + targetRef.CRD = &vmv1beta1.CRDRef{ + Name: d.zones.vmclusters[idx].Name, + Namespace: d.zones.vmclusters[idx].Namespace, + Kind: "VMCluster/vmselect", + } + targetRef.Paths = []string{"/select/.+", "/admin/tenants"} } } - - // Verify OwnerReference - assert.NotEmpty(t, createdVMAuth.OwnerReferences) - assert.Equal(t, d.cr.Name, createdVMAuth.OwnerReferences[0].Name) + var got vmv1beta1.VMAuth + nsn := types.NamespacedName{ + Name: vmAuth.Name, + Namespace: vmAuth.Namespace, + } + assert.NoError(t, rclient.Get(ctx, nsn, &got)) + assert.Equal(t, targetRefs, got.Spec.UnauthorizedUserAccessSpec.TargetRefs) }, }) @@ -413,7 +477,27 @@ func TestCreateOrUpdate(t *testing.T) { }, }) }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + actions: func(d *testData) []action { + return []action{ + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Update", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + } + }, + validate: func(ctx context.Context, rclient client.Client, d *testData) { clusters := []*vmv1beta1.VMCluster{d.zones.vmclusters[0]} vmAuth := buildVMAuthLB(d.cr, d.zones.vmagents, clusters) vmAuth.Status.StatusMetadata = vmv1beta1.StatusMetadata{ @@ -422,16 +506,13 @@ func TestCreateOrUpdate(t *testing.T) { } owner := d.cr.AsOwner() assert.NoError(t, reconcile.VMAuth(ctx, rclient, vmAuth, nil, &owner)) - updatedVMAuth := rclient.UpdateCalls.First(&vmv1beta1.VMAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: "vmauth-lb", - Namespace: "default", - }, - }) - assert.NotNil(t, updatedVMAuth) - ownerReferences := updatedVMAuth.GetOwnerReferences() - assert.NotEmpty(t, ownerReferences) - assert.Equal(t, d.cr.Name, ownerReferences[0].Name) + var got vmv1beta1.VMAuth + nsn := types.NamespacedName{ + Name: vmAuth.Name, + Namespace: vmAuth.Namespace, + } + assert.NoError(t, rclient.Get(ctx, nsn, &got)) + assert.Equal(t, []metav1.OwnerReference{d.cr.AsOwner()}, got.OwnerReferences) }, }) @@ -447,43 +528,62 @@ func TestCreateOrUpdate(t *testing.T) { d.zones.vmclusters[0].Spec.RequestsLoadBalancer.Enabled = true d.zones.vmclusters[0].Spec.RequestsLoadBalancer.Spec.Port = "8427" }, - verify: func(ctx context.Context, rclient *k8stools.TestClientWithStatsTrack, d *testData) { + actions: func(d *testData) []action { + return []action{ + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Create", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + { + verb: "Get", + key: "*v1beta1.VMAuth/default/vmauth-lb", + }, + } + }, + validate: func(ctx context.Context, rclient client.Client, d *testData) { vmAuth := buildVMAuthLB(d.cr, d.zones.vmagents, d.zones.vmclusters) vmAuth.Status.StatusMetadata = vmv1beta1.StatusMetadata{ UpdateStatus: vmv1beta1.UpdateStatusOperational, } owner := d.cr.AsOwner() assert.NoError(t, reconcile.VMAuth(ctx, rclient, vmAuth, nil, &owner)) - createdItem := rclient.CreateCalls.First(&vmv1beta1.VMAuth{ - ObjectMeta: metav1.ObjectMeta{ - Name: "vmauth-lb", - Namespace: "default", - }, - }) - - assert.NotNil(t, createdItem) - createdVMAuth := createdItem.(*vmv1beta1.VMAuth) - assert.NotNil(t, createdVMAuth.Spec.UnauthorizedUserAccessSpec) - assert.Len(t, createdVMAuth.Spec.UnauthorizedUserAccessSpec.TargetRefs, 6) - - for i := range createdVMAuth.Spec.UnauthorizedUserAccessSpec.TargetRefs { - targetRef := &createdVMAuth.Spec.UnauthorizedUserAccessSpec.TargetRefs[i] - assert.Equal(t, ptr.To("first_available"), targetRef.LoadBalancingPolicy) - assert.Equal(t, []int{500, 502, 503}, targetRef.RetryStatusCodes) - assert.NotNil(t, targetRef.CRD) + targetRefs := make([]vmv1beta1.TargetRef, 6) + for i := range targetRefs { + targetRef := &targetRefs[i] + targetRef.LoadBalancingPolicy = ptr.To("first_available") + targetRef.RetryStatusCodes = []int{500, 502, 503} if i < len(d.zones.vmagents) { - assert.Equal(t, d.zones.vmagents[i].Name, targetRef.CRD.Name) - assert.Equal(t, d.zones.vmagents[i].Namespace, targetRef.CRD.Namespace) - assert.Equal(t, []string{"/insert/.+", "/api/v1/write"}, targetRef.Paths) - assert.Equal(t, "VMAgent", targetRef.CRD.Kind) + targetRef.CRD = &vmv1beta1.CRDRef{ + Name: d.zones.vmagents[i].Name, + Namespace: d.zones.vmagents[i].Namespace, + Kind: "VMAgent", + } + targetRef.Paths = []string{"/insert/.+", "/api/v1/write"} } else { idx := i - len(d.zones.vmagents) - assert.Equal(t, d.zones.vmclusters[idx].Name, targetRef.CRD.Name) - assert.Equal(t, d.zones.vmclusters[idx].Namespace, targetRef.CRD.Namespace) - assert.Equal(t, []string{"/select/.+", "/admin/tenants"}, targetRef.Paths) - assert.Equal(t, "VMCluster/vmselect", targetRef.CRD.Kind) + targetRef.CRD = &vmv1beta1.CRDRef{ + Name: d.zones.vmclusters[idx].Name, + Namespace: d.zones.vmclusters[idx].Namespace, + Kind: "VMCluster/vmselect", + } + targetRef.Paths = []string{"/select/.+", "/admin/tenants"} } } + var got vmv1beta1.VMAuth + nsn := types.NamespacedName{ + Name: vmAuth.Name, + Namespace: vmAuth.Namespace, + } + assert.NoError(t, rclient.Get(ctx, nsn, &got)) + assert.Equal(t, targetRefs, got.Spec.UnauthorizedUserAccessSpec.TargetRefs) }, }) } diff --git a/internal/controller/operator/factory/vtcluster/cluster_test.go b/internal/controller/operator/factory/vtcluster/cluster_test.go index 69db334d0..f7d8f09d0 100644 --- a/internal/controller/operator/factory/vtcluster/cluster_test.go +++ b/internal/controller/operator/factory/vtcluster/cluster_test.go @@ -2,11 +2,8 @@ package vtcluster import ( "context" - "sync" "testing" - "time" - "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" @@ -19,6 +16,7 @@ import ( vpav1 "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" vmv1 "github.com/VictoriaMetrics/operator/api/operator/v1" vmv1beta1 "github.com/VictoriaMetrics/operator/api/operator/v1beta1" @@ -45,114 +43,38 @@ func TestCreateOrUpdate(t *testing.T) { *config.MustGetBaseConfig() = defaultCfg }() } - fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) - build.AddDefaults(fclient.Scheme()) - fclient.Scheme().Default(o.cr) - ctx, cancel := context.WithCancel(context.Background()) - var wg sync.WaitGroup - defer func() { - cancel() - wg.Wait() - }() - eventuallyUpdateStatusToOk := func(cb func() error) { - wg.Add(1) - go func() { - defer wg.Done() - tc := time.NewTicker(time.Millisecond * 100) - for { - select { - case <-ctx.Done(): - return - case <-tc.C: - if err := cb(); err != nil { - if k8serrors.IsNotFound(err) { - continue - } - t.Errorf("callback error: %s", err) - return - } - return - } - } - }() - } - if o.cr.Spec.Storage != nil { - var vtst appsv1.StatefulSet - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: o.cr.PrefixedName(vmv1beta1.ClusterComponentStorage), Namespace: o.cr.Namespace}, &vtst); err != nil { - return err - } - vtst.Status.ReadyReplicas = *o.cr.Spec.Storage.ReplicaCount - vtst.Status.UpdatedReplicas = *o.cr.Spec.Storage.ReplicaCount - if err := fclient.Status().Update(ctx, &vtst); err != nil { - return err - } - - return nil - }) - } - if o.cr.Spec.Select != nil { - var vts appsv1.Deployment - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: o.cr.PrefixedName(vmv1beta1.ClusterComponentSelect), Namespace: o.cr.Namespace}, &vts); err != nil { - return err - } - vts.Status.Conditions = append(vts.Status.Conditions, appsv1.DeploymentCondition{ - Type: appsv1.DeploymentProgressing, - Reason: "NewReplicaSetAvailable", - Status: "True", - }) - vts.Status.UpdatedReplicas = *vts.Spec.Replicas - vts.Status.AvailableReplicas = vts.Status.UpdatedReplicas - if err := fclient.Status().Update(ctx, &vts); err != nil { - return err - } - - return nil - }) - - } - if o.cr.Spec.Insert != nil { - var vti appsv1.Deployment - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: o.cr.PrefixedName(vmv1beta1.ClusterComponentInsert), Namespace: o.cr.Namespace}, &vti); err != nil { - return err + fclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, _ ...client.CreateOption) error { + if obj.GetNamespace() != o.cr.Namespace { + return nil } - vti.Status.Conditions = append(vti.Status.Conditions, appsv1.DeploymentCondition{ - Type: appsv1.DeploymentProgressing, - Reason: "NewReplicaSetAvailable", - Status: "True", - }) - vti.Status.UpdatedReplicas = *vti.Spec.Replicas - vti.Status.AvailableReplicas = vti.Status.UpdatedReplicas - if err := fclient.Status().Update(ctx, &vti); err != nil { - return err - } - return nil - }) - - } - if o.cr.Spec.RequestsLoadBalancer.Enabled { - var vmauthLB appsv1.Deployment - eventuallyUpdateStatusToOk(func() error { - if err := fclient.Get(ctx, types.NamespacedName{Name: o.cr.PrefixedName(vmv1beta1.ClusterComponentBalancer), Namespace: o.cr.Namespace}, &vmauthLB); err != nil { - return err - } - vmauthLB.Status.Conditions = append(vmauthLB.Status.Conditions, appsv1.DeploymentCondition{ - Type: appsv1.DeploymentProgressing, - Reason: "NewReplicaSetAvailable", - Status: "True", - }) - if err := fclient.Status().Update(ctx, &vmauthLB); err != nil { - return err + switch v := obj.(type) { + case *appsv1.StatefulSet: + if v.Name == o.cr.PrefixedName(vmv1beta1.ClusterComponentStorage) { + v.Status.ReadyReplicas = *o.cr.Spec.Storage.ReplicaCount + v.Status.UpdatedReplicas = *o.cr.Spec.Storage.ReplicaCount + } + case *appsv1.Deployment: + v.Status.Conditions = append(v.Status.Conditions, appsv1.DeploymentCondition{ + Type: appsv1.DeploymentProgressing, + Reason: "NewReplicaSetAvailable", + Status: "True", + }) + v.Status.UpdatedReplicas = *v.Spec.Replicas + v.Status.AvailableReplicas = v.Status.UpdatedReplicas } - + assert.NoError(t, cl.Create(ctx, obj)) return nil - }) - } + }, + }) + build.AddDefaults(fclient.Scheme()) + fclient.Scheme().Default(o.cr) + ctx := context.TODO() err := CreateOrUpdate(ctx, fclient, o.cr) - if (err != nil) != o.wantErr { - t.Fatalf("unexpected error: %s", err) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } if o.validate != nil { o.validate(ctx, fclient, o.cr) @@ -338,10 +260,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -410,10 +329,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -472,10 +388,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) @@ -557,10 +470,7 @@ func TestCreateOrUpdate(t *testing.T) { }, }, } - if !cmp.Equal(got, expected) { - diff := cmp.Diff(got, expected) - t.Fatal("not expected output with diff: ", diff) - } + assert.Equal(t, got, expected) }, }) From 6eae380d9e553dc8990348f2ad1190b29e074b2f Mon Sep 17 00:00:00 2001 From: Andrii Chubatiuk Date: Wed, 18 Feb 2026 18:41:20 +0200 Subject: [PATCH 2/2] reconcile: perform pods deletion instead of eviction with maxUnavailable set to 100% --- docs/CHANGELOG.md | 2 +- .../factory/reconcile/reconcile_test.go | 19 +- .../operator/factory/reconcile/statefulset.go | 76 ++-- .../factory/reconcile/statefulset_test.go | 359 ++++++++++++++++-- test/e2e/vmcluster_test.go | 2 +- 5 files changed, 392 insertions(+), 66 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 9b9e75316..8f28c4439 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -28,6 +28,7 @@ aliases: * FEATURE: [vmprobe](https://docs.victoriametrics.com/operator/resources/vmprobe/): added `spec.targets.kubernetes` property, that allows to configure probe for `ingress`, `pod` and `service` roles. See [#1078](https://github.com/VictoriaMetrics/operator/issues/1078) and [#1716](https://github.com/VictoriaMetrics/operator/issues/1716). * FEATURE: [vmscrapeconfig](https://docs.victoriametrics.com/operator/resources/vmscrapeconfig/): added nomad_sd_config support. See [#1809](https://github.com/VictoriaMetrics/operator/issues/1809). * FEATURE: [vmoperator](https://docs.victoriametrics.com/operator/): support VPA for vmcluster, vtcluster, vlcluster and vmauth. See [#1795](https://github.com/VictoriaMetrics/operator/issues/1795). Thanks to the @dctrwatson for the pull request [#1803](https://github.com/VictoriaMetrics/operator/pull/1803). +* FEATURE: [vmoperator](https://docs.victoriametrics.com/operator/): perform statefulset pods deletion instead of eviction when maxUnavailable set to 100%, which is important for [minimum downtime strategy](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/#minimum-downtime-strategy). See [#1706](https://github.com/VictoriaMetrics/operator/issues/1706). * BUGFIX: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/): previously the operator requested `nodes/proxy` RBAC permissions even though vmagent did not use them; now this permission is no longer required, reducing the default privilege footprint for users running vmagent. See [#1753](https://github.com/VictoriaMetrics/operator/issues/1753). * BUGFIX: [vmalert](https://docs.victoriametrics.com/operator/resources/vmalert/): throw error if no notifiers found. See [#1757](https://github.com/VictoriaMetrics/operator/issues/1757). @@ -43,7 +44,6 @@ aliases: * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): previously, recreating a resource after deletion could hang and block updates; now resource recreation completes normally. See [#1707](https://github.com/VictoriaMetrics/operator/issues/1707). * BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): use global image registry unless image.repository is defined. See [#1813](https://github.com/VictoriaMetrics/operator/issues/1813). * BUGFIX: [vmalertmanagerconfig](https://docs.victoriametrics.com/operator/resources/vmalertmanagerconfig/): previously spec.route and spec.receivers were required; now both parameters are optional to align with prometheus operator. VMAlertmanager now can be used to set just the global inhibition rules. See [#1800](https://github.com/VictoriaMetrics/operator/issues/1800). -* BUGFIX: [vmoperator](https://docs.victoriametrics.com/operator/): use global image registry unless image.repository is defined. See [#1813](https://github.com/VictoriaMetrics/operator/issues/1813). * BUGFIX: [vmagent](https://docs.victoriametrics.com/operator/resources/vmagent/): fixed RBAC, when ingestOnlyMode is enabled and relabel of stream aggregation configurations defined. See [#1828](https://github.com/VictoriaMetrics/operator/issues/1828). ## [v0.67.0](https://github.com/VictoriaMetrics/operator/releases/tag/v0.67.0) diff --git a/internal/controller/operator/factory/reconcile/reconcile_test.go b/internal/controller/operator/factory/reconcile/reconcile_test.go index 3b17c8b69..810773488 100644 --- a/internal/controller/operator/factory/reconcile/reconcile_test.go +++ b/internal/controller/operator/factory/reconcile/reconcile_test.go @@ -3,6 +3,7 @@ package reconcile import ( "context" "testing" + "testing/synctest" "time" "github.com/stretchr/testify/assert" @@ -31,14 +32,16 @@ func TestWaitForStatus(t *testing.T) { }, } rclient := k8stools.GetTestClientWithObjects([]runtime.Object{vmc}) - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - err := waitForStatus(ctx, rclient, vmc.DeepCopy(), 1*time.Second, vmv1beta1.UpdateStatusOperational) - if isErr { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + synctest.Test(t, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + err := waitForStatus(ctx, rclient, vmc.DeepCopy(), 1*time.Second, vmv1beta1.UpdateStatusOperational) + if isErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) } // success diff --git a/internal/controller/operator/factory/reconcile/statefulset.go b/internal/controller/operator/factory/reconcile/statefulset.go index d3263c608..446f10a94 100644 --- a/internal/controller/operator/factory/reconcile/statefulset.go +++ b/internal/controller/operator/factory/reconcile/statefulset.go @@ -156,14 +156,21 @@ func StatefulSet(ctx context.Context, rclient client.Client, cr STSOptions, newO // perform manual update only with OnDelete policy, which is default. switch updateStrategy { case appsv1.OnDeleteStatefulSetStrategyType: - podMaxUnavailable := 1 + opts := rollingUpdateOpts{ + recreate: mustRecreatePod, + selector: cr.SelectorLabels(), + maxUnavailable: 1, + } if cr.UpdateBehavior != nil { - podMaxUnavailable, err = intstr.GetScaledValueFromIntOrPercent(cr.UpdateBehavior.MaxUnavailable, int(*newObj.Spec.Replicas), false) + if cr.UpdateBehavior.MaxUnavailable.String() == "100%" { + opts.delete = true + } + opts.maxUnavailable, err = intstr.GetScaledValueFromIntOrPercent(cr.UpdateBehavior.MaxUnavailable, int(*newObj.Spec.Replicas), false) if err != nil { return err } } - if err := performRollingUpdateOnSts(ctx, mustRecreatePod, rclient, newObj.Name, newObj.Namespace, cr.SelectorLabels(), podMaxUnavailable); err != nil { + if err := performRollingUpdateOnSts(ctx, rclient, newObj, opts); err != nil { return fmt.Errorf("cannot handle rolling-update on StatefulSet=%s: %w", nsn, err) } return nil @@ -198,6 +205,13 @@ func getLatestStsState(ctx context.Context, rclient client.Client, targetSTS typ return &sts, nil } +type rollingUpdateOpts struct { + recreate bool + maxUnavailable int + selector map[string]string + delete bool +} + // we perform rolling update on sts by manually evicting pods one by one or in batches // we check sts revision (kubernetes controller-manager is responsible for that) // and compare pods revision label with sts revision @@ -205,9 +219,13 @@ func getLatestStsState(ctx context.Context, rclient client.Client, targetSTS typ // // we always check if sts.Status.CurrentRevision needs update, to keep it equal to UpdateRevision // see https://github.com/kubernetes/kube-state-metrics/issues/1324#issuecomment-1779751992 -func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclient client.Client, stsName string, ns string, podLabels map[string]string, podMaxUnavailable int) error { +func performRollingUpdateOnSts(ctx context.Context, rclient client.Client, obj *appsv1.StatefulSet, o rollingUpdateOpts) error { time.Sleep(podWaitReadyIntervalCheck) - sts, err := getLatestStsState(ctx, rclient, types.NamespacedName{Name: stsName, Namespace: ns}) + nsn := types.NamespacedName{ + Name: obj.Name, + Namespace: obj.Namespace, + } + sts, err := getLatestStsState(ctx, rclient, nsn) if err != nil { return err } @@ -226,11 +244,11 @@ func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclien l.Info("sts has 0 replicas configured, nothing to update") return nil } - l.Info(fmt.Sprintf("check if pod update needed to desiredVersion=%s, podMustRecreate=%v", stsVersion, podMustRecreate)) + l.Info(fmt.Sprintf("check if pod update needed to desiredVersion=%s, podMustRecreate=%v", stsVersion, o.recreate)) var podList corev1.PodList opts := &client.ListOptions{ - Namespace: ns, - LabelSelector: labels.SelectorFromSet(podLabels), + Namespace: obj.Namespace, + LabelSelector: labels.SelectorFromSet(o.selector), } if err := rclient.List(ctx, &podList, opts); err != nil { return fmt.Errorf("cannot list pods for statefulset rolling update: %w", err) @@ -238,7 +256,7 @@ func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclien if err := sortStsPodsByID(podList.Items); err != nil { return fmt.Errorf("cannot sort statefulset pods: %w", err) } - readyPods, updatedPods, podsForUpdate := filterSTSPods(podList.Items, stsVersion, sts.Spec.MinReadySeconds, podMustRecreate) + readyPods, updatedPods, podsForUpdate := filterSTSPods(podList.Items, stsVersion, sts.Spec.MinReadySeconds, o.recreate) totalPodsCount := len(readyPods) + len(updatedPods) + len(podsForUpdate) switch { @@ -265,18 +283,18 @@ func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclien // check updated, by not ready pods for _, pod := range updatedPods { l.Info(fmt.Sprintf("checking ready status for already updated pod %s to revision version=%q", pod.Name, stsVersion)) - podNsn := types.NamespacedName{Namespace: ns, Name: pod.Name} + podNsn := types.NamespacedName{Namespace: obj.Namespace, Name: pod.Name} if err := waitForPodReady(ctx, rclient, podNsn, stsVersion, sts.Spec.MinReadySeconds); err != nil { return fmt.Errorf("cannot wait for pod ready state for already updated pod: %w", err) } } // perform update for not updated pods in batches according to podMaxUnavailable - for batchStart := 0; batchStart < len(podsForUpdate); batchStart += podMaxUnavailable { + for batchStart := 0; batchStart < len(podsForUpdate); batchStart += o.maxUnavailable { var batch []corev1.Pod // determine batch of pods to update - batchClose := batchStart + podMaxUnavailable + batchClose := batchStart + o.maxUnavailable if batchClose > len(podsForUpdate) { batchClose = len(podsForUpdate) } @@ -286,19 +304,27 @@ func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclien for _, pod := range batch { errG.Go(func() error { l.Info(fmt.Sprintf("updating pod=%s revision label=%q", pod.Name, pod.Labels[podRevisionLabel])) - // eviction may fail due to podDisruption budget and it's unexpected // so retry pod eviction evictErr := wait.PollUntilContextTimeout(ctx, podWaitReadyIntervalCheck, podWaitReadyTimeout, true, func(ctx context.Context) (done bool, err error) { - // evict pod to trigger re-creation - podEviction := policyv1.Eviction{ObjectMeta: pod.ObjectMeta} - if err := rclient.SubResource("eviction").Create(ctx, &pod, &podEviction); err != nil { - // retry distruption interrupt error: - // https://github.com/kubernetes/kubernetes/blob/9a50e306361ea936e57fb6eb8c635f971e7bb707/pkg/registry/core/pod/storage/eviction.go#L418 - if strings.Contains(err.Error(), "Cannot evict pod as it would violate the pod's disruption budget") { - return false, nil + if o.delete { + if err := rclient.Delete(ctx, &pod); err != nil { + if k8serrors.IsNotFound(err) { + return true, nil + } + return false, fmt.Errorf("failed to delete pod %s: %w", pod.Name, err) + } + } else { + // evict pod to trigger re-creation + podEviction := policyv1.Eviction{ObjectMeta: pod.ObjectMeta} + if err := rclient.SubResource("eviction").Create(ctx, &pod, &podEviction); err != nil { + // retry distruption interrupt error: + // https://github.com/kubernetes/kubernetes/blob/9a50e306361ea936e57fb6eb8c635f971e7bb707/pkg/registry/core/pod/storage/eviction.go#L418 + if strings.Contains(err.Error(), "Cannot evict pod as it would violate the pod's disruption budget") { + return false, nil + } + return false, fmt.Errorf("cannot evict pod %s: %w", pod.Name, err) } - return false, fmt.Errorf("cannot evict pod %s: %w", pod.Name, err) } return true, nil }) @@ -306,7 +332,7 @@ func performRollingUpdateOnSts(ctx context.Context, podMustRecreate bool, rclien return fmt.Errorf("cannot perform pod eviction: %w", evictErr) } // wait for pod to be re-created - podNsn := types.NamespacedName{Namespace: ns, Name: pod.Name} + podNsn := types.NamespacedName{Namespace: obj.Namespace, Name: pod.Name} if err := waitForPodReady(ctx, rclient, podNsn, stsVersion, sts.Spec.MinReadySeconds); err != nil { return fmt.Errorf("cannot wait for pod ready state during re-creation for pod %s: %w", pod.Name, err) } @@ -342,7 +368,7 @@ func PodIsReady(pod *corev1.Pod, minReadySeconds int32) bool { func waitForPodReady(ctx context.Context, rclient client.Client, nsn types.NamespacedName, desiredRevision string, minReadySeconds int32) error { var pod corev1.Pod - if err := wait.PollUntilContextTimeout(ctx, podWaitReadyIntervalCheck, podWaitReadyTimeout, true, func(_ context.Context) (done bool, err error) { + if err := wait.PollUntilContextTimeout(ctx, podWaitReadyIntervalCheck, podWaitReadyTimeout, true, func(ctx context.Context) (done bool, err error) { if err := rclient.Get(ctx, nsn, &pod); err != nil { if k8serrors.IsNotFound(err) { return false, nil @@ -443,7 +469,7 @@ func sortStsPodsByID(src []corev1.Pod) error { return firstParseError } -func isSTSPod(pod *corev1.Pod) bool { +func isOwnedBySTS(pod *corev1.Pod) bool { for _, ref := range pod.OwnerReferences { if ref.Kind == "StatefulSet" { return true @@ -458,7 +484,7 @@ func filterSTSPods(pods []corev1.Pod, revision string, minReadySeconds int32, re // pod could be owned by Deployment due to Deployment -> StatefulSet transition isSameRevision := pod.Labels[podRevisionLabel] == revision switch { - case !isSTSPod(&pod): + case !isOwnedBySTS(&pod): case !pod.DeletionTimestamp.IsZero() || recreate: podsForUpdate = append(podsForUpdate, pod) case PodIsReady(&pod, minReadySeconds) && isSameRevision: diff --git a/internal/controller/operator/factory/reconcile/statefulset_test.go b/internal/controller/operator/factory/reconcile/statefulset_test.go index 8c69ba357..2309114e1 100644 --- a/internal/controller/operator/factory/reconcile/statefulset_test.go +++ b/internal/controller/operator/factory/reconcile/statefulset_test.go @@ -2,12 +2,15 @@ package reconcile import ( "context" + "sync" "testing" + "testing/synctest" "time" "github.com/stretchr/testify/assert" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -21,26 +24,31 @@ import ( func Test_waitForPodReady(t *testing.T) { type opts struct { - ns string - podName string + nsn types.NamespacedName desiredVersion string wantErr bool predefinedObjects []runtime.Object } f := func(o opts) { t.Helper() + ctx := context.Background() fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) - - nsn := types.NamespacedName{Namespace: o.ns, Name: o.podName} - if err := waitForPodReady(context.Background(), fclient, nsn, o.desiredVersion, 0); (err != nil) != o.wantErr { - t.Errorf("waitForPodReady() error = %v, wantErr %v", err, o.wantErr) - } + synctest.Test(t, func(t *testing.T) { + err := waitForPodReady(ctx, fclient, o.nsn, o.desiredVersion, 0) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) } // testing pod with unready status f(opts{ - ns: "default", - podName: "vmselect-example-0", + nsn: types.NamespacedName{ + Namespace: "default", + Name: "vmselect-example-0", + }, predefinedObjects: []runtime.Object{ &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -68,8 +76,10 @@ func Test_waitForPodReady(t *testing.T) { // testing pod with ready status f(opts{ - ns: "default", - podName: "vmselect-example-0", + nsn: types.NamespacedName{ + Namespace: "default", + Name: "vmselect-example-0", + }, predefinedObjects: []runtime.Object{ &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -98,8 +108,10 @@ func Test_waitForPodReady(t *testing.T) { // with desiredVersion f(opts{ - ns: "default", - podName: "vmselect-example-0", + nsn: types.NamespacedName{ + Namespace: "default", + Name: "vmselect-example-0", + }, desiredVersion: "some-version", predefinedObjects: []runtime.Object{ &corev1.Pod{ @@ -132,8 +144,10 @@ func Test_waitForPodReady(t *testing.T) { // with missing desiredVersion f(opts{ - ns: "default", - podName: "vmselect-example-0", + nsn: types.NamespacedName{ + Namespace: "default", + Name: "vmselect-example-0", + }, desiredVersion: "some-version", predefinedObjects: []runtime.Object{ &corev1.Pod{ @@ -263,28 +277,78 @@ func Test_podIsReady(t *testing.T) { func Test_performRollingUpdateOnSts(t *testing.T) { type opts struct { - stsName string - ns string - podLabels map[string]string - podMaxUnavailable int + sts *appsv1.StatefulSet + opts rollingUpdateOpts wantErr bool predefinedObjects []runtime.Object + actions []string } f := func(o opts) { t.Helper() - fclient := k8stools.GetTestClientWithObjects(o.predefinedObjects) - - if err := performRollingUpdateOnSts(context.Background(), false, fclient, o.stsName, o.ns, o.podLabels, o.podMaxUnavailable); (err != nil) != o.wantErr { - t.Errorf("performRollingUpdateOnSts() error = %v, wantErr %v", err, o.wantErr) + ctx := context.Background() + var mu sync.Mutex + var actions []string + fclient := k8stools.GetTestClientWithObjectsAndInterceptors(o.predefinedObjects, interceptor.Funcs{ + SubResourceCreate: func(ctx context.Context, cl client.Client, _ string, obj client.Object, subResource client.Object, _ ...client.SubResourceCreateOption) error { + pod, podOk := obj.(*corev1.Pod) + _, evictionOk := subResource.(*policyv1.Eviction) + if !evictionOk || !podOk { + return nil + } + mu.Lock() + actions = append(actions, "Evict") + mu.Unlock() + pod.Labels[podRevisionLabel] = o.sts.Status.UpdateRevision + return cl.Update(ctx, pod) + }, + Delete: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.DeleteOption) error { + assert.NoError(t, cl.Delete(ctx, obj, opts...)) + pod, podOk := obj.(*corev1.Pod) + if !podOk { + return nil + } + mu.Lock() + actions = append(actions, "Delete") + mu.Unlock() + pod.Labels[podRevisionLabel] = o.sts.Status.UpdateRevision + pod.ResourceVersion = "" + return cl.Create(ctx, pod) + }, + Get: func(ctx context.Context, cl client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + _, podOk := obj.(*corev1.Pod) + if podOk { + mu.Lock() + actions = append(actions, "Get") + mu.Unlock() + } + return cl.Get(ctx, key, obj) + }, + }) + err := performRollingUpdateOnSts(ctx, fclient, o.sts, o.opts) + if o.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } + assert.ElementsMatch(t, actions, o.actions) } // rolling update is not needed f(opts{ - stsName: "vmselect-sts", - ns: "default", - podLabels: map[string]string{"app": "vmselect"}, - podMaxUnavailable: 1, + sts: &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts", + Namespace: "default", + }, + Status: appsv1.StatefulSetStatus{ + CurrentRevision: "rev1", + UpdateRevision: "rev1", + }, + }, + opts: rollingUpdateOpts{ + selector: map[string]string{"app": "vmselect"}, + maxUnavailable: 1, + }, predefinedObjects: []runtime.Object{ &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ @@ -316,12 +380,245 @@ func Test_performRollingUpdateOnSts(t *testing.T) { }, }) + // evict pods + f(opts{ + sts: &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts", + Namespace: "default", + }, + Status: appsv1.StatefulSetStatus{ + CurrentRevision: "rev1", + UpdateRevision: "rev1", + }, + }, + opts: rollingUpdateOpts{ + selector: map[string]string{"app": "vmselect"}, + maxUnavailable: 4, + }, + predefinedObjects: []runtime.Object{ + &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts", + Namespace: "default", + Labels: map[string]string{"app": "vmselect"}, + }, + Spec: appsv1.StatefulSetSpec{ + Replicas: ptr.To(int32(4)), + }, + Status: appsv1.StatefulSetStatus{ + CurrentRevision: "rev1", + UpdateRevision: "rev1", + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts-0", + Namespace: "default", + Labels: map[string]string{"app": "vmselect", podRevisionLabel: "rev0"}, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "StatefulSet", + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: "True", + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts-1", + Namespace: "default", + Labels: map[string]string{"app": "vmselect", podRevisionLabel: "rev0"}, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "StatefulSet", + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: "True", + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts-2", + Namespace: "default", + Labels: map[string]string{"app": "vmselect", podRevisionLabel: "rev0"}, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "StatefulSet", + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: "True", + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts-3", + Namespace: "default", + Labels: map[string]string{"app": "vmselect", podRevisionLabel: "rev1"}, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "StatefulSet", + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: "True", + }, + }, + }, + }, + }, + actions: []string{"Evict", "Evict", "Evict", "Get", "Get", "Get"}, + }) + + // recreating pods + f(opts{ + sts: &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts", + Namespace: "default", + }, + Status: appsv1.StatefulSetStatus{ + CurrentRevision: "rev1", + UpdateRevision: "rev1", + }, + }, + opts: rollingUpdateOpts{ + selector: map[string]string{"app": "vmselect"}, + maxUnavailable: 4, + delete: true, + }, + predefinedObjects: []runtime.Object{ + &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts", + Namespace: "default", + Labels: map[string]string{"app": "vmselect"}, + }, + Spec: appsv1.StatefulSetSpec{ + Replicas: ptr.To(int32(4)), + }, + Status: appsv1.StatefulSetStatus{ + CurrentRevision: "rev1", + UpdateRevision: "rev1", + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts-0", + Namespace: "default", + Labels: map[string]string{"app": "vmselect", podRevisionLabel: "rev0"}, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "StatefulSet", + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: "True", + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts-1", + Namespace: "default", + Labels: map[string]string{"app": "vmselect", podRevisionLabel: "rev0"}, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "StatefulSet", + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: "True", + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts-2", + Namespace: "default", + Labels: map[string]string{"app": "vmselect", podRevisionLabel: "rev0"}, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "StatefulSet", + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: "True", + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts-3", + Namespace: "default", + Labels: map[string]string{"app": "vmselect", podRevisionLabel: "rev1"}, + OwnerReferences: []metav1.OwnerReference{{ + Kind: "StatefulSet", + }}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + Conditions: []corev1.PodCondition{ + { + Type: corev1.PodReady, + Status: "True", + }, + }, + }, + }, + }, + actions: []string{"Delete", "Get", "Delete", "Get", "Delete", "Get"}, + }) + // rolling update is timeout f(opts{ - stsName: "vmselect-sts", - ns: "default", - podLabels: map[string]string{"app": "vmselect"}, - podMaxUnavailable: 1, + sts: &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "vmselect-sts", + Namespace: "default", + }, + Status: appsv1.StatefulSetStatus{ + CurrentRevision: "rev1", + UpdateRevision: "rev1", + }, + }, + opts: rollingUpdateOpts{ + selector: map[string]string{"app": "vmselect"}, + maxUnavailable: 1, + }, predefinedObjects: []runtime.Object{ &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ diff --git a/test/e2e/vmcluster_test.go b/test/e2e/vmcluster_test.go index 7826db88c..083e49c8a 100644 --- a/test/e2e/vmcluster_test.go +++ b/test/e2e/vmcluster_test.go @@ -1471,7 +1471,7 @@ up{baz="bar"} 123 }, }, ), - Entry("configures vmstorage with MaxUnavailable 100% but limited by a PDB", "maxunavailable-100-percent-pdb", + Entry("configures vmstorage with MaxUnavailable 100% and ignored PDB limitation", "maxunavailable-100-percent-pdb", &vmv1beta1.VMCluster{ Spec: vmv1beta1.VMClusterSpec{ RetentionPeriod: "1",