Merge pull request #3321 from apostasie/b-dev-3320-canary-lint

Add linter to Canary and fix go 1.23 linting issues
containerd · Aug 17, 2024 · ef6c80b · ef6c80b
2 parents 7dd3050 + 787b4ac
commit ef6c80b
Show file tree

Hide file tree

Showing 13 changed files with 152 additions and 136 deletions.
diff --git a/.github/workflows/test-canary.yml b/.github/workflows/test-canary.yml
@@ -15,6 +15,27 @@ env:
   GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
 jobs:
+  lint:
+    runs-on: "ubuntu-24.04"
+    timeout-minutes: 20
+    steps:
+      - uses: actions/checkout@v4.1.7
+        with:
+          fetch-depth: 1
+      - name: Set GO env
+        run: |
+          . ./hack/build-integration-canary.sh
+          canary::golang::latest
+      - uses: actions/setup-go@v5
+        with:
+          go-version: ${{ env.GO_VERSION }}
+          check-latest: true
+          cache: true
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v6.1.0
+        with:
+          args: --verbose
+
   linux:
     runs-on: "ubuntu-24.04"
     timeout-minutes: 40
@@ -24,7 +45,8 @@ jobs:
           fetch-depth: 1
       - name: "Prepare integration test environment"
         run: |
-          ./hack/build-integration-canary.sh
+          . ./hack/build-integration-canary.sh
+          canary::build::integration
       - name: "Remove snap loopback devices (conflicts with our loopback devices in TestRunDevice)"
         run: |
           sudo systemctl disable --now snapd.service snapd.socket
@@ -53,40 +75,21 @@ jobs:
       run:
         shell: bash
     steps:
+      - uses: actions/checkout@v4.1.7
+        with:
+          fetch-depth: 1
       - name: Set GO env
         run: |
-          # Enable extended globbing features to use advanced pattern matching
-          shopt -s extglob
-
           # Get latest containerd
           args=(curl --proto '=https' --tlsv1.2 -fsSL -H "Accept: application/vnd.github+json" -H "X-GitHub-Api-Version: 2022-11-28")
           [ "${GITHUB_TOKEN:-}" == "" ] && {
             >&2 printf "GITHUB_TOKEN is not set - you might face rate limitations with the Github API\n"
           } || args+=(-H "Authorization: Bearer $GITHUB_TOKEN")
           ctd_v="$("${args[@]}" https://api.github.com/repos/containerd/containerd/tags | jq -rc .[0].name)"
-          echo "CONTAINERD_VERSION=${ctd_v:1}" >> $GITHUB_ENV
-
-          # Get latest golang version and split it in components
-          norm=()
-          while read -r line; do
-            line_trimmed="${line//+([[:space:]])/}"
-            norm+=("$line_trimmed")
-          done < \
-            <(sed -E 's/^go([0-9]+)[.]([0-9]+)([.]([0-9]+))?(([a-z]+)([0-9]+))?/\1.\2\n\4\n\6\n\7/i' \
-              <(curl -fsSL "https://go.dev/dl/?mode=json&include=all" | jq -rc .[0].version) \
-            )
+          echo "CONTAINERD_VERSION=${ctd_v:1}" >> "$GITHUB_ENV"
 
-          # Serialize version, making sure we have a patch version, and separate possible rcX into .rc-X
-          [ "${norm[1]}" != "" ] || norm[1]="0"
-          norm[1]=".${norm[1]}"
-          [ "${norm[2]}" == "" ] || norm[2]="-${norm[2]}"
-          [ "${norm[3]}" == "" ] || norm[3]=".${norm[3]}"
-          # Save it
-          IFS=
-          echo "GO_VERSION=${norm[*]}" >> $GITHUB_ENV
-      - uses: actions/checkout@v4.1.7
-        with:
-          fetch-depth: 1
+          . ./hack/build-integration-canary.sh
+          canary::golang::latest
       - uses: actions/setup-go@v5
         with:
           go-version: ${{ env.GO_VERSION }}

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -222,7 +222,7 @@ jobs:
           if echo "${ROOTLESSKIT_VERSION}" | grep -q v1; then
             WORKAROUND_ISSUE_622=1
           fi
-          echo "WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622}" >>$GITHUB_ENV
+          echo "WORKAROUND_ISSUE_622=${WORKAROUND_ISSUE_622}" >> "$GITHUB_ENV"
       - name: "Test (network driver=slirp4netns, port driver=builtin)"
         uses: nick-fields/retry@v3
         with:

diff --git a/cmd/nerdctl/compose_exec_linux_test.go b/cmd/nerdctl/compose_exec_linux_test.go
@@ -20,7 +20,6 @@ import (
 	"errors"
 	"fmt"
 	"net"
-	"runtime"
 	"strings"
 	"testing"
 
@@ -98,7 +97,7 @@ services:
 		if !strings.Contains(stdout, "\nBAR=bar1 bar2\n") {
 			return errors.New("got bad BAR")
 		}
-		if !strings.Contains(stdout, "\nBAZ=\n") && runtime.GOOS != "windows" {
+		if !strings.Contains(stdout, "\nBAZ=\n") {
 			return errors.New("got bad BAZ")
 		}
 		if strings.Contains(stdout, "QUX") {
@@ -113,10 +112,10 @@ services:
 		if !strings.Contains(stdout, "\nGRAULT=grault_key=grault_value\n") {
 			return errors.New("got bad GRAULT")
 		}
-		if !strings.Contains(stdout, "\nGARPLY=\n") && runtime.GOOS != "windows" {
+		if !strings.Contains(stdout, "\nGARPLY=\n") {
 			return errors.New("got bad GARPLY")
 		}
-		if !strings.Contains(stdout, "\nWALDO=\n") && runtime.GOOS != "windows" {
+		if !strings.Contains(stdout, "\nWALDO=\n") {
 			return errors.New("got bad WALDO")
 		}
 

diff --git a/cmd/nerdctl/container_create_linux_test.go b/cmd/nerdctl/container_create_linux_test.go
@@ -18,7 +18,6 @@ package main
 
 import (
 	"fmt"
-	"runtime"
 	"strings"
 	"testing"
 
@@ -154,9 +153,6 @@ func TestCreateWithMACAddress(t *testing.T) {
 }
 
 func TestCreateWithTty(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("json-file log driver is not yet implemented on Windows")
-	}
 	base := testutil.NewBase(t)
 	imageName := testutil.CommonImage
 	withoutTtyContainerName := "without-terminal-" + testutil.Identifier(t)

diff --git a/cmd/nerdctl/container_run_linux_test.go b/cmd/nerdctl/container_run_linux_test.go
@@ -26,7 +26,6 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strconv"
 	"strings"
 	"syscall"
@@ -400,9 +399,6 @@ func TestRunSigProxy(t *testing.T) {
 }
 
 func TestRunWithFluentdLogDriver(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("fluentd log driver is not yet implemented on Windows")
-	}
 	base := testutil.NewBase(t)
 	tempDirectory := t.TempDir()
 	err := os.Chmod(tempDirectory, 0777)
@@ -432,9 +428,6 @@ func TestRunWithFluentdLogDriver(t *testing.T) {
 }
 
 func TestRunWithFluentdLogDriverWithLogOpt(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("fluentd log driver is not yet implemented on Windows")
-	}
 	base := testutil.NewBase(t)
 	tempDirectory := t.TempDir()
 	err := os.Chmod(tempDirectory, 0777)

diff --git a/cmd/nerdctl/image_convert_linux_test.go b/cmd/nerdctl/image_convert_linux_test.go
@@ -18,7 +18,6 @@ package main
 
 import (
 	"fmt"
-	"runtime"
 	"testing"
 
 	"gotest.tools/v3/icmd"
@@ -29,9 +28,6 @@ import (
 )
 
 func TestImageConvertNydus(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("no windows support yet")
-	}
 	testutil.RequireExecutable(t, "nydus-image")
 	testutil.DockerIncompatible(t)
 

diff --git a/hack/build-integration-canary.sh b/hack/build-integration-canary.sh
@@ -215,91 +215,118 @@ assets::get(){
 # Script
 ######################
 
-docker_args=(docker build -t test-integration --target test-integration)
-
-for dep in "${dependencies[@]}"; do
-  shortname="${dep##*/}"
-  [ "$shortname" != "plugins" ] || shortname="cni-plugins"
-  [ "$shortname" != "fuse-overlayfs-snapshotter" ] || shortname="containerd-fuse-overlayfs"
-  for bl in "${blacklist[@]}"; do
-    if [ "$bl" == "$shortname" ]; then
-      log::warning "Dependency $shortname is blacklisted and will be left to its currently pinned version"
-      break
-    fi
-  done
-  [ "$bl" != "$shortname" ] || continue
-
-  shortsafename="$(printf "%s" "$shortname" | tr '[:lower:]' '[:upper:]' | tr '-' '_')"
-
-  exclusion="${shortsafename}_EXCLUDE"
-  latest::release "$dep" "${!exclusion:-}"
-
-  # XXX containerd does not display "v" in its released versions
-  [ "${higher_readable:0:1}" == v ] || higher_readable="v$higher_readable"
-
-  checksum="${shortsafename}_CHECKSUM"
-  if [ "${!checksum:-}" != "" ]; then
-    # Checksum file
-    checksum_file=./Dockerfile.d/SHA256SUMS.d/"${shortname}-${higher_readable}"
-    if [ ! -e "$checksum_file" ]; then
-      # Get assets - try first os/arch - fallback on gnu style arch otherwise
-      assets=()
-
-      # Most well behaved go projects will tag with a go os and arch
-      candidate="$(assets::get "${!checksum:-}" "amd64")"
-      # Then non go projects tend to use gnu style
-      [ "$candidate" != "" ] || candidate="$(assets::get "" "x86_64")"
-      # And then some projects which are linux only do not specify the OS
-      [ "$candidate" != "" ] || candidate="$(assets::get "" "amd64")"
-      [ "$candidate" == "" ] || assets+=("$candidate")
-
-      candidate="$(assets::get "${!checksum:-}" "arm64")"
-      [ "$candidate" != "" ] || candidate="$(assets::get "" "aarch64")"
-      [ "$candidate" != "" ] || candidate="$(assets::get "" "arm64")"
-      [ "$candidate" == "" ] || assets+=("$candidate")
-      # Fallback to source if there is nothing else
-
-      [ "${#assets[@]}" != 0 ] || candidate="$(assets::get "" "source")"
-      [ "$candidate" == "" ] || assets+=("$candidate")
-
-      # XXX very special...
-      if [ "$shortsafename" == "STARGZ_SNAPSHOTTER" ]; then
-        assets+=("https://raw.githubusercontent.com/containerd/stargz-snapshotter/${higher_readable}/script/config/etc/systemd/system/stargz-snapshotter.service")
+canary::build::integration(){
+  docker_args=(docker build -t test-integration --target test-integration)
+
+  for dep in "${dependencies[@]}"; do
+    shortname="${dep##*/}"
+    [ "$shortname" != "plugins" ] || shortname="cni-plugins"
+    [ "$shortname" != "fuse-overlayfs-snapshotter" ] || shortname="containerd-fuse-overlayfs"
+    for bl in "${blacklist[@]}"; do
+      if [ "$bl" == "$shortname" ]; then
+        log::warning "Dependency $shortname is blacklisted and will be left to its currently pinned version"
+        break
       fi
-
-      # Write the checksum for what we found
-      if [ "${#assets[@]}" == 0 ]; then
-        log::error "No asset found for this checksum-able dependency. Dropping off."
-        exit 1
+    done
+    [ "$bl" != "$shortname" ] || continue
+
+    shortsafename="$(printf "%s" "$shortname" | tr '[:lower:]' '[:upper:]' | tr '-' '_')"
+
+    exclusion="${shortsafename}_EXCLUDE"
+    latest::release "$dep" "${!exclusion:-}"
+
+    # XXX containerd does not display "v" in its released versions
+    [ "${higher_readable:0:1}" == v ] || higher_readable="v$higher_readable"
+
+    checksum="${shortsafename}_CHECKSUM"
+    if [ "${!checksum:-}" != "" ]; then
+      # Checksum file
+      checksum_file=./Dockerfile.d/SHA256SUMS.d/"${shortname}-${higher_readable}"
+      if [ ! -e "$checksum_file" ]; then
+        # Get assets - try first os/arch - fallback on gnu style arch otherwise
+        assets=()
+
+        # Most well behaved go projects will tag with a go os and arch
+        candidate="$(assets::get "${!checksum:-}" "amd64")"
+        # Then non go projects tend to use gnu style
+        [ "$candidate" != "" ] || candidate="$(assets::get "" "x86_64")"
+        # And then some projects which are linux only do not specify the OS
+        [ "$candidate" != "" ] || candidate="$(assets::get "" "amd64")"
+        [ "$candidate" == "" ] || assets+=("$candidate")
+
+        candidate="$(assets::get "${!checksum:-}" "arm64")"
+        [ "$candidate" != "" ] || candidate="$(assets::get "" "aarch64")"
+        [ "$candidate" != "" ] || candidate="$(assets::get "" "arm64")"
+        [ "$candidate" == "" ] || assets+=("$candidate")
+        # Fallback to source if there is nothing else
+
+        [ "${#assets[@]}" != 0 ] || candidate="$(assets::get "" "source")"
+        [ "$candidate" == "" ] || assets+=("$candidate")
+
+        # XXX very special...
+        if [ "$shortsafename" == "STARGZ_SNAPSHOTTER" ]; then
+          assets+=("https://raw.githubusercontent.com/containerd/stargz-snapshotter/${higher_readable}/script/config/etc/systemd/system/stargz-snapshotter.service")
+        fi
+
+        # Write the checksum for what we found
+        if [ "${#assets[@]}" == 0 ]; then
+          log::error "No asset found for this checksum-able dependency. Dropping off."
+          exit 1
+        fi
+        http::checksum "${assets[@]}" > "$checksum_file"
       fi
-      http::checksum "${assets[@]}" > "$checksum_file"
     fi
-  fi
 
-  while read -r line; do
-    # Extract value after "=" from a possible dockerfile `ARG XXX_VERSION`
-    old_version=$(echo "$line" | grep "ARG ${shortsafename}_VERSION=") || true
-    old_version="${old_version##*=}"
-    [ "$old_version" != "" ] || continue
-    # If the Dockerfile version does NOT start with a v, adapt to that
-    [ "${old_version:0:1}" == "v" ] || higher_readable="${higher_readable:1}"
-
-    if [ "$old_version" != "$higher_readable" ]; then
-      log::warning "Dependency ${shortsafename} is going to use an updated version $higher_readable (currently: $old_version)"
-    fi
-  done < ./Dockerfile
+    while read -r line; do
+      # Extract value after "=" from a possible dockerfile `ARG XXX_VERSION`
+      old_version=$(echo "$line" | grep "ARG ${shortsafename}_VERSION=") || true
+      old_version="${old_version##*=}"
+      [ "$old_version" != "" ] || continue
+      # If the Dockerfile version does NOT start with a v, adapt to that
+      [ "${old_version:0:1}" == "v" ] || higher_readable="${higher_readable:1}"
 
-  docker_args+=(--build-arg "${shortsafename}_VERSION=$higher_readable")
-done
+      if [ "$old_version" != "$higher_readable" ]; then
+        log::warning "Dependency ${shortsafename} is going to use an updated version $higher_readable (currently: $old_version)"
+      fi
+    done < ./Dockerfile
+
+    docker_args+=(--build-arg "${shortsafename}_VERSION=$higher_readable")
+  done
 
 
-GO_VERSION="$(curl -fsSL "https://go.dev/dl/?mode=json&include=all" | jq -rc .[0].version)"
-GO_VERSION="${GO_VERSION##*go}"
-# If a release candidate, docker hub may not have the corresponding image yet.
-# So, soften the version to just "rc", as they provide that as an alias to the latest available rc on their side
-# See https://github.com/containerd/nerdctl/issues/3223
-! grep -Eq "rc[0-9]+$" <<<"$GO_VERSION" || GO_VERSION="${GO_VERSION%rc[0-9]*}-rc"
-docker_args+=(--build-arg "GO_VERSION=$GO_VERSION")
+  GO_VERSION="$(curl -fsSL "https://go.dev/dl/?mode=json&include=all" | jq -rc .[0].version)"
+  GO_VERSION="${GO_VERSION##*go}"
+  # If a release candidate, docker hub may not have the corresponding image yet.
+  # So, soften the version to just "rc", as they provide that as an alias to the latest available rc on their side
+  # See https://github.com/containerd/nerdctl/issues/3223
+  ! grep -Eq "rc[0-9]+$" <<<"$GO_VERSION" || GO_VERSION="${GO_VERSION%rc[0-9]*}-rc"
+  docker_args+=(--build-arg "GO_VERSION=$GO_VERSION")
+
+  log::debug "${docker_args[*]} ."
+  "${docker_args[@]}" "."
+}
+
 
-log::debug "${docker_args[*]} ."
-"${docker_args[@]}" "."
+canary::golang::latest(){
+    # Enable extended globbing features to use advanced pattern matching
+    shopt -s extglob
+
+    # Get latest golang version and split it in components
+    norm=()
+    while read -r line; do
+      line_trimmed="${line//+([[:space:]])/}"
+      norm+=("$line_trimmed")
+    done < \
+      <(sed -E 's/^go([0-9]+)[.]([0-9]+)([.]([0-9]+))?(([a-z]+)([0-9]+))?/\1.\2\n\4\n\6\n\7/i' \
+        <(curl -fsSL "https://go.dev/dl/?mode=json&include=all" | jq -rc .[0].version) \
+      )
+
+    # Serialize version, making sure we have a patch version, and separate possible rcX into .rc-X
+    [ "${norm[1]}" != "" ] || norm[1]="0"
+    norm[1]=".${norm[1]}"
+    [ "${norm[2]}" == "" ] || norm[2]="-${norm[2]}"
+    [ "${norm[3]}" == "" ] || norm[3]=".${norm[3]}"
+    # Save it
+    IFS=
+    echo "GO_VERSION=${norm[*]}" >> "$GITHUB_ENV"
+}