Merge pull request #1358 from aptly-dev/improve/grab-downloader

improve and test grab downloader

.github/workflows/ci.yml

@@ -18,7 +18,7 @@ env:
 
 jobs:
   test:
-    name: "System Tests (Ubuntu 22.04)"
+    name: "Test (Ubuntu 22.04)"
     runs-on: ubuntu-22.04
     continue-on-error: false
     timeout-minutes: 30
@@ -33,14 +33,18 @@ jobs:
       - name: "Install packages"
         run: |
           sudo apt-get update
-          sudo apt-get install -y --no-install-recommends graphviz gnupg2 gpgv2 git gcc make devscripts
+          sudo apt-get install -y --no-install-recommends graphviz gnupg2 gpgv2 git gcc make devscripts python3 python3-requests-unixsocket python3-termcolor python3-swiftclient python3-boto python3-azure-storage python3-etcd3 python3-plyvel flake8
 
       - name: "Checkout repository"
         uses: actions/checkout@v4
         with:
           # fetch the whole repo for `git describe` to work
           fetch-depth: 0
 
+      - name: "Run flake8"
+        run: |
+          make flake8
+
       - name: "Read go version from go.mod"
         run: |
           gover=$(sed -n 's/^go \(.*\)/\1/p' go.mod)
@@ -53,23 +57,28 @@ jobs:
         with:
           go-version: ${{ steps.goversion.outputs.GOVER }}
 
-      - name: "Setup Python"
-        uses: actions/setup-python@v4
-        with:
-          python-version: '3.11'
-
-      - name: "Install Python packages"
-        run: |
-          pip install six packaging appdirs virtualenv
-          pip install -U pip setuptools
-          pip install -r system/requirements.txt
-
       - name: "Install Azurite"
         id: azuright
         uses: potatoqualitee/azuright@v1.1
         with:
           directory: ${{ runner.temp }}
 
+      - name: "Run Unit Tests"
+        env:
+          RUN_LONG_TESTS: 'yes'
+          AZURE_STORAGE_ENDPOINT: "http://127.0.0.1:10000/devstoreaccount1"
+          AZURE_STORAGE_ACCOUNT: "devstoreaccount1"
+          AZURE_STORAGE_ACCESS_KEY: "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw=="
+          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
+          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
+        run: |
+          sudo mkdir -p /srv ; sudo chown runner /srv
+          COVERAGE_DIR=${{ runner.temp }} make test
+
+      - name: "Run Benchmark"
+        run: |
+          COVERAGE_DIR=${{ runner.temp }} make bench
+
       - name: "Run System Tests"
         env:
           RUN_LONG_TESTS: 'yes'
@@ -80,7 +89,7 @@ jobs:
           AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
         run: |
           sudo mkdir -p /srv ; sudo chown runner /srv
-          COVERAGE_DIR=${{ runner.temp }} make all
+          COVERAGE_DIR=${{ runner.temp }} make system-test
 
       - name: "Merge code coverage"
         run: |

.gitignore

@@ -43,9 +43,8 @@ aptly.test
 
 build/
 
-pgp/keyrings/aptly2*.gpg
-pgp/keyrings/aptly2*.gpg~
-pgp/keyrings/.#*
+system/files/aptly2.gpg~
+system/files/aptly2_passphrase.gpg~
 
 *.creds
 

Makefile

@@ -17,8 +17,6 @@ RELEASE=no
 help:  ## Print this help
 	@grep -E '^[a-zA-Z][a-zA-Z0-9_-]*:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
 
-all: prepare test bench check system-test  # used for system tests in ci
-
 prepare:  ## Install go module dependencies
 	# set version
 	@make version > VERSION
@@ -31,24 +29,12 @@ prepare:  ## Install go module dependencies
 	go mod tidy -v
 	go generate
 
-check: system/env
-ifeq ($(RUN_LONG_TESTS), yes)
-	system/env/bin/flake8
-endif
-
 install:
 	@echo "\e[33m\e[1mBuilding aptly ...\e[0m"
 	go generate
 	@out=`mktemp`; if ! go install -v > $$out 2>&1; then cat $$out; rm -f $$out; echo "\nBuild failed\n"; exit 1; else rm -f $$out; fi
 
-system/env: system/requirements.txt
-ifeq ($(RUN_LONG_TESTS), yes)
-	rm -rf system/env
-	$(PYTHON) -m venv system/env
-	system/env/bin/pip install -r system/requirements.txt
-endif
-
-system-test: install system/env  ## Run system tests in github CI
+system-test: install ## Run system tests in github CI
 ifeq ($(RUN_LONG_TESTS), yes)
 	go generate
 	# install etcd
@@ -61,7 +47,7 @@ ifeq ($(RUN_LONG_TESTS), yes)
 	if [ ! -e ~/aptly-fixture-db ]; then git clone https://github.com/aptly-dev/aptly-fixture-db.git ~/aptly-fixture-db/; fi
 	if [ ! -e ~/aptly-fixture-pool ]; then git clone https://github.com/aptly-dev/aptly-fixture-pool.git ~/aptly-fixture-pool/; fi
 	cd /home/runner; curl -O http://repo.aptly.info/system-tests/etcd.db.xz; xz -d etcd.db.xz
-	PATH=$(BINPATH)/:$(PATH) && . system/env/bin/activate && APTLY_VERSION=$(VERSION) FORCE_COLOR=1 $(PYTHON) system/run.py --long $(TESTS) --coverage-dir $(COVERAGE_DIR) $(CAPTURE)
+	PATH=$(BINPATH)/:$(PATH) && APTLY_VERSION=$(VERSION) FORCE_COLOR=1 $(PYTHON) system/run.py --long $(TESTS) --coverage-dir $(COVERAGE_DIR) $(CAPTURE)
 endif
 
 docker-test: ## Run system tests
@@ -203,7 +189,7 @@ docker-deb:  ## Build debian packages in docker container
 docker-unit-tests:  ## Run unit tests in docker container
 	@docker run -it --rm -v ${PWD}:/app aptly-dev /app/system/run-unit-tests
 
-docker-system-tests:  ## Run system tests in docker container (add TEST=t04_mirror to run only specific tests)
+docker-system-tests:  ## Run system tests in docker container (add TEST=t04_mirror or TEST=UpdateMirror26Test to run only specific tests)
 	@docker run -it --rm -v ${PWD}:/app aptly-dev /app/system/run-system-tests $(TEST)
 
 docker-dev-server:  ## Run development server (auto recompiling) on http://localhost:3142
@@ -223,4 +209,4 @@ clean:  ## remove local build and module cache
 	rm -rf build/ docs/ obj-*-linux-gnu*
 	rm -f unit.out aptly.test
 
-.PHONY: help man prepare version binaries docker-release docker-system-tests docker-unit-tests docker-lint docker-build docker-image build docker-shell clean releasetype dpkg dev-server docker-dev-server
+.PHONY: help man prepare version binaries docker-release docker-system-tests docker-unit-tests docker-lint docker-build docker-image build docker-shell clean releasetype dpkg dev-server docker-dev-server flake8

http/grab.go

@@ -59,14 +59,13 @@ func (d *GrabDownloader) DownloadWithChecksum(ctx context.Context, url string, d
 			// Success
 			break
 		}
-		d.log("Download Error: %v\n", err)
 		if retryableError(err) {
 			maxTries--
-			d.log("Retrying download %s: %d\n", url, maxTries)
+			d.log("Retrying %d %s\n", maxTries, url)
 			time.Sleep(delay)
 		} else {
 			// Can't retry
-			d.log("Cannot retry download %s\n", url)
+			d.log("Error (retrying): HTTP code %s while fetching %s\n", err, url)
 			break
 		}
 	}
@@ -115,8 +114,7 @@ func (d *GrabDownloader) maybeSetupChecksum(req *grab.Request, expected *utils.C
 }
 
 func (d *GrabDownloader) download(_ context.Context, url string, destination string, expected *utils.ChecksumInfo, ignoreMismatch bool) error {
-	// TODO clean up dest dir on permanent failure
-	d.log("Download %s -> %s\n", url, destination)
+	d.log("Downloading: %s\n", url)
 
 	req, err := grab.NewRequest(destination, url)
 	if err != nil {

pgp/gnupg_test.go

@@ -110,17 +110,17 @@ func (s *Gnupg1SignerSuite) SetUpTest(c *C) {
 		c.Skip(err.Error())
 	}
 
-	s.keyringNoPassphrase = [2]string{"keyrings/aptly.pub", "keyrings/aptly.sec"}
-	s.keyringPassphrase = [2]string{"keyrings/aptly_passphrase.pub", "keyrings/aptly_passphrase.sec"}
+	s.keyringNoPassphrase = [2]string{"../system/files/aptly.pub", "../system/files/aptly.sec"}
+	s.keyringPassphrase = [2]string{"../system/files/aptly_passphrase.pub", "../system/files/aptly_passphrase.sec"}
 	s.passphraseKey = "F30E8CB9CDDE2AF8"
 	s.noPassphraseKey = "21DBB89C16DB3E6D"
 
 	s.signer = NewGpgSigner(finder)
 	s.signer.SetBatch(true)
 
 	s.verifier = &GoVerifier{}
-	s.verifier.AddKeyring("./keyrings/aptly.pub")
-	s.verifier.AddKeyring("./keyrings/aptly_passphrase.pub")
+	s.verifier.AddKeyring("../system/files/aptly.pub")
+	s.verifier.AddKeyring("../system/files/aptly_passphrase.pub")
 
 	c.Assert(s.verifier.InitKeyring(false), IsNil)
 
@@ -183,7 +183,7 @@ func (s *Gnupg2SignerSuite) SetUpTest(c *C) {
 				args = append(args, "--pinentry-mode", "loopback")
 			}
 		}
-		args = append(args, "keyrings/aptly2"+item.suffix+".sec.armor")
+		args = append(args, "../system/files/aptly2"+item.suffix+".sec.armor")
 
 		output, err := exec.Command(gpg, args...).CombinedOutput()
 		c.Log(string(output))
@@ -193,22 +193,22 @@ func (s *Gnupg2SignerSuite) SetUpTest(c *C) {
 	// import public keys into gpg2
 	// we can't use pre-built keyrings as gpg 2.0.x and 2.1+ have different keyring formats
 	for _, suffix := range []string{"", "_passphrase"} {
-		output, err := exec.Command(gpg, "--no-default-keyring", "--batch", "--keyring", "./keyrings/aptly2"+suffix+".gpg",
-			"--import", "keyrings/aptly2"+suffix+".pub.armor").CombinedOutput()
+		output, err := exec.Command(gpg, "--no-default-keyring", "--batch", "--keyring", "../system/files/aptly2"+suffix+".gpg",
+			"--import", "../system/files/aptly2"+suffix+".pub.armor").CombinedOutput()
 		c.Log(string(output))
 		c.Check(err, IsNil)
 	}
 
-	s.keyringNoPassphrase = [2]string{"./keyrings/aptly2.gpg", ""}
-	s.keyringPassphrase = [2]string{"./keyrings/aptly2_passphrase.gpg", ""}
+	s.keyringNoPassphrase = [2]string{"../system/files/aptly2.gpg", ""}
+	s.keyringPassphrase = [2]string{"../system/files/aptly2_passphrase.gpg", ""}
 	s.noPassphraseKey = "751DF85C2B220D45"
 	s.passphraseKey = "6656CD181E92D2D5"
 
 	s.signer = NewGpgSigner(finder)
 	s.signer.SetBatch(true)
 
 	s.verifier = &GoVerifier{}
-	s.verifier.AddKeyring("./keyrings/aptly2_trusted.pub")
+	s.verifier.AddKeyring("../system/files/aptly2_trusted.pub")
 
 	c.Assert(s.verifier.InitKeyring(false), IsNil)
 
@@ -220,6 +220,6 @@ func (s *Gnupg2SignerSuite) SetUpTest(c *C) {
 func (s *Gnupg2SignerSuite) TearDownTest(c *C) {
 	s.SignerSuite.TearDownTest(c)
 
-	os.Remove("./keyrings/aptly2.gpg")
-	os.Remove("./keyrings/aptly2_passphrase.gpg")
+	os.Remove("../system/files/aptly2.gpg")
+	os.Remove("../system/files/aptly2_passphrase.gpg")
 }

pgp/internal_test.go

@@ -24,17 +24,17 @@ type GoSignerSuite struct {
 var _ = Suite(&GoSignerSuite{})
 
 func (s *GoSignerSuite) SetUpTest(c *C) {
-	s.keyringNoPassphrase = [2]string{"keyrings/aptly.pub", "keyrings/aptly.sec"}
-	s.keyringPassphrase = [2]string{"keyrings/aptly_passphrase.pub", "keyrings/aptly_passphrase.sec"}
+	s.keyringNoPassphrase = [2]string{"../system/files/aptly.pub", "../system/files/aptly.sec"}
+	s.keyringPassphrase = [2]string{"../system/files/aptly_passphrase.pub", "../system/files/aptly_passphrase.sec"}
 	s.passphraseKey = "F30E8CB9CDDE2AF8"
 	s.noPassphraseKey = "21DBB89C16DB3E6D"
 
 	s.signer = &GoSigner{}
 	s.signer.SetBatch(true)
 
 	s.verifier = &GoVerifier{}
-	s.verifier.AddKeyring("./keyrings/aptly.pub")
-	s.verifier.AddKeyring("./keyrings/aptly_passphrase.pub")
+	s.verifier.AddKeyring("../system/files/aptly.pub")
+	s.verifier.AddKeyring("../system/files/aptly_passphrase.pub")
 
 	c.Assert(s.verifier.InitKeyring(false), IsNil)
 

system/lib.py

@@ -103,7 +103,7 @@ def __init__(self):
     def find_dot(self, executables):
         for executable in executables:
             try:
-                subprocess.check_output([executable, "-V"], text=True)
+                subprocess.check_output([executable, "-V"], text=True, stderr=subprocess.DEVNULL)
                 return executable
             except Exception:
                 pass

system/t04_mirror/UpdateMirror26Test_gold

@@ -0,0 +1,33 @@
+
+
+Applying filter...
+Building download queue...
+Download queue: 1 items (3.35 KiB)
+Downloading & parsing package files...
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/InRelease
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/InRelease
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/Release
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/Release
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/Release.gpg
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/Release.gpg
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/main/binary-i386/Packages.gz
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/main/binary-i386/Packages.gz
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/pool/main/b/boost-defaults/libboost-program-options-dev_1.62.0.1_i386.deb
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/pool/main/b/boost-defaults/libboost-program-options-dev_1.62.0.1_i386.deb
+Mirror `grab` has been successfully updated.
+Packages filtered: 50604 -> 1.
+Retrying 0 http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/InRelease
+Retrying 0 http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/InRelease
+gpgv:                issuer "debian-release@lists.debian.org"
+gpgv:                using RSA key 0146DC6D4A0B2914BDED34DB648ACFD622F3D138
+gpgv:                using RSA key 067E3C456BAE240ACEE88F6FEF0F382A1A7B6500
+gpgv:                using RSA key 16E90B3FDF65EDE3AA7F323C04EE7237B7D453EC
+gpgv:                using RSA key A7236886F3CCCAAD148A27F80E98404D386FA1D9
+gpgv: Good signature from "Debian Archive Automatic Signing Key (10/buster) <ftpmaster@debian.org>"
+gpgv: Good signature from "Debian Archive Automatic Signing Key (11/bullseye) <ftpmaster@debian.org>"
+gpgv: Good signature from "Debian Archive Automatic Signing Key (9/stretch) <ftpmaster@debian.org>"
+gpgv: Good signature from "Debian Stable Release Key (9/stretch) <debian-release@lists.debian.org>"
+gpgv: Signature made Sat Aug 14 07:43:24 2021 UTC
+gpgv: Signature made Sat Aug 14 07:43:25 2021 UTC
+gpgv: Signature made Sat Aug 14 08:26:43 2021 UTC
+gpgv: Signature made Sat Aug 14 08:46:19 2021 UTC

system/t04_mirror/UpdateMirror27Test_gold

@@ -0,0 +1,39 @@
+
+  server returned 404 Not Found
+Applying filter...
+Building download queue...
+Download queue: 2 items (6.70 KiB)
+Downloading & parsing package files...
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/InRelease
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/InRelease
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/Release
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/Release
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/Release.gpg
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/Release.gpg
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/main/binary-amd64/Packages.gz
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/main/binary-amd64/Packages.gz
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/main/binary-i386/Packages.gz
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/main/binary-i386/Packages.gz
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/pool/main/b/boost-defaults/libboost-program-options-dev_1.62.0.1_amd64.deb
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/pool/main/b/boost-defaults/libboost-program-options-dev_1.62.0.1_amd64.deb
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/pool/main/b/boost-defaults/libboost-program-options-dev_1.62.0.1_i386.deb
+Downloading: http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/pool/main/b/boost-defaults/libboost-program-options-dev_1.62.0.1_i386.deb
+ERROR: unable to update: download errors:
+Packages filtered: 76844 -> 2.
+Retrying 0 http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/InRelease
+Retrying 0 http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/dists/stretch/InRelease
+Retrying 0 http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/pool/main/b/boost-defaults/libboost-program-options-dev_1.62.0.1_amd64.deb
+Retrying 0 http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian/pool/main/b/boost-defaults/libboost-program-options-dev_1.62.0.1_amd64.deb
+gpgv:                issuer "debian-release@lists.debian.org"
+gpgv:                using RSA key 0146DC6D4A0B2914BDED34DB648ACFD622F3D138
+gpgv:                using RSA key 067E3C456BAE240ACEE88F6FEF0F382A1A7B6500
+gpgv:                using RSA key 16E90B3FDF65EDE3AA7F323C04EE7237B7D453EC
+gpgv:                using RSA key A7236886F3CCCAAD148A27F80E98404D386FA1D9
+gpgv: Good signature from "Debian Archive Automatic Signing Key (10/buster) <ftpmaster@debian.org>"
+gpgv: Good signature from "Debian Archive Automatic Signing Key (11/bullseye) <ftpmaster@debian.org>"
+gpgv: Good signature from "Debian Archive Automatic Signing Key (9/stretch) <ftpmaster@debian.org>"
+gpgv: Good signature from "Debian Stable Release Key (9/stretch) <debian-release@lists.debian.org>"
+gpgv: Signature made Sat Aug 14 07:43:24 2021 UTC
+gpgv: Signature made Sat Aug 14 07:43:25 2021 UTC
+gpgv: Signature made Sat Aug 14 08:26:43 2021 UTC
+gpgv: Signature made Sat Aug 14 08:46:19 2021 UTC

system/t04_mirror/update.py

@@ -451,3 +451,34 @@ class UpdateMirror25Test(BaseTest):
     ]
     runCmd = "aptly mirror update -keyring=aptlytest.gpg mirror19"
     outputMatchPrepare = filterOutSignature
+
+
+class UpdateMirror26Test(BaseTest):
+    """
+    update mirrors: regular update, grab downloader
+    """
+    configOverride = {"downloader": "grab"}
+    sortOutput = True
+    longTest = False
+    fixtureGpg = True
+    fixtureCmds = [
+        "aptly mirror create -architectures=i386 -keyring=aptlytest.gpg -filter=libboost-program-options-dev grab http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian stretch main",
+    ]
+    runCmd = "aptly mirror update -downloader=grab -keyring=aptlytest.gpg grab"
+    outputMatchPrepare = filterOutRedirects
+
+
+class UpdateMirror27Test(BaseTest):
+    """
+    update mirrors: failing update, grab downloader
+    """
+    configOverride = {"downloader": "grab"}
+    sortOutput = True
+    longTest = False
+    fixtureGpg = True
+    fixtureCmds = [
+        "aptly mirror create -architectures=amd64,i386 -keyring=aptlytest.gpg -filter=libboost-program-options-dev grab-fail http://repo.aptly.info/system-tests/archive.debian.org/debian-archive/debian stretch main",
+    ]
+    runCmd = "aptly mirror update -downloader=grab -keyring=aptlytest.gpg grab-fail"
+    outputMatchPrepare = filterOutRedirects
+    expectedCode = 1

system/t12_api/publish.py

@@ -6,7 +6,6 @@
 
 DefaultSigningOptions = {
     "Keyring": os.path.join(os.path.dirname(inspect.getsourcefile(APITest)), "files") + "/aptly.pub",
-    "SecretKeyring": os.path.join(os.path.dirname(inspect.getsourcefile(APITest)), "files") + "/aptly.sec",
 }