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.

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

🌱 (ci): Add linter for the samples scaffolded under docs #4464

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,16 @@ import (
"sort"
"time"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/robfig/cron"
kbatch "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
ref "k8s.io/client-go/tools/reference"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

batchv1 "tutorial.kubebuilder.io/project/api/v1"
)
Expand Down Expand Up @@ -96,6 +97,7 @@ var (
//
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
// nolint:gocyclo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HI @jongwooo

That is nice, but see that ALL samples under the docs are generated with the tools and we have a code implementation to add the code on top. They are generated automatically when we run make generate-docs.

See: https://github.com/kubernetes-sigs/kubebuilder/tree/master/hack/docs

So, the changes on the docs need to be addressed there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change anything that change the default scaffold then we run make generate which will re-generate the samples under testdata and the docs as well

func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := log.FromContext(ctx)

Expand Down
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(HaveLen(0)) // nolint:ginkgolinter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @jongwooo

to apply changes to these files, you need to change code in hack/docs/internal folder, and then run make generate.

For example, for this line, you need to change it here

g.Expect(createdCronjob.Status.Active).To(HaveLen(0))

}, 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
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,20 @@ package v1
import (
"context"
"fmt"

"github.com/robfig/cron"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
validationutils "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"

"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"k8s.io/apimachinery/pkg/runtime"

batchv1 "tutorial.kubebuilder.io/project/api/v1"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,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) // nolint:errcheck
jongwooo marked this conversation as resolved.
Show resolved Hide resolved

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 +100,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) // nolint:errcheck
jongwooo marked this conversation as resolved.
Show resolved Hide resolved

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 +119,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 = "valid-cronjob-name" // nolint:goconst
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
"Expected name validation to pass for a valid name")
})
Expand All @@ -132,7 +132,7 @@ var _ = Describe("CronJob Webhook", func() {
})

It("Should admit creation if the schedule is valid", func() {
obj.Spec.Schedule = "*/5 * * * *"
obj.Spec.Schedule = "*/5 * * * *" // nolint:goconst
jongwooo marked this conversation as resolved.
Show resolved Hide resolved
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
"Expected spec validation to pass for a valid schedule")
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,22 @@ 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"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

"k8s.io/apimachinery/pkg/runtime"

cachev1alpha1 "example.com/memcached/api/v1alpha1"
)

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 memcached.Status.Conditions == nil || len(memcached.Status.Conditions) == 0 { // nolint:gosimple
jongwooo marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,16 @@ import (
"sort"
"time"

ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/robfig/cron"
kbatch "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
ref "k8s.io/client-go/tools/reference"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

batchv1 "tutorial.kubebuilder.io/project/api/v1"
)
Expand Down Expand Up @@ -96,6 +97,7 @@ var (
//
// For more details, check Reconcile and its Result here:
// - https://pkg.go.dev/sigs.k8s.io/[email protected]/pkg/reconcile
// nolint:gocyclo
jongwooo marked this conversation as resolved.
Show resolved Hide resolved
func (r *CronJobReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
log := log.FromContext(ctx)

Expand Down
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(HaveLen(0)) // nolint:ginkgolinter
jongwooo marked this conversation as resolved.
Show resolved Hide resolved
}, 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
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,20 @@ package v1
import (
"context"
"fmt"

"github.com/robfig/cron"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
validationutils "k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"

"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"k8s.io/apimachinery/pkg/runtime"

batchv1 "tutorial.kubebuilder.io/project/api/v1"
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,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) // nolint:errcheck

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 +100,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) //nolint:errcheck

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 +119,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 = "valid-cronjob-name" // nolint:goconst
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
"Expected name validation to pass for a valid name")
})
Expand All @@ -132,7 +132,7 @@ var _ = Describe("CronJob Webhook", func() {
})

It("Should admit creation if the schedule is valid", func() {
obj.Spec.Schedule = "*/5 * * * *"
obj.Spec.Schedule = "*/5 * * * *" // nolint:goconst
Expect(validator.ValidateCreate(ctx, obj)).To(BeNil(),
"Expected spec validation to pass for a valid schedule")
})
Expand Down
Loading