From 0c4ce6d70f7ee0940a57f74ba73e308ba6bbd407 Mon Sep 17 00:00:00 2001 From: Wazery Date: Mon, 10 Feb 2025 12:13:12 +0000 Subject: [PATCH] =?UTF-8?q?=20=F0=9F=8C=B1=20Improve=20samples=20linting?= =?UTF-8?q?=20&=20fix=20samples=20lint=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit includes: - Fix linter issues - Refactor fetchCronJob in sample controller reconcile func - Fix getting-started tutorial lint issues - Fix multiversion tutorial lint issues - Fix cronjob-tutorial lint issues - Refactor cronjob controller reconcile to reduce cyclomatic complexity --- .github/workflows/lint-sample.yml | 5 ++- .../internal/controller/cronjob_controller.go | 1 + .../controller/cronjob_controller_test.go | 2 +- .../internal/webhook/v1/cronjob_webhook.go | 1 + .../webhook/v1/cronjob_webhook_test.go | 23 ++++++------ .../controller/memcached_controller.go | 6 ++-- .../internal/controller/cronjob_controller.go | 1 + .../controller/cronjob_controller_test.go | 2 +- .../internal/webhook/v1/cronjob_webhook.go | 1 + .../webhook/v1/cronjob_webhook_test.go | 22 ++++++------ .../controller_implementation.go | 3 ++ .../cronjob-tutorial/generate_cronjob.go | 11 ++++++ .../webhook_implementation.go | 35 +++++++++++++------ .../writing_tests_controller.go | 2 +- .../generate_getting_started.go | 4 ++- .../generate_multiversion.go | 22 ++++++------ 16 files changed, 94 insertions(+), 47 deletions(-) diff --git a/.github/workflows/lint-sample.yml b/.github/workflows/lint-sample.yml index 61d7533eb40..2f7f38ea588 100644 --- a/.github/workflows/lint-sample.yml +++ b/.github/workflows/lint-sample.yml @@ -16,7 +16,10 @@ jobs: folder: [ "testdata/project-v4", "testdata/project-v4-with-plugins", - "testdata/project-v4-multigroup" + "testdata/project-v4-multigroup", + "docs/book/src/cronjob-tutorial/testdata/project", + "docs/book/src/getting-started/testdata/project", + "docs/book/src/multiversion-tutorial/testdata/project" ] if: (github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository) steps: diff --git a/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go b/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go index 5afead5012e..04694b49266 100644 --- a/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go +++ b/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go @@ -96,6 +96,7 @@ var ( // // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.20.1/pkg/reconcile +// nolint:gocyclo func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := log.FromContext(ctx) diff --git a/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller_test.go b/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller_test.go index 6da599b350f..03e161efd6c 100644 --- a/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller_test.go +++ b/docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller_test.go @@ -129,7 +129,7 @@ var _ = Describe("CronJob controller", func() { By("By checking the CronJob has zero active Jobs") Consistently(func(g Gomega) { g.Expect(k8sClient.Get(ctx, cronjobLookupKey, createdCronjob)).To(Succeed()) - g.Expect(createdCronjob.Status.Active).To(HaveLen(0)) + g.Expect(createdCronjob.Status.Active).To(BeEmpty()) }, duration, interval).Should(Succeed()) /* Next, we actually create a stubbed Job that will belong to our CronJob, as well as its downstream template specs. diff --git a/docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go b/docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go index 55b66e4c7d4..7a1acf49d0b 100644 --- a/docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go +++ b/docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go @@ -20,6 +20,7 @@ package v1 import ( "context" "fmt" + "github.com/robfig/cron" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" diff --git a/docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go b/docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go index da4421ba71e..9bec5e3e2d4 100644 --- a/docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go +++ b/docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go @@ -32,10 +32,13 @@ var _ = Describe("CronJob Webhook", func() { defaulter CronJobCustomDefaulter ) + const validCronJobName = "valid-cronjob-name" + const schedule = "*/5 * * * *" + BeforeEach(func() { obj = &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: "*/5 * * * *", + Schedule: schedule, ConcurrencyPolicy: batchv1.AllowConcurrent, SuccessfulJobsHistoryLimit: new(int32), FailedJobsHistoryLimit: new(int32), @@ -46,7 +49,7 @@ var _ = Describe("CronJob Webhook", func() { oldObj = &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: "*/5 * * * *", + Schedule: schedule, ConcurrencyPolicy: batchv1.AllowConcurrent, SuccessfulJobsHistoryLimit: new(int32), FailedJobsHistoryLimit: new(int32), @@ -80,7 +83,7 @@ var _ = Describe("CronJob Webhook", func() { obj.Spec.FailedJobsHistoryLimit = nil // This should default to 1 By("calling the Default method to apply defaults") - defaulter.Default(ctx, obj) + _ = defaulter.Default(ctx, obj) By("checking that the default values are set") Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent") @@ -100,7 +103,7 @@ var _ = Describe("CronJob Webhook", func() { *obj.Spec.FailedJobsHistoryLimit = 2 By("calling the Default method to apply defaults") - defaulter.Default(ctx, obj) + _ = defaulter.Default(ctx, obj) By("checking that the fields were not overwritten") Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value") @@ -119,7 +122,7 @@ var _ = Describe("CronJob Webhook", func() { }) It("Should admit creation if the name is valid", func() { - obj.ObjectMeta.Name = "valid-cronjob-name" + obj.ObjectMeta.Name = validCronJobName Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(), "Expected name validation to pass for a valid name") }) @@ -132,14 +135,14 @@ var _ = Describe("CronJob Webhook", func() { }) It("Should admit creation if the schedule is valid", func() { - obj.Spec.Schedule = "*/5 * * * *" + obj.Spec.Schedule = schedule Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(), "Expected spec validation to pass for a valid schedule") }) It("Should deny update if both name and spec are invalid", func() { - oldObj.ObjectMeta.Name = "valid-cronjob-name" - oldObj.Spec.Schedule = "*/5 * * * *" + oldObj.ObjectMeta.Name = validCronJobName + oldObj.Spec.Schedule = schedule By("simulating an update") obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long" @@ -151,8 +154,8 @@ var _ = Describe("CronJob Webhook", func() { }) It("Should admit update if both name and spec are valid", func() { - oldObj.ObjectMeta.Name = "valid-cronjob-name" - oldObj.Spec.Schedule = "*/5 * * * *" + oldObj.ObjectMeta.Name = validCronJobName + oldObj.Spec.Schedule = schedule By("simulating an update") obj.ObjectMeta.Name = "valid-cronjob-name-updated" diff --git a/docs/book/src/getting-started/testdata/project/internal/controller/memcached_controller.go b/docs/book/src/getting-started/testdata/project/internal/controller/memcached_controller.go index 5b0f7d59e58..71766380a19 100644 --- a/docs/book/src/getting-started/testdata/project/internal/controller/memcached_controller.go +++ b/docs/book/src/getting-started/testdata/project/internal/controller/memcached_controller.go @@ -19,6 +19,9 @@ package controller import ( "context" "fmt" + + "time" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -26,7 +29,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" - "time" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" @@ -89,7 +91,7 @@ func (r *MemcachedReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } // Let's just set the status as Unknown when no status is available - if memcached.Status.Conditions == nil || len(memcached.Status.Conditions) == 0 { + if len(memcached.Status.Conditions) == 0 { meta.SetStatusCondition(&memcached.Status.Conditions, metav1.Condition{Type: typeAvailableMemcached, Status: metav1.ConditionUnknown, Reason: "Reconciling", Message: "Starting reconciliation"}) if err = r.Status().Update(ctx, memcached); err != nil { log.Error(err, "Failed to update Memcached status") diff --git a/docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller.go b/docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller.go index 5afead5012e..04694b49266 100644 --- a/docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller.go +++ b/docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller.go @@ -96,6 +96,7 @@ var ( // // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.20.1/pkg/reconcile +// nolint:gocyclo func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { log := log.FromContext(ctx) diff --git a/docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller_test.go b/docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller_test.go index 6da599b350f..03e161efd6c 100644 --- a/docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller_test.go +++ b/docs/book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller_test.go @@ -129,7 +129,7 @@ var _ = Describe("CronJob controller", func() { By("By checking the CronJob has zero active Jobs") Consistently(func(g Gomega) { g.Expect(k8sClient.Get(ctx, cronjobLookupKey, createdCronjob)).To(Succeed()) - g.Expect(createdCronjob.Status.Active).To(HaveLen(0)) + g.Expect(createdCronjob.Status.Active).To(BeEmpty()) }, duration, interval).Should(Succeed()) /* Next, we actually create a stubbed Job that will belong to our CronJob, as well as its downstream template specs. diff --git a/docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go b/docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go index f0fdd40d15d..55f3ce98012 100644 --- a/docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go +++ b/docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go @@ -20,6 +20,7 @@ package v1 import ( "context" "fmt" + "github.com/robfig/cron" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" diff --git a/docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go b/docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go index b6d3ac3e720..a5474f47d52 100644 --- a/docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go +++ b/docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go @@ -31,11 +31,13 @@ var _ = Describe("CronJob Webhook", func() { validator CronJobCustomValidator defaulter CronJobCustomDefaulter ) + const validCronJobName = "valid-cronjob-name" + const schedule = "*/5 * * * *" BeforeEach(func() { obj = &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: "*/5 * * * *", + Schedule: schedule, ConcurrencyPolicy: batchv1.AllowConcurrent, SuccessfulJobsHistoryLimit: new(int32), FailedJobsHistoryLimit: new(int32), @@ -46,7 +48,7 @@ var _ = Describe("CronJob Webhook", func() { oldObj = &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: "*/5 * * * *", + Schedule: schedule, ConcurrencyPolicy: batchv1.AllowConcurrent, SuccessfulJobsHistoryLimit: new(int32), FailedJobsHistoryLimit: new(int32), @@ -80,7 +82,7 @@ var _ = Describe("CronJob Webhook", func() { obj.Spec.FailedJobsHistoryLimit = nil // This should default to 1 By("calling the Default method to apply defaults") - defaulter.Default(ctx, obj) + _ = defaulter.Default(ctx, obj) By("checking that the default values are set") Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent") @@ -100,7 +102,7 @@ var _ = Describe("CronJob Webhook", func() { *obj.Spec.FailedJobsHistoryLimit = 2 By("calling the Default method to apply defaults") - defaulter.Default(ctx, obj) + _ = defaulter.Default(ctx, obj) By("checking that the fields were not overwritten") Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value") @@ -119,7 +121,7 @@ var _ = Describe("CronJob Webhook", func() { }) It("Should admit creation if the name is valid", func() { - obj.ObjectMeta.Name = "valid-cronjob-name" + obj.ObjectMeta.Name = validCronJobName Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(), "Expected name validation to pass for a valid name") }) @@ -132,14 +134,14 @@ var _ = Describe("CronJob Webhook", func() { }) It("Should admit creation if the schedule is valid", func() { - obj.Spec.Schedule = "*/5 * * * *" + obj.Spec.Schedule = schedule Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(), "Expected spec validation to pass for a valid schedule") }) It("Should deny update if both name and spec are invalid", func() { - oldObj.ObjectMeta.Name = "valid-cronjob-name" - oldObj.Spec.Schedule = "*/5 * * * *" + oldObj.ObjectMeta.Name = validCronJobName + oldObj.Spec.Schedule = schedule By("simulating an update") obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long" @@ -151,8 +153,8 @@ var _ = Describe("CronJob Webhook", func() { }) It("Should admit update if both name and spec are valid", func() { - oldObj.ObjectMeta.Name = "valid-cronjob-name" - oldObj.Spec.Schedule = "*/5 * * * *" + oldObj.ObjectMeta.Name = validCronJobName + oldObj.Spec.Schedule = schedule By("simulating an update") obj.ObjectMeta.Name = "valid-cronjob-name-updated" diff --git a/hack/docs/internal/cronjob-tutorial/controller_implementation.go b/hack/docs/internal/cronjob-tutorial/controller_implementation.go index 738cba2f0e1..1be782f8758 100644 --- a/hack/docs/internal/cronjob-tutorial/controller_implementation.go +++ b/hack/docs/internal/cronjob-tutorial/controller_implementation.go @@ -84,6 +84,9 @@ var ( ) ` +const skipGoCycloLint = ` +// nolint:gocyclo` + const controllerReconcileLogic = `log := log.FromContext(ctx) /* diff --git a/hack/docs/internal/cronjob-tutorial/generate_cronjob.go b/hack/docs/internal/cronjob-tutorial/generate_cronjob.go index d4324d5fcbf..57d49e4cd93 100644 --- a/hack/docs/internal/cronjob-tutorial/generate_cronjob.go +++ b/hack/docs/internal/cronjob-tutorial/generate_cronjob.go @@ -286,6 +286,11 @@ func (sp *Sample) updateController() { `// +kubebuilder:rbac:groups=batch.tutorial.kubebuilder.io,resources=cronjobs/finalizers,verbs=update`, controllerReconcile) hackutils.CheckError("fixing cronjob_controller.go", err) + err = pluginutil.InsertCode( + filepath.Join(sp.ctx.Dir, "internal/controller/cronjob_controller.go"), + `// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.20.1/pkg/reconcile`, skipGoCycloLint) + hackutils.CheckError("fixing cronjob_controller.go", err) + err = pluginutil.ReplaceInFile( filepath.Join(sp.ctx.Dir, "internal/controller/cronjob_controller.go"), ` _ = log.FromContext(ctx) @@ -398,6 +403,11 @@ func (sp *Sample) updateWebhookTests() { webhookTestingValidatingExampleFragment) hackutils.CheckError("replace validating defaulting test", err) + err = pluginutil.ReplaceInFile(file, + webhookTestsVars, + webhookTestsConstants) + hackutils.CheckError("replace before each webhook test ", err) + err = pluginutil.ReplaceInFile(file, webhookTestsBeforeEachOriginal, webhookTestsBeforeEachChanged) @@ -420,6 +430,7 @@ func (sp *Sample) updateWebhook() { "context" "fmt"`, ` + "github.com/robfig/cron" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" diff --git a/hack/docs/internal/cronjob-tutorial/webhook_implementation.go b/hack/docs/internal/cronjob-tutorial/webhook_implementation.go index 7fcff38509e..79584a14df2 100644 --- a/hack/docs/internal/cronjob-tutorial/webhook_implementation.go +++ b/hack/docs/internal/cronjob-tutorial/webhook_implementation.go @@ -205,7 +205,7 @@ const webhookTestCreateDefaultingReplaceFragment = `It("Should apply defaults wh obj.Spec.FailedJobsHistoryLimit = nil // This should default to 1 By("calling the Default method to apply defaults") - defaulter.Default(ctx, obj) + _ = defaulter.Default(ctx, obj) By("checking that the default values are set") Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent") @@ -225,7 +225,7 @@ const webhookTestCreateDefaultingReplaceFragment = `It("Should apply defaults wh *obj.Spec.FailedJobsHistoryLimit = 2 By("calling the Default method to apply defaults") - defaulter.Default(ctx, obj) + _ = defaulter.Default(ctx, obj) By("checking that the fields were not overwritten") Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value") @@ -263,7 +263,7 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the }) It("Should admit creation if the name is valid", func() { - obj.ObjectMeta.Name = "valid-cronjob-name" + obj.ObjectMeta.Name = validCronJobName Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(), "Expected name validation to pass for a valid name") }) @@ -276,14 +276,14 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the }) It("Should admit creation if the schedule is valid", func() { - obj.Spec.Schedule = "*/5 * * * *" + obj.Spec.Schedule = schedule Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(), "Expected spec validation to pass for a valid schedule") }) It("Should deny update if both name and spec are invalid", func() { - oldObj.ObjectMeta.Name = "valid-cronjob-name" - oldObj.Spec.Schedule = "*/5 * * * *" + oldObj.ObjectMeta.Name = validCronJobName + oldObj.Spec.Schedule = schedule By("simulating an update") obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long" @@ -295,8 +295,8 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the }) It("Should admit update if both name and spec are valid", func() { - oldObj.ObjectMeta.Name = "valid-cronjob-name" - oldObj.Spec.Schedule = "*/5 * * * *" + oldObj.ObjectMeta.Name = validCronJobName + oldObj.Spec.Schedule = schedule By("simulating an update") obj.ObjectMeta.Name = "valid-cronjob-name-updated" @@ -307,6 +307,21 @@ const webhookTestingValidatingExampleFragment = `It("Should deny creation if the "Expected validation to pass for a valid update") })` +const webhookTestsVars = `var ( + obj *batchv1.CronJob + oldObj *batchv1.CronJob + validator CronJobCustomValidator + defaulter CronJobCustomDefaulter + )` +const webhookTestsConstants = ` var ( + obj *batchv1.CronJob + oldObj *batchv1.CronJob + validator CronJobCustomValidator + defaulter CronJobCustomDefaulter + ) + + const validCronJobName = "valid-cronjob-name" + const schedule = "*/5 * * * *"` const webhookTestsBeforeEachOriginal = `obj = &batchv1.CronJob{} oldObj = &batchv1.CronJob{} validator = CronJobCustomValidator{} @@ -319,7 +334,7 @@ const webhookTestsBeforeEachOriginal = `obj = &batchv1.CronJob{} const webhookTestsBeforeEachChanged = `obj = &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: "*/5 * * * *", + Schedule: schedule, ConcurrencyPolicy: batchv1.AllowConcurrent, SuccessfulJobsHistoryLimit: new(int32), FailedJobsHistoryLimit: new(int32), @@ -330,7 +345,7 @@ const webhookTestsBeforeEachChanged = `obj = &batchv1.CronJob{ oldObj = &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: "*/5 * * * *", + Schedule: schedule, ConcurrencyPolicy: batchv1.AllowConcurrent, SuccessfulJobsHistoryLimit: new(int32), FailedJobsHistoryLimit: new(int32), diff --git a/hack/docs/internal/cronjob-tutorial/writing_tests_controller.go b/hack/docs/internal/cronjob-tutorial/writing_tests_controller.go index 097cb488be4..160e1594218 100644 --- a/hack/docs/internal/cronjob-tutorial/writing_tests_controller.go +++ b/hack/docs/internal/cronjob-tutorial/writing_tests_controller.go @@ -148,7 +148,7 @@ var _ = Describe("CronJob controller", func() { By("By checking the CronJob has zero active Jobs") Consistently(func(g Gomega) { g.Expect(k8sClient.Get(ctx, cronjobLookupKey, createdCronjob)).To(Succeed()) - g.Expect(createdCronjob.Status.Active).To(HaveLen(0)) + g.Expect(createdCronjob.Status.Active).To(BeEmpty()) }, duration, interval).Should(Succeed()) /* Next, we actually create a stubbed Job that will belong to our CronJob, as well as its downstream template specs. diff --git a/hack/docs/internal/getting-started/generate_getting_started.go b/hack/docs/internal/getting-started/generate_getting_started.go index 36b04ea514d..6c71a7fd4bc 100644 --- a/hack/docs/internal/getting-started/generate_getting_started.go +++ b/hack/docs/internal/getting-started/generate_getting_started.go @@ -271,7 +271,9 @@ const sampleSizeFragment = `# TODO(user): edit the following value to ensure the const controllerImports = `"context" "fmt" + "time" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -320,7 +322,7 @@ const controllerReconcileImplementation = `// Fetch the Memcached instance } // Let's just set the status as Unknown when no status is available - if memcached.Status.Conditions == nil || len(memcached.Status.Conditions) == 0 { + if len(memcached.Status.Conditions) == 0 { meta.SetStatusCondition(&memcached.Status.Conditions, metav1.Condition{Type: typeAvailableMemcached, Status: metav1.ConditionUnknown, Reason: "Reconciling", Message: "Starting reconciliation"}) if err = r.Status().Update(ctx, memcached); err != nil { log.Error(err, "Failed to update Memcached status") diff --git a/hack/docs/internal/multiversion-tutorial/generate_multiversion.go b/hack/docs/internal/multiversion-tutorial/generate_multiversion.go index fe278f8ea2c..17f2fd5d594 100644 --- a/hack/docs/internal/multiversion-tutorial/generate_multiversion.go +++ b/hack/docs/internal/multiversion-tutorial/generate_multiversion.go @@ -139,11 +139,13 @@ interfaces, a conversion webhook will be registered. validator CronJobCustomValidator defaulter CronJobCustomDefaulter ) + const validCronJobName = "valid-cronjob-name" + const schedule = "*/5 * * * *" BeforeEach(func() { obj = &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: "*/5 * * * *", + Schedule: schedule, ConcurrencyPolicy: batchv1.AllowConcurrent, SuccessfulJobsHistoryLimit: new(int32), FailedJobsHistoryLimit: new(int32), @@ -154,7 +156,7 @@ interfaces, a conversion webhook will be registered. oldObj = &batchv1.CronJob{ Spec: batchv1.CronJobSpec{ - Schedule: "*/5 * * * *", + Schedule: schedule, ConcurrencyPolicy: batchv1.AllowConcurrent, SuccessfulJobsHistoryLimit: new(int32), FailedJobsHistoryLimit: new(int32), @@ -191,7 +193,7 @@ interfaces, a conversion webhook will be registered. obj.Spec.FailedJobsHistoryLimit = nil // This should default to 1 By("calling the Default method to apply defaults") - defaulter.Default(ctx, obj) + _ = defaulter.Default(ctx, obj) By("checking that the default values are set") Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.AllowConcurrent), "Expected ConcurrencyPolicy to default to AllowConcurrent") @@ -211,7 +213,7 @@ interfaces, a conversion webhook will be registered. *obj.Spec.FailedJobsHistoryLimit = 2 By("calling the Default method to apply defaults") - defaulter.Default(ctx, obj) + _ = defaulter.Default(ctx, obj) By("checking that the fields were not overwritten") Expect(obj.Spec.ConcurrencyPolicy).To(Equal(batchv1.ForbidConcurrent), "Expected ConcurrencyPolicy to retain its set value") @@ -230,7 +232,7 @@ interfaces, a conversion webhook will be registered. }) It("Should admit creation if the name is valid", func() { - obj.ObjectMeta.Name = "valid-cronjob-name" + obj.ObjectMeta.Name = validCronJobName Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(), "Expected name validation to pass for a valid name") }) @@ -243,14 +245,14 @@ interfaces, a conversion webhook will be registered. }) It("Should admit creation if the schedule is valid", func() { - obj.Spec.Schedule = "*/5 * * * *" + obj.Spec.Schedule = schedule Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(), "Expected spec validation to pass for a valid schedule") }) It("Should deny update if both name and spec are invalid", func() { - oldObj.ObjectMeta.Name = "valid-cronjob-name" - oldObj.Spec.Schedule = "*/5 * * * *" + oldObj.ObjectMeta.Name = validCronJobName + oldObj.Spec.Schedule = schedule By("simulating an update") obj.ObjectMeta.Name = "this-name-is-way-too-long-and-should-fail-validation-because-it-is-way-too-long" @@ -262,8 +264,8 @@ interfaces, a conversion webhook will be registered. }) It("Should admit update if both name and spec are valid", func() { - oldObj.ObjectMeta.Name = "valid-cronjob-name" - oldObj.Spec.Schedule = "*/5 * * * *" + oldObj.ObjectMeta.Name = validCronJobName + oldObj.Spec.Schedule = schedule By("simulating an update") obj.ObjectMeta.Name = "valid-cronjob-name-updated"