From 8424e40180deb705500b5d4a47c92f651e04d2bb Mon Sep 17 00:00:00 2001 From: David Grove Date: Thu, 20 Mar 2025 14:00:21 -0400 Subject: [PATCH 1/2] Add linter rule to organize imports into sections --- .golangci.yml | 57 ++++++++++++------- cmd/main.go | 12 ++-- .../appwrapper/appwrapper_controller.go | 1 - .../appwrapper/appwrapper_controller_test.go | 5 +- .../controller/appwrapper/fixtures_test.go | 4 +- .../appwrapper/node_health_monitor.go | 1 - .../appwrapper/node_health_monitor_test.go | 8 ++- .../appwrapper/resource_management.go | 7 ++- internal/controller/appwrapper/suite_test.go | 8 +-- internal/webhook/appwrapper_fixtures_test.go | 7 ++- internal/webhook/appwrapper_webhook_test.go | 5 +- internal/webhook/suite_test.go | 8 +-- pkg/controller/setup.go | 3 +- pkg/logger/logger.go | 1 - pkg/utils/utils.go | 11 ++-- test/e2e/appwrapper_test.go | 5 +- test/e2e/e2e_test.go | 6 +- test/e2e/fixtures_test.go | 4 +- test/e2e/metrics_test.go | 6 +- test/e2e/util_test.go | 11 ++-- 20 files changed, 89 insertions(+), 81 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ced5821..eb21067 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -2,6 +2,41 @@ run: deadline: 5m allow-parallel-runners: true +# Settings of specific linters +linters-settings: + gci: + sections: + - standard + - default + - prefix(github.com/project-codeflare/appwrapper) + - blank + - dot + skip-generated: true # Skip generated files. + +linters: + disable-all: true + enable: + - copyloopvar + - dupl + - errcheck + - gci + - goconst + - gocyclo + - gofmt + - goimports + - gosimple + - govet + - ineffassign + - lll + - misspell + - nakedret + - prealloc + - staticcheck + - typecheck + - unconvert + - unparam + - unused + issues: # don't skip warning about doc comments # don't exclude the default set of lint @@ -28,25 +63,3 @@ issues: - path: "pkg/*" linters: - lll -linters: - disable-all: true - enable: - - copyloopvar - - dupl - - errcheck - - goconst - - gocyclo - - gofmt - - goimports - - gosimple - - govet - - ineffassign - - lll - - misspell - - nakedret - - prealloc - - staticcheck - - typecheck - - unconvert - - unparam - - unused diff --git a/cmd/main.go b/cmd/main.go index 5b5a426..d5cb713 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -24,14 +24,8 @@ import ( "os" "strings" - // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) - // to ensure that exec-entrypoint and run can make use of them. - _ "k8s.io/client-go/plugin/pkg/client/auth" - zaplog "go.uber.org/zap" "go.uber.org/zap/zapcore" - "k8s.io/utils/ptr" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -39,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -52,7 +47,10 @@ import ( "github.com/project-codeflare/appwrapper/pkg/config" "github.com/project-codeflare/appwrapper/pkg/controller" "github.com/project-codeflare/appwrapper/pkg/logger" - //+kubebuilder:scaffold:imports + + // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) + // to ensure that exec-entrypoint and run can make use of them. + _ "k8s.io/client-go/plugin/pkg/client/auth" ) var ( diff --git a/internal/controller/appwrapper/appwrapper_controller.go b/internal/controller/appwrapper/appwrapper_controller.go index 7ffaf63..0f4b66c 100644 --- a/internal/controller/appwrapper/appwrapper_controller.go +++ b/internal/controller/appwrapper/appwrapper_controller.go @@ -33,7 +33,6 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" diff --git a/internal/controller/appwrapper/appwrapper_controller_test.go b/internal/controller/appwrapper/appwrapper_controller_test.go index 3bf75d0..2907a95 100644 --- a/internal/controller/appwrapper/appwrapper_controller_test.go +++ b/internal/controller/appwrapper/appwrapper_controller_test.go @@ -19,8 +19,6 @@ package appwrapper import ( "time" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" @@ -33,6 +31,9 @@ import ( awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" "github.com/project-codeflare/appwrapper/pkg/config" "github.com/project-codeflare/appwrapper/pkg/utils" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) var _ = Describe("AppWrapper Controller", func() { diff --git a/internal/controller/appwrapper/fixtures_test.go b/internal/controller/appwrapper/fixtures_test.go index 8e6e900..4abb821 100644 --- a/internal/controller/appwrapper/fixtures_test.go +++ b/internal/controller/appwrapper/fixtures_test.go @@ -21,8 +21,6 @@ import ( "math/rand" "time" - . "github.com/onsi/gomega" - v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -33,6 +31,8 @@ import ( "sigs.k8s.io/yaml" awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" + + . "github.com/onsi/gomega" ) const charset = "abcdefghijklmnopqrstuvwxyz0123456789" diff --git a/internal/controller/appwrapper/node_health_monitor.go b/internal/controller/appwrapper/node_health_monitor.go index 76fce24..4c935b6 100644 --- a/internal/controller/appwrapper/node_health_monitor.go +++ b/internal/controller/appwrapper/node_health_monitor.go @@ -25,7 +25,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" - ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" diff --git a/internal/controller/appwrapper/node_health_monitor_test.go b/internal/controller/appwrapper/node_health_monitor_test.go index 744c59b..1c8160f 100644 --- a/internal/controller/appwrapper/node_health_monitor_test.go +++ b/internal/controller/appwrapper/node_health_monitor_test.go @@ -17,14 +17,16 @@ limitations under the License. package appwrapper import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/project-codeflare/appwrapper/pkg/config" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/project-codeflare/appwrapper/pkg/config" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) var _ = Describe("NodeMonitor Controller", func() { diff --git a/internal/controller/appwrapper/resource_management.go b/internal/controller/appwrapper/resource_management.go index e815a73..79ec104 100644 --- a/internal/controller/appwrapper/resource_management.go +++ b/internal/controller/appwrapper/resource_management.go @@ -22,9 +22,6 @@ import ( "fmt" "time" - awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" - utilmaps "github.com/project-codeflare/appwrapper/internal/util" - "github.com/project-codeflare/appwrapper/pkg/utils" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -35,6 +32,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" + + awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" + utilmaps "github.com/project-codeflare/appwrapper/internal/util" + "github.com/project-codeflare/appwrapper/pkg/utils" ) func parseComponent(raw []byte, expectedNamespace string) (*unstructured.Unstructured, error) { diff --git a/internal/controller/appwrapper/suite_test.go b/internal/controller/appwrapper/suite_test.go index e979b52..200e98d 100644 --- a/internal/controller/appwrapper/suite_test.go +++ b/internal/controller/appwrapper/suite_test.go @@ -23,11 +23,6 @@ import ( "runtime" "testing" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - //+kubebuilder:scaffold:imports - admissionv1 "k8s.io/api/admission/v1" rbacv1 "k8s.io/api/rbac/v1" apimachineryruntime "k8s.io/apimachinery/pkg/runtime" @@ -39,6 +34,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to diff --git a/internal/webhook/appwrapper_fixtures_test.go b/internal/webhook/appwrapper_fixtures_test.go index 4330ffa..72da2a3 100644 --- a/internal/webhook/appwrapper_fixtures_test.go +++ b/internal/webhook/appwrapper_fixtures_test.go @@ -21,15 +21,16 @@ import ( "math/rand" "time" - . "github.com/onsi/gomega" - - awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" "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/utils/ptr" "sigs.k8s.io/yaml" + + awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" + + . "github.com/onsi/gomega" ) const charset = "abcdefghijklmnopqrstuvwxyz0123456789" diff --git a/internal/webhook/appwrapper_webhook_test.go b/internal/webhook/appwrapper_webhook_test.go index 37b6a4f..c0a5d10 100644 --- a/internal/webhook/appwrapper_webhook_test.go +++ b/internal/webhook/appwrapper_webhook_test.go @@ -19,14 +19,15 @@ package webhook import ( "encoding/json" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" utilmaps "github.com/project-codeflare/appwrapper/internal/util" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) var _ = Describe("AppWrapper Webhook Tests", func() { diff --git a/internal/webhook/suite_test.go b/internal/webhook/suite_test.go index 07a5655..ff1c992 100644 --- a/internal/webhook/suite_test.go +++ b/internal/webhook/suite_test.go @@ -26,11 +26,6 @@ import ( "testing" "time" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - //+kubebuilder:scaffold:imports - admissionv1 "k8s.io/api/admission/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,6 +42,9 @@ import ( awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" "github.com/project-codeflare/appwrapper/pkg/config" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) // These tests use Ginkgo (BDD-style Go testing framework). Refer to diff --git a/pkg/controller/setup.go b/pkg/controller/setup.go index c1a653c..f3d517b 100644 --- a/pkg/controller/setup.go +++ b/pkg/controller/setup.go @@ -22,12 +22,11 @@ import ( "fmt" "net/http" + cert "github.com/open-policy-agent/cert-controller/pkg/rotator" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" - cert "github.com/open-policy-agent/cert-controller/pkg/rotator" - "github.com/project-codeflare/appwrapper/internal/controller/appwrapper" "github.com/project-codeflare/appwrapper/internal/webhook" "github.com/project-codeflare/appwrapper/pkg/config" diff --git a/pkg/logger/logger.go b/pkg/logger/logger.go index 38c3a2f..9b6d8a9 100644 --- a/pkg/logger/logger.go +++ b/pkg/logger/logger.go @@ -18,7 +18,6 @@ package logger import ( "github.com/go-logr/logr" - "k8s.io/apimachinery/pkg/api/errors" ) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index ec0a40f..e3bcad7 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -22,13 +22,7 @@ import ( "strconv" "strings" - // Import the crypto sha256 algorithm for the docker image parser to work - _ "crypto/sha256" - // Import the crypto/sha512 algorithm for the docker image parser to work with 384 and 512 sha hashes - _ "crypto/sha512" - dockerref "github.com/distribution/reference" - kftraining "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1" batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" @@ -39,6 +33,11 @@ import ( jobsetapi "sigs.k8s.io/jobset/api/jobset/v1alpha2" awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" + + // Import the crypto sha256 algorithm for the docker image parser to work + _ "crypto/sha256" + // Import the crypto/sha512 algorithm for the docker image parser to work with 384 and 512 sha hashes + _ "crypto/sha512" ) const templateString = "template" diff --git a/test/e2e/appwrapper_test.go b/test/e2e/appwrapper_test.go index a1146ad..f487df2 100644 --- a/test/e2e/appwrapper_test.go +++ b/test/e2e/appwrapper_test.go @@ -22,8 +22,6 @@ import ( "fmt" "time" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -32,6 +30,9 @@ import ( "k8s.io/utils/ptr" awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) var _ = Describe("AppWrapper E2E Test", func() { diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 314271b..ee7b7c7 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -20,11 +20,11 @@ import ( "context" "testing" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) var ctx context.Context diff --git a/test/e2e/fixtures_test.go b/test/e2e/fixtures_test.go index 191ff05..fb8440d 100644 --- a/test/e2e/fixtures_test.go +++ b/test/e2e/fixtures_test.go @@ -21,14 +21,14 @@ import ( "math/rand" "time" - . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" "sigs.k8s.io/yaml" awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" + + . "github.com/onsi/gomega" ) const charset = "abcdefghijklmnopqrstuvwxyz0123456789" diff --git a/test/e2e/metrics_test.go b/test/e2e/metrics_test.go index 9ba249e..3c82cf2 100644 --- a/test/e2e/metrics_test.go +++ b/test/e2e/metrics_test.go @@ -21,13 +21,13 @@ import ( "os/exec" "time" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) const ( diff --git a/test/e2e/util_test.go b/test/e2e/util_test.go index 1b680b2..0bcd947 100644 --- a/test/e2e/util_test.go +++ b/test/e2e/util_test.go @@ -21,11 +21,10 @@ import ( "fmt" "time" - // . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - + v1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -36,11 +35,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2" "github.com/project-codeflare/appwrapper/pkg/utils" + + // . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) const ( From e5c56c611e0f23e19e18d76d7dfc993086419b16 Mon Sep 17 00:00:00 2001 From: David Grove Date: Thu, 20 Mar 2025 14:32:58 -0400 Subject: [PATCH 2/2] enable a few more linters --- .golangci.yml | 2 ++ .../controller/appwrapper/node_health_monitor_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index eb21067..775aa25 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,8 +18,10 @@ linters: enable: - copyloopvar - dupl + - dupword - errcheck - gci + - ginkgolinter - goconst - gocyclo - gofmt diff --git a/internal/controller/appwrapper/node_health_monitor_test.go b/internal/controller/appwrapper/node_health_monitor_test.go index 1c8160f..2a3f8cb 100644 --- a/internal/controller/appwrapper/node_health_monitor_test.go +++ b/internal/controller/appwrapper/node_health_monitor_test.go @@ -76,7 +76,7 @@ var _ = Describe("NodeMonitor Controller", func() { Expect(err).NotTo(HaveOccurred()) By("Healthy cluster has no unhealthy nodes") - Expect(len(noExecuteNodes)).Should(Equal(0)) + Expect(noExecuteNodes).Should(BeEmpty()) By("A node labeled EVICT is detected as unhealthy") node := getNode(node1Name.Name) @@ -86,7 +86,7 @@ var _ = Describe("NodeMonitor Controller", func() { Expect(err).NotTo(HaveOccurred()) _, err = nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node2Name}) Expect(err).NotTo(HaveOccurred()) - Expect(len(noExecuteNodes)).Should(Equal(1)) + Expect(noExecuteNodes).Should(HaveLen(1)) Expect(noExecuteNodes).Should(HaveKey(node1Name.Name)) Expect(noExecuteNodes[node1Name.Name]).Should(HaveKey("nvidia.com/gpu")) @@ -95,7 +95,7 @@ var _ = Describe("NodeMonitor Controller", func() { Expect(err).NotTo(HaveOccurred()) _, err = nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node2Name}) Expect(err).NotTo(HaveOccurred()) - Expect(len(noExecuteNodes)).Should(Equal(1)) + Expect(noExecuteNodes).Should(HaveLen(1)) Expect(noExecuteNodes).Should(HaveKey(node1Name.Name)) Expect(noExecuteNodes[node1Name.Name]).Should(HaveKey("nvidia.com/gpu")) @@ -104,7 +104,7 @@ var _ = Describe("NodeMonitor Controller", func() { Expect(k8sClient.Update(ctx, node)).Should(Succeed()) _, err = nodeMonitor.Reconcile(ctx, reconcile.Request{NamespacedName: node1Name}) Expect(err).NotTo(HaveOccurred()) - Expect(len(noExecuteNodes)).Should(Equal(0)) + Expect(noExecuteNodes).Should(BeEmpty()) deleteNode(node1Name.Name) deleteNode(node2Name.Name)