From 7df9ba19ffe5464aa212115162410ad70b2db1e8 Mon Sep 17 00:00:00 2001 From: Lucas Caparelli Date: Thu, 29 Aug 2024 18:39:16 -0300 Subject: [PATCH] fix: don't run starter job if TestRun/K6 is paused --- api/v1alpha1/k6_types.go | 16 ++++ api/v1alpha1/testrun_types.go | 7 +- api/v1alpha1/testruni.go | 3 +- controllers/suite_test.go | 18 +++- controllers/testrun_controller.go | 9 +- controllers/testrun_controller_test.go | 125 +++++++++++++++++++++++++ go.mod | 4 +- go.sum | 1 + pkg/resources/jobs/runner.go | 15 +-- pkg/segmentation/suite_test.go | 5 +- 10 files changed, 181 insertions(+), 22 deletions(-) create mode 100644 controllers/testrun_controller_test.go diff --git a/api/v1alpha1/k6_types.go b/api/v1alpha1/k6_types.go index 8fe5a787..be6b368c 100644 --- a/api/v1alpha1/k6_types.go +++ b/api/v1alpha1/k6_types.go @@ -15,6 +15,8 @@ limitations under the License. package v1alpha1 import ( + "strconv" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" @@ -91,6 +93,15 @@ type TestRunSpec struct { Token string `json:"token,omitempty"` // PLZ reserved field (for now) } +func (k6 TestRunSpec) isPaused() bool { + if k6.Paused == "" { + return true + } + + paused, _ := strconv.ParseBool(k6.Paused) + return paused +} + // K6Script describes where the script to execute the tests is found type K6Script struct { VolumeClaim K6VolumeClaim `json:"volumeClaim,omitempty"` @@ -146,6 +157,11 @@ type K6 struct { Status TestRunStatus `json:"status,omitempty"` } +// IsPaused returns whether this K6 is paused or not +func (in *K6) IsPaused() bool { + return in.Spec.isPaused() +} + // K6List contains a list of K6 // +kubebuilder:object:root=true type K6List struct { diff --git a/api/v1alpha1/testrun_types.go b/api/v1alpha1/testrun_types.go index ee2a8324..228675c2 100644 --- a/api/v1alpha1/testrun_types.go +++ b/api/v1alpha1/testrun_types.go @@ -20,9 +20,10 @@ import ( "errors" "path/filepath" - "github.com/grafana/k6-operator/pkg/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" + + "github.com/grafana/k6-operator/pkg/types" ) //+kubebuilder:object:root=true @@ -53,6 +54,10 @@ func init() { SchemeBuilder.Register(&TestRun{}, &TestRunList{}) } +func (in *TestRun) IsPaused() bool { + return in.Spec.isPaused() +} + // Parse extracts Script data bits from K6 spec and performs basic validation func (k6 TestRunSpec) ParseScript() (*types.Script, error) { spec := k6.Script diff --git a/api/v1alpha1/testruni.go b/api/v1alpha1/testruni.go index 8dd78394..1abc57b7 100644 --- a/api/v1alpha1/testruni.go +++ b/api/v1alpha1/testruni.go @@ -3,7 +3,7 @@ package v1alpha1 import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -20,6 +20,7 @@ type TestRunI interface { GetStatus() *TestRunStatus GetSpec() *TestRunSpec NamespacedName() types.NamespacedName + IsPaused() bool } // TestRunID is a tiny helper to get k6 Cloud test run ID. diff --git a/controllers/suite_test.go b/controllers/suite_test.go index 199ff750..444feb27 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -17,8 +17,10 @@ package controllers import ( "path/filepath" "testing" + "time" - . "github.com/onsi/ginkgo" + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -44,8 +46,8 @@ func TestAPIs(t *testing.T) { RunSpecs(t, "Controller Suite") } -var _ = BeforeSuite(func(done Done) { - logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter))) +var _ = BeforeSuite(func(ctx SpecContext) { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) By("bootstrapping test environment") testEnv = &envtest.Environment{ @@ -69,8 +71,14 @@ var _ = BeforeSuite(func(done Done) { Expect(err).ToNot(HaveOccurred()) Expect(k8sClient).ToNot(BeNil()) - close(done) -}, 60) + testRunSuiteReconciler = &TestRunReconciler{ + Client: k8sClient, + Log: logr.Logger{}, + Scheme: k8sClient.Scheme(), + k6CloudClient: nil, + } + +}, NodeTimeout(time.Minute)) var _ = AfterSuite(func() { By("tearing down the test environment") diff --git a/controllers/testrun_controller.go b/controllers/testrun_controller.go index 665978d3..cb8e6195 100644 --- a/controllers/testrun_controller.go +++ b/controllers/testrun_controller.go @@ -25,8 +25,6 @@ import ( "k8s.io/apimachinery/pkg/types" "github.com/go-logr/logr" - "github.com/grafana/k6-operator/api/v1alpha1" - "github.com/grafana/k6-operator/pkg/cloud" batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -38,6 +36,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/grafana/k6-operator/api/v1alpha1" + "github.com/grafana/k6-operator/pkg/cloud" ) // TestRunReconciler reconciles a K6 object @@ -194,6 +195,10 @@ func (r *TestRunReconciler) reconcile(ctx context.Context, req ctrl.Request, log return CreateJobs(ctx, log, k6, r) case "created": + if k6.IsPaused() { + // nothing to do. When the user updates spec.paused a new reconciliation will trigger and we'll check again + return ctrl.Result{}, nil + } return StartJobs(ctx, log, k6, r) case "started": diff --git a/controllers/testrun_controller_test.go b/controllers/testrun_controller_test.go new file mode 100644 index 00000000..28fa7460 --- /dev/null +++ b/controllers/testrun_controller_test.go @@ -0,0 +1,125 @@ +package controllers + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + batchv1 "k8s.io/api/batch/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/grafana/k6-operator/api/v1alpha1" +) + +var testRunSuiteReconciler *TestRunReconciler + +var _ = Describe("TestRun", func() { + ctx := context.Background() + + testRun := &v1alpha1.TestRun{} + starterJob := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "some-test-starter", + }, + } + + BeforeEach(func() { + testRun = &v1alpha1.TestRun{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{k6CrLabelName: "some-test"}, + Name: "some-test", + Namespace: "default", + }, + } + }) + + AfterEach(func() { + Expect(k8sClient.Delete(ctx, testRun)).Error().ToNot(HaveOccurred()) + err := k8sClient.Delete(ctx, starterJob) + Expect(client.IgnoreNotFound(err)).Error().ToNot(HaveOccurred()) + }) + + When("Reconciling a TestRun that is in 'created' stage and spec.paused is set to 'true'", func() { + It("should prevent the starter job from running", func() { + + testRun.Spec.Paused = "true" + Expect(k8sClient.Create(ctx, testRun)).Error().ToNot(HaveOccurred()) + testRun.Status.Stage = "created" + Expect(k8sClient.Status().Update(ctx, testRun)).Error().ToNot(HaveOccurred()) + + By("returning no error and no requeue when reconciled", func() { + result, err := testRunSuiteReconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "some-test", + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeZero()) + }) + + By("not having started the jobs", func() { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(starterJob), &batchv1.Job{}) + Expect(k8sErrors.IsNotFound(err)).To(BeTrue()) + }) + }) + }) + + When("Reconciling a TestRun that is in 'created' stage and spec.paused isn't set", func() { + It("should prevent the starter job from running", func() { + + testRun.Spec.Paused = "" + Expect(k8sClient.Create(ctx, testRun)).Error().ToNot(HaveOccurred()) + testRun.Status.Stage = "created" + Expect(k8sClient.Status().Update(ctx, testRun)).Error().ToNot(HaveOccurred()) + + By("returning no error and no requeue when reconciled", func() { + result, err := testRunSuiteReconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "some-test", + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeZero()) + }) + + By("not having started the jobs", func() { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(starterJob), &batchv1.Job{}) + Expect(k8sErrors.IsNotFound(err)).To(BeTrue()) + }) + }) + }) + + When("Reconciling a TestRun that is in 'created' stage and spec.paused is set to 'false'", func() { + It("should create the starter job", func() { + + testRun.Spec.Paused = "false" + Expect(k8sClient.Create(ctx, testRun)).Error().ToNot(HaveOccurred()) + testRun.Status.Stage = "created" + Expect(k8sClient.Status().Update(ctx, testRun)).Error().ToNot(HaveOccurred()) + + By("returning no error and no requeue when reconciled", func() { + // we don't care about the result itself for what this test asserts, just the error + _, err := testRunSuiteReconciler.Reconcile(context.Background(), ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: "default", + Name: "some-test", + }, + }) + Expect(err).ToNot(HaveOccurred()) + }) + + By("having started the jobs", func() { + err := k8sClient.Get(ctx, client.ObjectKeyFromObject(starterJob), &batchv1.Job{}) + Expect(err).NotTo(HaveOccurred()) + }) + }) + }) +}) diff --git a/go.mod b/go.mod index bd58b2a2..e05b4069 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/go-logr/logr v1.4.1 github.com/go-test/deep v1.0.7 github.com/google/uuid v1.6.0 - github.com/onsi/ginkgo v1.16.5 + github.com/onsi/ginkgo/v2 v2.11.0 github.com/onsi/gomega v1.27.10 github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.9.0 @@ -35,6 +35,7 @@ require ( github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.3 // indirect + github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.4 // indirect @@ -82,6 +83,7 @@ require ( golang.org/x/term v0.21.0 // indirect golang.org/x/text v0.16.0 // indirect golang.org/x/time v0.5.0 // indirect + golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240325203815-454cdb8f5daa // indirect diff --git a/go.sum b/go.sum index 445b2e4e..6356d8b6 100644 --- a/go.sum +++ b/go.sum @@ -202,6 +202,7 @@ github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSS github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= diff --git a/pkg/resources/jobs/runner.go b/pkg/resources/jobs/runner.go index 54105401..ca97b33c 100644 --- a/pkg/resources/jobs/runner.go +++ b/pkg/resources/jobs/runner.go @@ -3,17 +3,17 @@ package jobs import ( "fmt" "strconv" + "strings" "k8s.io/apimachinery/pkg/util/intstr" - "strings" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/grafana/k6-operator/api/v1alpha1" "github.com/grafana/k6-operator/pkg/cloud" "github.com/grafana/k6-operator/pkg/segmentation" - batchv1 "k8s.io/api/batch/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // NewRunnerJob creates a new k6 job from a CRD @@ -58,12 +58,7 @@ func NewRunnerJob(k6 v1alpha1.TestRunI, index int, token string) (*batchv1.Job, script.FullName(), "--address=0.0.0.0:6565") - paused := true - if k6.GetSpec().Paused != "" { - paused, _ = strconv.ParseBool(k6.GetSpec().Paused) - } - - if paused { + if k6.IsPaused() { command = append(command, "--paused") } diff --git a/pkg/segmentation/suite_test.go b/pkg/segmentation/suite_test.go index b64ae18c..f5d9f6fc 100644 --- a/pkg/segmentation/suite_test.go +++ b/pkg/segmentation/suite_test.go @@ -4,9 +4,10 @@ import ( "fmt" "testing" - "github.com/grafana/k6-operator/pkg/segmentation" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + + "github.com/grafana/k6-operator/pkg/segmentation" ) func TestSegmentation(t *testing.T) {