Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 : Improve samples linting & fix samples lint issues #4548

Merged
merged 1 commit into from
Feb 10, 2025
Merged

🌱 : Improve samples linting & fix samples lint issues #4548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/lint-sample.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions docs/book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion ...book/src/cronjob-tutorial/testdata/project/internal/controller/cronjob_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
23 changes: 13 additions & 10 deletions docs/book/src/cronjob-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
})
Expand All @@ -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"
Expand All @@ -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"
Expand Down
6 changes: 4 additions & 2 deletions docs/book/src/getting-started/testdata/project/internal/controller/memcached_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ 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"
"k8s.io/apimachinery/pkg/api/meta"
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"
Expand Down Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions ...book/src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion ...src/multiversion-tutorial/testdata/project/internal/controller/cronjob_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions docs/book/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
22 changes: 12 additions & 10 deletions ...ok/src/multiversion-tutorial/testdata/project/internal/webhook/v1/cronjob_webhook_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand All @@ -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")
})
Expand All @@ -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"
Expand All @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions hack/docs/internal/cronjob-tutorial/controller_implementation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@ var (
)
`

const skipGoCycloLint = `
// nolint:gocyclo`

const controllerReconcileLogic = `log := log.FromContext(ctx)

/*
Expand Down
11 changes: 11 additions & 0 deletions hack/docs/internal/cronjob-tutorial/generate_cronjob.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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"
Expand Down
35 changes: 25 additions & 10 deletions hack/docs/internal/cronjob-tutorial/webhook_implementation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
})
Expand All @@ -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"
Expand All @@ -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"
Expand All @@ -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{}
Expand All @@ -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),
Expand All @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion hack/docs/internal/cronjob-tutorial/writing_tests_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading
Loading