Improve Github Actions Workflow and GoLint (#4672)

jamlo · web-flow · commit 520e46b114e4 · 2024-10-29T11:42:49.000-04:00
### Details: This PR improves the Go-Linting structure in our CI, as well as move the Github Actions workflows into the "Pipeline" mentality, rather than individual Workflows. Changes: 1. Currently Go linting is tightly coupled with pre-commits, which make things very complex and error prone. This commit removes Go linting from any pre-commit logic, and only have it as a first class citizen in Github Actions. 2. Create the Notion of a pipeline in github actions, and not just standalone workflows. One step towards migrating from BuildKite 3. Use CI built binaries in Test Containers Suite. ### Github actions pipeline will look like this: ![Screenshot 2024-10-28 at 6 00 11 PM](https://github.com/user-attachments/assets/9f927e69-4d9a-4a4a-b021-9ee5eb1df143)
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -0,0 +1,77 @@
+name: Build Bacalhau Binaries
+
+on:
+  workflow_call:
+
+jobs:
+  build:
+    name: Build Binary
+    runs-on: ubuntu-latest
+    outputs:
+      status: ${{ job.status }}
+    strategy:
+      matrix:
+        include:
+          - os: linux
+            goarch: amd64
+          - os: linux
+            goarch: arm64
+          - os: linux
+            goarch: armv7
+          - os: linux
+            goarch: armv6
+          - os: darwin
+            goarch: amd64
+          - os: darwin
+            goarch: arm64
+          - os: windows
+            goarch: amd64
+
+    steps:
+      - name: Install earthly
+        uses: earthly/actions-setup@v1
+
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: "0" # Need to fetch all due to how bacalhau constructs semver
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: go.work
+
+      - name: Build
+        env:
+          GOOS: ${{ matrix.os }}
+          GOARCH: ${{ matrix.goarch }}
+          PRIVATE_PEM: ${{ secrets.PRIVATE_PEM }}
+          PUBLIC_PEM: ${{ secrets.PUBLIC_PEM }}
+          PRIVATE_KEY_PASSPHRASE: ${{ secrets.PRIVATE_KEY_PASSPHRASE }}
+        run: |
+          # Add Keys to expected files
+          echo "${PRIVATE_PEM}" > /tmp/private.pem && chmod 600 /tmp/private.pem
+          echo "${PUBLIC_PEM}" > /tmp/public.pem && chmod 600 /tmp/public.pem
+          
+          # Start build
+          echo "==> Building bacalhau binary for: ${GOOS} ${GOARCH}..."
+          make build-bacalhau-tgz
+          echo "===> Done building bacalhau binary."
+          
+          # Listing Builds
+          echo "===> Built Artifacts:"
+          ls -lh dist/
+          
+          # Remove keys, good security practice
+          rm /tmp/private.pem /tmp/public.pem
+
+      - name: Upload binary artifacts
+        uses: actions/upload-artifact@v4
+        with:
+          name: ${{ matrix.os }}-${{ matrix.goarch }}
+          path: "dist/bacalhau_*"
+          retention-days: 1  # Short retention since these are intermediate artifacts, also save money
+
+      - name: Report build status
+        if: always()
+        run: |
+          echo "Build completed for ${{ matrix.os }}-${{ matrix.goarch }}"
+          echo "Status: ${{ job.status }}"
diff --git a/.github/workflows/cspell.yml b/.github/workflows/cspell.yml
@@ -2,12 +2,7 @@
 name: Codespell
 
 on:
-  push:
-    branches: [main]
-  pull_request:
-    branches: [main]
-  repository_dispatch:
-    types: [ok-to-test]
+  workflow_call:
 
 jobs:
   codespell:
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -1,13 +1,32 @@
 name: Lints
 
 on:
-  pull_request:
-  push:
-    branches: [main]
-  repository_dispatch:
-    types: [ok-to-test]
+  workflow_call:
 
 jobs:
+  # A separate Go-Lint job since the current lint job is heavily
+  # coupled with a lot of tooling, which will be removed and
+  # simplified, one step at a time.
+  # For now, we are just extracting the go-lint.
+  go-lint:
+    strategy:
+      matrix:
+        os: [ ubuntu-latest ]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: go.work
+      - name: Setup Dummy WebUi Build
+        if: matrix.os == 'ubuntu-latest'
+        run: |
+          mkdir -p webui/build && touch webui/build/index.html
+      - uses: golangci/golangci-lint-action@v6
+        with:
+          skip-cache: true
+
+  # TODO: Decouple Each Component Linting
   lint:
     runs-on: ubuntu-latest
     steps:
diff --git a/.github/workflows/pipeline-main.yml b/.github/workflows/pipeline-main.yml
@@ -0,0 +1,31 @@
+name: Main Pipeline
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+    branches: [main]
+  repository_dispatch:
+    types: [ok-to-test]
+
+jobs:
+  build:
+    uses: ./.github/workflows/build.yml
+    secrets: inherit
+
+  lint:
+    uses: ./.github/workflows/lint.yml
+
+  swagger-validation:
+    uses: ./.github/workflows/swagger-validation.yml
+
+  cspell:
+    uses: ./.github/workflows/cspell.yml
+
+  testcontainers-suite:
+    needs: [build, lint, swagger-validation, cspell]
+    if: success()
+    uses: ./.github/workflows/testcontainers-suite.yml
+    with:
+      os: linux
+      arch: amd64
diff --git a/.github/workflows/swagger-validation.yml b/.github/workflows/swagger-validation.yml
@@ -1,9 +1,7 @@
 name: Swagger Validation
 
 on:
-  pull_request:
-    branches:
-      - main
+  workflow_call:
 
 jobs:
   ensure-up-to-date:
@@ -17,50 +15,31 @@ jobs:
         with:
           fetch-depth: 1
 
-      # Detect Go version from go.mod
-      - name: Detect Go version from go.mod
-        id: detect-go-version
-        shell: bash
-        run: |
-          set -euo pipefail
-          go_version=$(grep '^go ' go.mod | awk '{print $2}')
-          echo "Go version detected: $go_version"
-          echo "golang-version=$go_version" >> $GITHUB_ENV
-
-      # Setup Go using the detected version
       - name: Setup Go
         uses: actions/setup-go@v5
         with:
-          go-version: ${{ env.golang-version }}
+          go-version-file: go.work
           cache: true
 
       # Install swag (Swagger generator)
       - name: Install Swag
-        shell: bash
         run: |
-          set -euo pipefail
           go install github.com/swaggo/swag/cmd/swag@latest
 
       # Verify swag installation
       - name: Verify Swag Installation
-        shell: bash
         run: |
-          set -euo pipefail
           if ! command -v swag &> /dev/null; then
             echo "Swag is not installed. Please ensure Go is properly configured and Swag is installed."
             exit 1
           fi
 
       # Generate the swagger.json
       - name: Generate Swagger file
-        shell: bash
         run: |
-          set -euo pipefail
           make generate-swagger
 
       # Compare the newly generated swagger.json with the committed swagger.json
       - name: Check for Swagger differences
-        shell: bash
         run: |
-          set -euo pipefail
           git diff --exit-code pkg/swagger/swagger.json || (echo "Swagger is outdated. Please regenerate it with 'make generate-swagger' and commit the changes." && exit 1)
diff --git a/.github/workflows/testcontainers-integration-tests.yml b/.github/workflows/testcontainers-integration-tests.yml
diff --git a/.github/workflows/testcontainers-suite.yml b/.github/workflows/testcontainers-suite.yml
@@ -0,0 +1,41 @@
+name: TestContainers Suite
+
+on:
+  workflow_call:
+    inputs:
+      os:
+        required: true
+        type: string
+      arch:
+        required: true
+        type: string
+
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Download compiled binary
+        uses: actions/download-artifact@v4
+        with:
+          name: ${{ inputs.os }}-${{ inputs.arch }}
+          path: compiled-artifacts/
+
+      - name: Install Golang
+        uses: actions/setup-go@v5
+        with:
+          go-version-file: go.work
+
+      - name: Run Go tests in integration test directory
+        shell: bash
+        run: |
+          echo "===> Copy compiled artifact to use in docker images..."
+          cd compiled-artifacts
+          tar -xzf bacalhau_*.tar.gz
+          cp ./bacalhau ../test_integration/common_assets/bacalhau_bin
+          
+          echo "===> Running tests..."       
+          cd ../test_integration
+          go test -v -count=1 ./...
diff --git a/.github/workflows/trigger-development-deployment.yaml b/.github/workflows/trigger-development-deployment.yaml
diff --git a/.gitprecommit/golangci-lint.sh b/.gitprecommit/golangci-lint.sh
diff --git a/.golangci.yml b/.golangci.yml
@@ -69,7 +69,7 @@ linters:
   - depguard
   - dogsled
   - errcheck
-  - exportloopref
+  - copyloopvar
   - funlen
   - gochecknoinits
   - goconst
@@ -100,7 +100,7 @@ run:
   # concurrency: 16
   # Timeout for analysis, e.g. 30s, 5m.
   # Default: 1m
-  timeout: 5m
+  timeout: 10m
   # Exit code when at least one issue was found.
   # Default: 1
   issues-exit-code: 2
diff --git a/buildkite/scripts/lint.sh b/buildkite/scripts/lint.sh
@@ -6,9 +6,6 @@ set -x
 # NB(forrest/udit): this step needs to be done before linting as without it the code doesn't compile since webuid/build DNE.
 make build-webui
 
-# NB(forrest/udit): linting cannot be done by pre-commit because it doesn't work...
-make lint
-
 # TODO(forrest/udit): deprecate pre-commit and replace it with the individual steps required to validate the code.
 # e.g. modtidy check, credentials check, go fmt, test file header validation.
 pre-commit run --show-diff-on-failure --color=always --all-files
diff --git a/cmd/util/flags/types.go b/cmd/util/flags/types.go
@@ -96,7 +96,6 @@ func (s *ArrayValueFlag[T]) Set(input string) error {
 func (s *ArrayValueFlag[T]) String() string {
 	strs := make([]string, 0, len(*s.value))
 	for _, spec := range *s.value {
-		spec := spec
 		strs = append(strs, s.stringer(&spec))
 	}
 	return strings.Join(strs, ", ")
@@ -152,7 +151,6 @@ func (s *MapValueFlag[K, V]) Set(input string) error {
 func (s *MapValueFlag[K, V]) String() string {
 	strs := make([]string, len(*s.value))
 	for key, value := range *s.value {
-		key, value := key, value
 		strs = append(strs, s.stringer(&key, &value))
 	}
 	return strings.Join(strs, ", ")
diff --git a/pkg/executor/results.go b/pkg/executor/results.go
@@ -139,7 +139,6 @@ func WriteJobResults(
 
 	wg := multierrgroup.Group{}
 	for _, output := range outputs {
-		output := output
 		wg.Go(func() error {
 			return writeOutputResult(resultsDir, output)
 		})
diff --git a/pkg/test/scenario/results.go b/pkg/test/scenario/results.go
@@ -132,7 +132,6 @@ func ManyChecks(checks ...CheckResults) CheckResults {
 	return func(resultsDir string) error {
 		var wg multierrgroup.Group
 		for _, check := range checks {
-			check := check
 			wg.Go(func() error { return check(resultsDir) })
 		}
 		return wg.Wait()
diff --git a/test_integration/README.md b/test_integration/README.md
@@ -38,5 +38,5 @@ Then:
 
 ```shell
 cd test_integration
-go test -v -count=1 ./...
+./run_locally.sh
 ```
diff --git a/test_integration/init_test.go b/test_integration/init_test.go
diff --git a/test_integration/run_locally.sh b/test_integration/run_locally.sh
diff --git a/test_integration/utils/test_utils.go b/test_integration/utils/test_utils.go

Original file line number	Diff line number	Diff line change
`@@ -132,7 +132,6 @@ func ManyChecks(checks ...CheckResults) CheckResults {`
`132`	`132`	`return func(resultsDir string) error {`
`133`	`133`	`var wg multierrgroup.Group`
`134`	`134`	`for _, check := range checks {`
`135`		`- check := check`
`136`	`135`	`wg.Go(func() error { return check(resultsDir) })`
`137`	`136`	`}`
`138`	`137`	`return wg.Wait()`