From 1d5dc0b02d1773d96a284396c0b94280ba86bcae Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Fri, 31 Oct 2025 14:11:01 +0000 Subject: [PATCH 1/2] Run pre-commit checksin the gate Adds golangci-lint to the gate and pre-commit hooks. The golangci-lint is a meta-linter built for CI use-case. This patch also: * Migrates the .golangci.yml configuration to version 2 * Updates the Makefile to be able to install the version 2.X of golangci-lint * Fix the current errors in the code Signed-off-by: Lucas Alvares Gomes --- .github/workflows/pre-commit.yaml | 25 +++++++++ .golangci.yml | 55 ++++++++++--------- .pre-commit-config.yaml | 9 +++ Makefile | 4 +- internal/controller/funcs.go | 16 +++--- .../openstacklightspeed_controller.go | 20 +++---- test/utils/utils.go | 4 +- 7 files changed, 86 insertions(+), 47 deletions(-) create mode 100644 .github/workflows/pre-commit.yaml create mode 100644 .pre-commit-config.yaml diff --git a/.github/workflows/pre-commit.yaml b/.github/workflows/pre-commit.yaml new file mode 100644 index 00000000..6a2d3be4 --- /dev/null +++ b/.github/workflows/pre-commit.yaml @@ -0,0 +1,25 @@ +name: Pre-commit checks CI + +on: + push: + branches: + - main + pull_request: + +jobs: + pre-commit-test: + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v5 + + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version: '1.23' + + - name: Run pre-commit hooks + uses: pre-commit/action@v3.0.1 + with: + extra_args: --all-files --show-diff-on-failure diff --git a/.golangci.yml b/.golangci.yml index aac8a13f..9ee1feec 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,33 +1,14 @@ +version: "2" run: - timeout: 5m allow-parallel-runners: true - -issues: - # don't skip warning about doc comments - # don't exclude the default set of lint - exclude-use-default: false - # restore some of the defaults - # (fill in the rest as needed) - exclude-rules: - - path: "api/*" - linters: - - lll - - path: "internal/*" - linters: - - dupl - - lll linters: - disable-all: true + default: none enable: - dupl - errcheck - - exportloopref - ginkgolinter - goconst - gocyclo - - gofmt - - goimports - - gosimple - govet - ineffassign - lll @@ -36,12 +17,34 @@ linters: - prealloc - revive - staticcheck - - typecheck - unconvert - unparam - unused - -linters-settings: - revive: + settings: + revive: + rules: + - name: comment-spacings + exclusions: + generated: lax rules: - - name: comment-spacings + - linters: + - lll + path: api/* + - linters: + - dupl + - lll + path: internal/* + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gofmt + - goimports + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..c59d8a62 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,9 @@ +repos: + - repo: local + hooks: + - id: make-lint + name: make-lint + language: system + entry: make + args: ['lint'] + pass_filenames: false diff --git a/Makefile b/Makefile index 5c358f6c..57c7d38b 100644 --- a/Makefile +++ b/Makefile @@ -252,7 +252,7 @@ GOLANGCI_LINT = $(LOCALBIN)/golangci-lint KUSTOMIZE_VERSION ?= v5.4.2 CONTROLLER_TOOLS_VERSION ?= v0.15.0 ENVTEST_VERSION ?= release-0.18 -GOLANGCI_LINT_VERSION ?= v1.59.1 +GOLANGCI_LINT_VERSION ?= v2.6.0 .PHONY: kustomize kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. @@ -272,7 +272,7 @@ $(ENVTEST): $(LOCALBIN) .PHONY: golangci-lint golangci-lint: $(GOLANGCI_LINT) ## Download golangci-lint locally if necessary. $(GOLANGCI_LINT): $(LOCALBIN) - $(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint,$(GOLANGCI_LINT_VERSION)) + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(LOCALBIN) $(GOLANGCI_LINT_VERSION) # go-install-tool will 'go install' any package with custom target and name of binary, if it doesn't exist # $1 - target path with name of binary diff --git a/internal/controller/funcs.go b/internal/controller/funcs.go index 386de804..a1d29cbb 100644 --- a/internal/controller/funcs.go +++ b/internal/controller/funcs.go @@ -250,9 +250,9 @@ func ResolveIndexID( helper *common_helper.Helper, instance *apiv1beta1.OpenStackLightspeed, ) (string, ctrl.Result, error) { - result, err := createOLSJob(ctx, helper, instance) + err := createOLSJob(ctx, helper, instance) if err != nil { - return "", result, err + return "", ctrl.Result{}, err } podList := &corev1.PodList{} @@ -304,7 +304,9 @@ func extractEnvFromPodLogs(ctx context.Context, pod *corev1.Pod, envVarName stri if err != nil { return "", err } - defer podLogs.Close() + defer func() { + _ = podLogs.Close() + }() buf := new(strings.Builder) _, err = io.Copy(buf, podLogs) @@ -333,7 +335,7 @@ func createOLSJob( ctx context.Context, helper *common_helper.Helper, instance *apiv1beta1.OpenStackLightspeed, -) (ctrl.Result, error) { +) error { imageHash := sha256.Sum256([]byte(instance.Spec.RAGImage)) imageHashStr := fmt.Sprintf("%x", imageHash) imageHashStr = imageHashStr[len(imageHashStr)-9:] @@ -374,15 +376,15 @@ func createOLSJob( } if err := controllerutil.SetControllerReference(instance, OLSPod, helper.GetScheme()); err != nil { - return ctrl.Result{}, err + return err } err := helper.GetClient().Create(ctx, OLSPod) if err != nil && !k8s_errors.IsAlreadyExists(err) { - return ctrl.Result{}, err + return err } - return ctrl.Result{}, nil + return nil } func requeueWaitingPod(helper *common_helper.Helper, instance *apiv1beta1.OpenStackLightspeed) (string, ctrl.Result, error) { diff --git a/internal/controller/openstacklightspeed_controller.go b/internal/controller/openstacklightspeed_controller.go index 35f4e037..fee2a4c6 100644 --- a/internal/controller/openstacklightspeed_controller.go +++ b/internal/controller/openstacklightspeed_controller.go @@ -74,7 +74,7 @@ func (r *OpenStackLightspeedReconciler) Reconcile(ctx context.Context, req ctrl. Log.Info("OpenStackLightspeed Reconciling") instance := &apiv1beta1.OpenStackLightspeed{} - err := r.Client.Get(ctx, req.NamespacedName, instance) + err := r.Get(ctx, req.NamespacedName, instance) if err != nil { if k8s_errors.IsNotFound(err) { Log.Info("OpenStackLightspeed CR not found") @@ -136,7 +136,7 @@ func (r *OpenStackLightspeedReconciler) Reconcile(ctx context.Context, req ctrl. instance.Status.ObservedGeneration = instance.Generation if !instance.DeletionTimestamp.IsZero() { - return r.reconcileDelete(ctx, helper, instance) + return ctrl.Result{}, r.reconcileDelete(ctx, helper, instance) } if instance.DeletionTimestamp.IsZero() && controllerutil.AddFinalizer(instance, helper.GetFinalizer()) { @@ -248,23 +248,23 @@ func (r *OpenStackLightspeedReconciler) reconcileDelete( ctx context.Context, helper *common_helper.Helper, instance *apiv1beta1.OpenStackLightspeed, -) (ctrl.Result, error) { +) error { Log := r.GetLogger(ctx) Log.Info("OpenStackLightspeed Reconciling Delete") olsConfig, err := GetOLSConfig(ctx, helper) if err != nil && k8s_errors.IsNotFound(err) { controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) - return ctrl.Result{}, nil + return nil } else if err != nil { - return ctrl.Result{}, err + return err } ownerLabel := olsConfig.GetLabels()[OpenStackLightspeedOwnerIDLabel] if ownerLabel == "" || ownerLabel != string(instance.GetObjectMeta().GetUID()) { Log.Info("Skipping OLSConfig deletion as it is not managed by the OpenStackLightspeed instance") controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) - return ctrl.Result{}, nil + return nil } _, err = controllerutil.CreateOrPatch(ctx, r.Client, &olsConfig, func() error { @@ -275,18 +275,18 @@ func (r *OpenStackLightspeedReconciler) reconcileDelete( return nil }) if err != nil { - return ctrl.Result{}, err + return err } - err = r.Client.Delete(ctx, &olsConfig) + err = r.Delete(ctx, &olsConfig) if err != nil { - return ctrl.Result{}, err + return err } controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) Log.Info("OpenStackLightspeed Reconciling Delete completed") - return ctrl.Result{}, nil + return nil } // SetupWithManager sets up the controller with the Manager. diff --git a/test/utils/utils.go b/test/utils/utils.go index 87db8781..c136acee 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -22,7 +22,7 @@ import ( "os/exec" "strings" - . "github.com/onsi/ginkgo/v2" //nolint:golint,revive + . "github.com/onsi/ginkgo/v2" //nolint:staticcheck ) const ( @@ -135,6 +135,6 @@ func GetProjectDir() (string, error) { if err != nil { return wd, err } - wd = strings.Replace(wd, "/test/e2e", "", -1) + wd = strings.ReplaceAll(wd, "/test/e2e", "") return wd, nil } From 65f238adc44c3ff24d487c9c0256ac9b2e1ab4ec Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Mon, 3 Nov 2025 15:04:16 +0000 Subject: [PATCH 2/2] Update README on running pre-commit hooks Update the README file on running pre-commit hooks prior to submitting a PR to the project. Signed-off-by: Lucas Alvares Gomes --- README.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/README.md b/README.md index c1c89777..9e1e7b1d 100644 --- a/README.md +++ b/README.md @@ -189,3 +189,39 @@ Use this for quick development and testing. *Attention*: In this mode RBACs are ignored, so when changing those please run the operator in the OpenShift cluster with an image. + +## Quickstart + +### Run OpenShift Cluster + +We'll use CRC and deploy it using the development tools from `install_yamls`. + +Get install-yamls: +```bash +git clone https://github.com/openstack-k8s-operators/install_yamls.git +cd install_yamls/devsetup +make download_tools +``` + +Get pull credentials (pull secret) from `https://cloud.redhat.com/openshift/create/local` +and save it in `pull-secret.txt` of the current path, or you can save it anywhere +and use the `PULL_SECRET` env var to point to it like in the next example. + +## Development + +### Running Pre-Commit Hooks + +To ensure code quality and consistency, run pre-commit hooks locally before +submitting a pull request. + +Install hooks: + +```bash +pre-commit install +``` + +Run all hooks manually: + +```bash +pre-commit run --all-files +```