Merge branch 'main' into release/v0.1

2023-06-16 17:36:40 +03:00 · 2023-06-16 17:36:40 +03:00 · 481ce343bc
commit 481ce343bc
parent 31b3d07dcf 8f31db1f67
14 changed files with 600 additions and 169 deletions
--- a/.gitignore
+++ b/.gitignore
@ -15,3 +15,4 @@ bin/
 # Dependency directories (remove the comment below to include it)
 # vendor/
 .vscode
+cmd/temp
--- a/12
+++ b/12
@ -6,10 +6,11 @@ USER_ID=$(shell ((docker --version | grep -q podman) && echo "0" || id -u))
 USER_GROUP=$(shell ((docker --version | grep -q podman) && echo "0" || id -g))
 ROOTDIR=$(dir $(abspath $(lastword $(MAKEFILE_LIST))))
 GOPATH ?= $(shell go env GOPATH)
+VERSION ?= $(shell git describe --tags --match='v[0-9]*' --dirty --always)
 GO ?= go


-default: install
+default: build

 .PHONY : build-static test install-lint-deps lint go-test fmt fmtcheck verify-vendor verify
 build-static:
@ -18,9 +19,12 @@ build-static:
 	docker run --rm -e USER_ID=$(USER_ID) -e USER_GROUP=$(USER_GROUP) -v $(PWD):/build/garm:z $(IMAGE_TAG) /build-static.sh
 	@echo Binaries are available in $(PWD)/bin

-install:
-	@$(GO) install -tags osusergo,netgo,sqlite_omit_load_extension ./...
-	@echo Binaries available in ${GOPATH}
+build:
+	@echo Building garm ${VERSION}
+	$(shell mkdir -p ./bin)
+	@$(GO) build -ldflags "-s -w -X main.Version=${VERSION}" -tags osusergo,netgo,sqlite_omit_load_extension -o bin/garm ./cmd/garm
+	@$(GO) build -ldflags "-s -w -X github.com/cloudbase/garm/cmd/garm-cli/cmd.Version=${VERSION}" -tags osusergo,netgo,sqlite_omit_load_extension -o bin/garm-cli ./cmd/garm-cli
+	@echo Binaries are available in $(PWD)/bin

 test: verify go-test

--- a/README.md
+++ b/README.md
@ -1,4 +1,4 @@
-# GitHub Actions Runners Manager (garm)
+# GitHub Actions Runner Manager (garm)

 [![Go Tests](https://github.com/cloudbase/garm/actions/workflows/go-tests.yml/badge.svg)](https://github.com/cloudbase/garm/actions/workflows/go-tests.yml)

@ -10,6 +10,12 @@ The goal of ```garm``` is to be simple to set up, simple to configure and simple

 Garm supports creating pools on either GitHub itself or on your own deployment of [GitHub Enterprise Server](https://docs.github.com/en/enterprise-server@3.5/admin/overview/about-github-enterprise-server). For instructions on how to use ```garm``` with GHE, see the [credentials](/doc/github_credentials.md) section of the documentation.

+## Join us on slack
+
+Whether you're running into issues or just want to drop by and say "hi", feel free to [join us on slack](https://communityinviter.com/apps/garm-hq/garm).
+
+[![slack](https://img.shields.io/badge/slack-garm-brightgreen.svg?logo=slack)](https://communityinviter.com/apps/garm-hq/garm)
+
 ## Installing

 ## Build from source
--- a/cloudconfig/templates.go
+++ b/cloudconfig/templates.go
@ -36,11 +36,11 @@ if [ -z "$METADATA_URL" ];then
 	echo "no token is available and METADATA_URL is not set"
 	exit 1
 fi
-GITHUB_TOKEN=$(curl --fail -s -X GET -H 'Accept: application/json' -H "Authorization: Bearer ${BEARER_TOKEN}" "${METADATA_URL}/runner-registration-token/")
+GITHUB_TOKEN=$(curl --retry 5 --retry-max-time 5 --fail -s -X GET -H 'Accept: application/json' -H "Authorization: Bearer ${BEARER_TOKEN}" "${METADATA_URL}/runner-registration-token/")

 function call() {
 	PAYLOAD="$1"
-	curl --fail -s -X POST -d "${PAYLOAD}" -H 'Accept: application/json' -H "Authorization: Bearer ${BEARER_TOKEN}" "${CALLBACK_URL}" || echo "failed to call home: exit code ($?)"
+	curl --retry 5 --retry-max-time 5 --retry-all-errors --fail -s -X POST -d "${PAYLOAD}" -H 'Accept: application/json' -H "Authorization: Bearer ${BEARER_TOKEN}" "${CALLBACK_URL}" || echo "failed to call home: exit code ($?)"
 }

 function sendStatus() {
@ -63,41 +63,41 @@ function fail() {
 # This will echo the version number in the filename. Given a file name like: actions-runner-osx-x64-2.299.1.tar.gz
 # this will output: 2.299.1
 function getRunnerVersion() {
-    FILENAME="{{ .FileName }}"
-    [[ $FILENAME =~ ([0-9]+\.[0-9]+\.[0-9+]) ]]
-    echo $BASH_REMATCH
+	FILENAME="{{ .FileName }}"
+	[[ $FILENAME =~ ([0-9]+\.[0-9]+\.[0-9+]) ]]
+	echo $BASH_REMATCH
 }

 function getCachedToolsPath() {
-    CACHED_RUNNER="/opt/cache/actions-runner/latest"
-    if [ -d "$CACHED_RUNNER" ];then
-        echo "$CACHED_RUNNER"
-        return 0
-    fi
+	CACHED_RUNNER="/opt/cache/actions-runner/latest"
+	if [ -d "$CACHED_RUNNER" ];then
+		echo "$CACHED_RUNNER"
+		return 0
+	fi

-    VERSION=$(getRunnerVersion)
-    if [ -z "$VERSION" ]; then
-        return 0
-    fi
+	VERSION=$(getRunnerVersion)
+	if [ -z "$VERSION" ]; then
+		return 0
+	fi

-    CACHED_RUNNER="/opt/cache/actions-runner/$VERSION"
-    if [ -d "$CACHED_RUNNER" ];then
-        echo "$CACHED_RUNNER"
-        return 0
-    fi
-    return 0
+	CACHED_RUNNER="/opt/cache/actions-runner/$VERSION"
+	if [ -d "$CACHED_RUNNER" ];then
+		echo "$CACHED_RUNNER"
+		return 0
+	fi
+	return 0
 }

 function downloadAndExtractRunner() {
-    sendStatus "downloading tools from {{ .DownloadURL }}"
-    if [ ! -z "{{ .TempDownloadToken }}" ]; then
+	sendStatus "downloading tools from {{ .DownloadURL }}"
+	if [ ! -z "{{ .TempDownloadToken }}" ]; then
 	TEMP_TOKEN="Authorization: Bearer {{ .TempDownloadToken }}"
-    fi
-    curl -L -H "${TEMP_TOKEN}" -o "/home/{{ .RunnerUsername }}/{{ .FileName }}" "{{ .DownloadURL }}" || fail "failed to download tools"
-    mkdir -p /home/runner/actions-runner || fail "failed to create actions-runner folder"
-    sendStatus "extracting runner"
-    tar xf "/home/{{ .RunnerUsername }}/{{ .FileName }}" -C /home/{{ .RunnerUsername }}/actions-runner/ || fail "failed to extract runner"
-    chown {{ .RunnerUsername }}:{{ .RunnerGroup }} -R /home/{{ .RunnerUsername }}/actions-runner/ || fail "failed to change owner"
+	fi
+	curl --retry 5 --retry-max-time 5 --retry-all-errors --fail -L -H "${TEMP_TOKEN}" -o "/home/{{ .RunnerUsername }}/{{ .FileName }}" "{{ .DownloadURL }}" || fail "failed to download tools"
+	mkdir -p /home/runner/actions-runner || fail "failed to create actions-runner folder"
+	sendStatus "extracting runner"
+	tar xf "/home/{{ .RunnerUsername }}/{{ .FileName }}" -C /home/{{ .RunnerUsername }}/actions-runner/ || fail "failed to extract runner"
+	chown {{ .RunnerUsername }}:{{ .RunnerGroup }} -R /home/{{ .RunnerUsername }}/actions-runner/ || fail "failed to change owner"
 }

 TEMP_TOKEN=""
@ -107,31 +107,59 @@ GH_RUNNER_GROUP="{{.GitHubRunnerGroup}}"
 # if it holds a value, it will be part of the command.
 RUNNER_GROUP_OPT=""
 if [ ! -z $GH_RUNNER_GROUP ];then
-    RUNNER_GROUP_OPT="--runnergroup=$GH_RUNNER_GROUP"
+	RUNNER_GROUP_OPT="--runnergroup=$GH_RUNNER_GROUP"
 fi

 CACHED_RUNNER=$(getCachedToolsPath)
 if [ -z "$CACHED_RUNNER" ];then
-    downloadAndExtractRunner
-    sendStatus "installing dependencies"
-    cd /home/{{ .RunnerUsername }}/actions-runner
-    sudo ./bin/installdependencies.sh || fail "failed to install dependencies"
+	downloadAndExtractRunner
+	sendStatus "installing dependencies"
+	cd /home/{{ .RunnerUsername }}/actions-runner
+	sudo ./bin/installdependencies.sh || fail "failed to install dependencies"
 else
-    sudo cp -a "$CACHED_RUNNER"  "/home/{{ .RunnerUsername }}/actions-runner"
-    cd /home/{{ .RunnerUsername }}/actions-runner
-    chown {{ .RunnerUsername }}:{{ .RunnerGroup }} -R "/home/{{ .RunnerUsername }}/actions-runner" || fail "failed to change owner"
+	sendStatus "using cached runner found in $CACHED_RUNNER"
+	sudo cp -a "$CACHED_RUNNER"  "/home/{{ .RunnerUsername }}/actions-runner"
+	cd /home/{{ .RunnerUsername }}/actions-runner
+	chown {{ .RunnerUsername }}:{{ .RunnerGroup }} -R "/home/{{ .RunnerUsername }}/actions-runner" || fail "failed to change owner"
 fi


 sendStatus "configuring runner"
-sudo -u {{ .RunnerUsername }} -- ./config.sh --unattended --url "{{ .RepoURL }}" --token "$GITHUB_TOKEN" $RUNNER_GROUP_OPT --name "{{ .RunnerName }}" --labels "{{ .RunnerLabels }}" --ephemeral || fail "failed to configure runner"
+set +e
+attempt=1
+while true; do
+	ERROUT=$(mktemp)
+	sudo -u {{ .RunnerUsername }} -- ./config.sh --unattended --url "{{ .RepoURL }}" --token "$GITHUB_TOKEN" $RUNNER_GROUP_OPT --name "{{ .RunnerName }}" --labels "{{ .RunnerLabels }}" --ephemeral 2>$ERROUT
+	if [ $? -eq 0 ]; then
+		rm $ERROUT || true
+		sendStatus "runner successfully configured after $attempt attempt(s)"
+		break
+	fi
+	LAST_ERR=$(cat $ERROUT)
+	echo "$LAST_ERR"
+
+	# if the runner is already configured, remove it and try again. In the past configuring a runner
+	# managed to register it but timed out later, resulting in an error.
+	sudo -u {{ .RunnerUsername }} -- ./config.sh remove --token "$GITHUB_TOKEN" || true
+
+	if [ $attempt -gt 5 ];then
+		rm $ERROUT || true
+		fail "failed to configure runner: $LAST_ERR"
+	fi
+
+	sendStatus "failed to configure runner (attempt $attempt): $LAST_ERR (retrying in 5 seconds)"
+	attempt=$((attempt+1))
+	rm $ERROUT || true
+	sleep 5
+done
+set -e

 sendStatus "installing runner service"
 ./svc.sh install {{ .RunnerUsername }} || fail "failed to install service"

 if [ -e "/sys/fs/selinux" ];then
-    sudo chcon -h user_u:object_r:bin_t /home/runner/ || fail "failed to change selinux context"
-    sudo chcon -R -h {{ .RunnerUsername }}:object_r:bin_t /home/runner/* || fail "failed to change selinux context"
+	sudo chcon -h user_u:object_r:bin_t /home/runner/ || fail "failed to change selinux context"
+	sudo chcon -R -h {{ .RunnerUsername }}:object_r:bin_t /home/runner/* || fail "failed to change selinux context"
 fi

 sendStatus "starting service"
@ -156,105 +184,105 @@ Param(
 $ErrorActionPreference="Stop"

 function Invoke-FastWebRequest {
-    [CmdletBinding()]
-    Param(
-        [Parameter(Mandatory=$True,ValueFromPipeline=$true,Position=0)]
-        [System.Uri]$Uri,
-        [Parameter(Position=1)]
-        [string]$OutFile,
-        [Hashtable]$Headers=@{},
-        [switch]$SkipIntegrityCheck=$false
-    )
-    PROCESS
-    {
-        if(!([System.Management.Automation.PSTypeName]'System.Net.Http.HttpClient').Type)
-        {
-            $assembly = [System.Reflection.Assembly]::LoadWithPartialName("System.Net.Http")
-        }
+	[CmdletBinding()]
+	Param(
+		[Parameter(Mandatory=$True,ValueFromPipeline=$true,Position=0)]
+		[System.Uri]$Uri,
+		[Parameter(Position=1)]
+		[string]$OutFile,
+		[Hashtable]$Headers=@{},
+		[switch]$SkipIntegrityCheck=$false
+	)
+	PROCESS
+	{
+		if(!([System.Management.Automation.PSTypeName]'System.Net.Http.HttpClient').Type)
+		{
+			$assembly = [System.Reflection.Assembly]::LoadWithPartialName("System.Net.Http")
+		}

-        if(!$OutFile) {
-            $OutFile = $Uri.PathAndQuery.Substring($Uri.PathAndQuery.LastIndexOf("/") + 1)
-            if(!$OutFile) {
-                throw "The ""OutFile"" parameter needs to be specified"
-            }
-        }
+		if(!$OutFile) {
+			$OutFile = $Uri.PathAndQuery.Substring($Uri.PathAndQuery.LastIndexOf("/") + 1)
+			if(!$OutFile) {
+				throw "The ""OutFile"" parameter needs to be specified"
+			}
+		}

-        $fragment = $Uri.Fragment.Trim('#')
-        if ($fragment) {
-            $details = $fragment.Split("=")
-            $algorithm = $details[0]
-            $hash = $details[1]
-        }
+		$fragment = $Uri.Fragment.Trim('#')
+		if ($fragment) {
+			$details = $fragment.Split("=")
+			$algorithm = $details[0]
+			$hash = $details[1]
+		}

-        if (!$SkipIntegrityCheck -and $fragment -and (Test-Path $OutFile)) {
-            try {
-                return (Test-FileIntegrity -File $OutFile -Algorithm $algorithm -ExpectedHash $hash)
-            } catch {
-                Remove-Item $OutFile
-            }
-        }
+		if (!$SkipIntegrityCheck -and $fragment -and (Test-Path $OutFile)) {
+			try {
+				return (Test-FileIntegrity -File $OutFile -Algorithm $algorithm -ExpectedHash $hash)
+			} catch {
+				Remove-Item $OutFile
+			}
+		}

-        $client = new-object System.Net.Http.HttpClient
-        foreach ($k in $Headers.Keys){
-            $client.DefaultRequestHeaders.Add($k, $Headers[$k])
-        }
-        $task = $client.GetStreamAsync($Uri)
-        $response = $task.Result
-        if($task.IsFaulted) {
-            $msg = "Request for URL '{0}' is faulted. Task status: {1}." -f @($Uri, $task.Status)
-            if($task.Exception) {
-                $msg += "Exception details: {0}" -f @($task.Exception)
-            }
-            Throw $msg
-        }
-        $outStream = New-Object IO.FileStream $OutFile, Create, Write, None
+		$client = new-object System.Net.Http.HttpClient
+		foreach ($k in $Headers.Keys){
+			$client.DefaultRequestHeaders.Add($k, $Headers[$k])
+		}
+		$task = $client.GetStreamAsync($Uri)
+		$response = $task.Result
+		if($task.IsFaulted) {
+			$msg = "Request for URL '{0}' is faulted. Task status: {1}." -f @($Uri, $task.Status)
+			if($task.Exception) {
+				$msg += "Exception details: {0}" -f @($task.Exception)
+			}
+			Throw $msg
+		}
+		$outStream = New-Object IO.FileStream $OutFile, Create, Write, None

-        try {
-            $totRead = 0
-            $buffer = New-Object Byte[] 1MB
-            while (($read = $response.Read($buffer, 0, $buffer.Length)) -gt 0) {
-                $totRead += $read
-                $outStream.Write($buffer, 0, $read);
-            }
-        }
-        finally {
-            $outStream.Close()
-        }
-        if(!$SkipIntegrityCheck -and $fragment) {
-            Test-FileIntegrity -File $OutFile -Algorithm $algorithm -ExpectedHash $hash
-        }
-    }
+		try {
+			$totRead = 0
+			$buffer = New-Object Byte[] 1MB
+			while (($read = $response.Read($buffer, 0, $buffer.Length)) -gt 0) {
+				$totRead += $read
+				$outStream.Write($buffer, 0, $read);
+			}
+		}
+		finally {
+			$outStream.Close()
+		}
+		if(!$SkipIntegrityCheck -and $fragment) {
+			Test-FileIntegrity -File $OutFile -Algorithm $algorithm -ExpectedHash $hash
+		}
+	}
 }

 function Import-Certificate() {
-    [CmdletBinding()]
-    param (
-        [parameter(Mandatory=$true)]
-        [string]$CertificatePath,
-        [parameter(Mandatory=$true)]
-        [System.Security.Cryptography.X509Certificates.StoreLocation]$StoreLocation="LocalMachine",
-        [parameter(Mandatory=$true)]
-        [System.Security.Cryptography.X509Certificates.StoreName]$StoreName="TrustedPublisher"
-    )
-    PROCESS
-    {
-        $store = New-Object System.Security.Cryptography.X509Certificates.X509Store(
-            $StoreName, $StoreLocation)
-        $store.Open([System.Security.Cryptography.X509Certificates.OpenFlags]::ReadWrite)
-        $cert = New-Object System.Security.Cryptography.X509Certificates.X509Certificate2(
-            $CertificatePath)
-        $store.Add($cert)
-    }
+	[CmdletBinding()]
+	param (
+		[parameter(Mandatory=$true)]
+		[string]$CertificatePath,
+		[parameter(Mandatory=$true)]
+		[System.Security.Cryptography.X509Certificates.StoreLocation]$StoreLocation="LocalMachine",
+		[parameter(Mandatory=$true)]
+		[System.Security.Cryptography.X509Certificates.StoreName]$StoreName="TrustedPublisher"
+	)
+	PROCESS
+	{
+		$store = New-Object System.Security.Cryptography.X509Certificates.X509Store(
+			$StoreName, $StoreLocation)
+		$store.Open([System.Security.Cryptography.X509Certificates.OpenFlags]::ReadWrite)
+		$cert = New-Object System.Security.Cryptography.X509Certificates.X509Certificate2(
+			$CertificatePath)
+		$store.Add($cert)
+	}
 }

 function Invoke-APICall() {
 	[CmdletBinding()]
-    param (
-        [parameter(Mandatory=$true)]
-        [object]$Payload,
+	param (
+		[parameter(Mandatory=$true)]
+		[object]$Payload,
 		[parameter(Mandatory=$true)]
 		[string]$CallbackURL
-    )
+	)
 	PROCESS{
 		Invoke-WebRequest -UseBasicParsing -Method Post -Headers @{"Accept"="application/json"; "Authorization"="Bearer $Token"} -Uri $CallbackURL -Body (ConvertTo-Json $Payload) | Out-Null
 	}
@ -262,12 +290,12 @@ function Invoke-APICall() {

 function Update-GarmStatus() {
 	[CmdletBinding()]
-    param (
-        [parameter(Mandatory=$true)]
-        [string]$Message,
+	param (
+		[parameter(Mandatory=$true)]
+		[string]$Message,
 		[parameter(Mandatory=$true)]
 		[string]$CallbackURL
-    )
+	)
 	PROCESS{
 		$body = @{
 			"status"="installing"
@ -279,14 +307,14 @@ function Update-GarmStatus() {

 function Invoke-GarmSuccess() {
 	[CmdletBinding()]
-    param (
-        [parameter(Mandatory=$true)]
-        [string]$Message,
+	param (
 		[parameter(Mandatory=$true)]
-        [int64]$AgentID,
+		[string]$Message,
+		[parameter(Mandatory=$true)]
+		[int64]$AgentID,
 		[parameter(Mandatory=$true)]
 		[string]$CallbackURL
-    )
+	)
 	PROCESS{
 		$body = @{
 			"status"="idle"
@ -299,12 +327,12 @@ function Invoke-GarmSuccess() {

 function Invoke-GarmFailure() {
 	[CmdletBinding()]
-    param (
-        [parameter(Mandatory=$true)]
-        [string]$Message,
+	param (
+		[parameter(Mandatory=$true)]
+		[string]$Message,
 		[parameter(Mandatory=$true)]
 		[string]$CallbackURL
-    )
+	)
 	PROCESS{
 		$body = @{
 			"status"="failed"
--- a/database/sql/users_test.go
+++ b/database/sql/users_test.go
@ -16,25 +16,40 @@ package sql

 import (
 	"context"
+	"flag"
 	"fmt"
+	"regexp"
 	"testing"

 	dbCommon "github.com/cloudbase/garm/database/common"
 	garmTesting "github.com/cloudbase/garm/internal/testing"
 	"github.com/cloudbase/garm/params"
 	"github.com/stretchr/testify/suite"
+	"gopkg.in/DATA-DOG/go-sqlmock.v1"
+	"gorm.io/driver/mysql"
+	"gorm.io/gorm"
+	"gorm.io/gorm/logger"
 )

 type UserTestFixtures struct {
-	Users         []params.User
-	NewUserParams params.NewUserParams
-	AdminContext  context.Context
+	Users            []params.User
+	NewUserParams    params.NewUserParams
+	UpdateUserParams params.UpdateUserParams
+	SQLMock          sqlmock.Sqlmock
 }

 type UserTestSuite struct {
 	suite.Suite
-	Store    dbCommon.Store
-	Fixtures *UserTestFixtures
+	Store          dbCommon.Store
+	StoreSQLMocked *sqlDatabase
+	Fixtures       *UserTestFixtures
+}
+
+func (s *UserTestSuite) assertSQLMockExpectations() {
+	err := s.Fixtures.SQLMock.ExpectationsWereMet()
+	if err != nil {
+		s.FailNow(fmt.Sprintf("failed to meet sqlmock expectations, got error: %v", err))
+	}
 }

 func (s *UserTestSuite) SetupTest() {
@ -64,7 +79,31 @@ func (s *UserTestSuite) SetupTest() {
 		users = append(users, user)
 	}

+	// create store with mocked sql connection
+	sqlDB, sqlMock, err := sqlmock.New()
+	if err != nil {
+		s.FailNow(fmt.Sprintf("failed to run 'sqlmock.New()', got error: %v", err))
+	}
+	s.T().Cleanup(func() { sqlDB.Close() })
+	mysqlConfig := mysql.Config{
+		Conn:                      sqlDB,
+		SkipInitializeWithVersion: true,
+	}
+	gormConfig := &gorm.Config{}
+	if flag.Lookup("test.v").Value.String() == "false" {
+		gormConfig.Logger = logger.Default.LogMode(logger.Silent)
+	}
+	gormConn, err := gorm.Open(mysql.New(mysqlConfig), gormConfig)
+	if err != nil {
+		s.FailNow(fmt.Sprintf("fail to open gorm connection: %v", err))
+	}
+	s.StoreSQLMocked = &sqlDatabase{
+		conn: gormConn,
+		cfg:  garmTesting.GetTestSqliteDBConfig(s.T()),
+	}
+
 	// setup test fixtures
+	var enabled bool
 	fixtures := &UserTestFixtures{
 		Users: users,
 		NewUserParams: params.NewUserParams{
@ -73,6 +112,12 @@ func (s *UserTestSuite) SetupTest() {
 			FullName: "test-fullname",
 			Password: "test-password",
 		},
+		UpdateUserParams: params.UpdateUserParams{
+			FullName: "test-update-fullname",
+			Password: "test-update-password",
+			Enabled:  &enabled,
+		},
+		SQLMock: sqlMock,
 	}
 	s.Fixtures = fixtures
 }
@ -120,6 +165,46 @@ func (s *UserTestSuite) TestCreateUserEmailAlreadyExist() {
 	s.Require().Equal(("email already exists"), err.Error())
 }

+func (s *UserTestSuite) TestCreateUserDBCreateErr() {
+	s.Fixtures.SQLMock.
+		ExpectQuery(regexp.QuoteMeta("SELECT * FROM `users` WHERE username = ? AND `users`.`deleted_at` IS NULL ORDER BY `users`.`id` LIMIT 1")).
+		WithArgs(s.Fixtures.NewUserParams.Username).
+		WillReturnRows(sqlmock.NewRows([]string{"id"}))
+	s.Fixtures.SQLMock.
+		ExpectQuery(regexp.QuoteMeta("SELECT * FROM `users` WHERE email = ? AND `users`.`deleted_at` IS NULL ORDER BY `users`.`id` LIMIT 1")).
+		WithArgs(s.Fixtures.NewUserParams.Email).
+		WillReturnRows(sqlmock.NewRows([]string{"id"}))
+	s.Fixtures.SQLMock.ExpectBegin()
+	s.Fixtures.SQLMock.
+		ExpectExec("INSERT INTO `users`").
+		WillReturnError(fmt.Errorf("creating user mock error"))
+	s.Fixtures.SQLMock.ExpectRollback()
+
+	_, err := s.StoreSQLMocked.CreateUser(context.Background(), s.Fixtures.NewUserParams)
+
+	s.assertSQLMockExpectations()
+	s.Require().NotNil(err)
+	s.Require().Equal("creating user: creating user mock error", err.Error())
+}
+
+func (s *UserTestSuite) TestHasAdminUserNoAdmin() {
+	hasAdmin := s.Store.HasAdminUser(context.Background())
+
+	// initially, we don't have any admin users in the store
+	s.Require().False(hasAdmin)
+}
+
+func (s *UserTestSuite) TestHasAdminUser() {
+	// create an admin user
+	s.Fixtures.NewUserParams.IsAdmin = true
+	_, err := s.Store.CreateUser(context.Background(), s.Fixtures.NewUserParams)
+	s.Require().Nil(err)
+
+	// check again if the store has any admin users
+	hasAdmin := s.Store.HasAdminUser(context.Background())
+	s.Require().True(hasAdmin)
+}
+
 func (s *UserTestSuite) TestGetUser() {
 	user, err := s.Store.GetUser(context.Background(), s.Fixtures.Users[0].Username)

@ -154,6 +239,40 @@ func (s *UserTestSuite) TestGetUserByIDNotFound() {
 	s.Require().Equal("fetching user: not found", err.Error())
 }

+func (s *UserTestSuite) TestUpdateUser() {
+	user, err := s.Store.UpdateUser(context.Background(), s.Fixtures.Users[0].Username, s.Fixtures.UpdateUserParams)
+
+	s.Require().Nil(err)
+	s.Require().Equal(s.Fixtures.UpdateUserParams.FullName, user.FullName)
+	s.Require().Equal(s.Fixtures.UpdateUserParams.Password, user.Password)
+	s.Require().Equal(*s.Fixtures.UpdateUserParams.Enabled, user.Enabled)
+}
+
+func (s *UserTestSuite) TestUpdateUserNotFound() {
+	_, err := s.Store.UpdateUser(context.Background(), "dummy-user", s.Fixtures.UpdateUserParams)
+
+	s.Require().NotNil(err)
+	s.Require().Equal("fetching user: not found", err.Error())
+}
+
+func (s *UserTestSuite) TestUpdateUserDBSaveErr() {
+	s.Fixtures.SQLMock.
+		ExpectQuery(regexp.QuoteMeta("SELECT * FROM `users` WHERE username = ? AND `users`.`deleted_at` IS NULL ORDER BY `users`.`id` LIMIT 1")).
+		WithArgs(s.Fixtures.Users[0].ID).
+		WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(s.Fixtures.Users[0].ID))
+	s.Fixtures.SQLMock.ExpectBegin()
+	s.Fixtures.SQLMock.
+		ExpectExec(("UPDATE `users` SET")).
+		WillReturnError(fmt.Errorf("saving user mock error"))
+	s.Fixtures.SQLMock.ExpectRollback()
+
+	_, err := s.StoreSQLMocked.UpdateUser(context.Background(), s.Fixtures.Users[0].ID, s.Fixtures.UpdateUserParams)
+
+	s.assertSQLMockExpectations()
+	s.Require().NotNil(err)
+	s.Require().Equal("saving user: saving user mock error", err.Error())
+}
+
 func TestUserTestSuite(t *testing.T) {
 	suite.Run(t, new(UserTestSuite))
 }
--- a/params/params.go
+++ b/params/params.go
@ -222,7 +222,8 @@ type BootstrapInstance struct {
 }

 type UserDataOptions struct {
-	DisableUpdatesOnBoot bool `json:"disable_updates_on_boot"`
+	DisableUpdatesOnBoot bool     `json:"disable_updates_on_boot"`
+	ExtraPackages        []string `json:"extra_packages"`
 }

 type Tag struct {
--- a/runner/pool/pool.go
+++ b/runner/pool/pool.go
@ -365,26 +365,44 @@ func (r *basePoolManager) reapTimedOutRunners(runners []*github.Runner) error {
 		return errors.Wrap(err, "fetching instances from db")
 	}

-	runnerNames := map[string]bool{}
+	runnersByName := map[string]*github.Runner{}
 	for _, run := range runners {
 		if !r.isManagedRunner(labelsFromRunner(run)) {
 			log.Printf("runner %s is not managed by a pool belonging to %s", *run.Name, r.helper.String())
 			continue
 		}
-		runnerNames[*run.Name] = true
+		runnersByName[*run.Name] = run
 	}

 	for _, instance := range dbInstances {
-		if ok := runnerNames[instance.Name]; !ok {
-			pool, err := r.store.GetPoolByID(r.ctx, instance.PoolID)
-			if err != nil {
-				return errors.Wrap(err, "fetching instance pool info")
-			}
-			if time.Since(instance.UpdatedAt).Minutes() < float64(pool.RunnerTimeout()) {
-				continue
-			}
-			log.Printf("reaping instance %s due to timeout", instance.Name)
-			if err := r.setInstanceStatus(instance.Name, providerCommon.InstancePendingDelete, nil); err != nil {
+		pool, err := r.store.GetPoolByID(r.ctx, instance.PoolID)
+		if err != nil {
+			return errors.Wrap(err, "fetching instance pool info")
+		}
+		if time.Since(instance.UpdatedAt).Minutes() < float64(pool.RunnerTimeout()) {
+			continue
+		}
+
+		// There are 2 cases (currently) where we consider a runner as timed out:
+		//   * The runner never joined github within the pool timeout
+		//   * The runner managed to join github, but the setup process failed later and the runner
+		//     never started on the instance.
+		//
+		// There are several steps in the user data that sets up the runner:
+		//   * Download and unarchive the runner from github (or used the cached version)
+		//   * Configure runner (connects to github). At this point the runner is seen as offline.
+		//   * Install the service
+		//   * Set SELinux context (if SELinux is enabled)
+		//   * Start the service (if successful, the runner will transition to "online")
+		//   * Get the runner ID
+		//
+		// If we fail getting the runner ID after it's started, garm will set the runner status to "failed",
+		// even though, technically the runner is online and fully functional. This is why we check here for
+		// both the runner status as reported by GitHub and the runner status as reported by the provider.
+		// If the runner is "offline" and marked as "failed", it should be safe to reap it.
+		if runner, ok := runnersByName[instance.Name]; !ok || (runner.GetStatus() == "offline" && instance.RunnerStatus == providerCommon.RunnerFailed) {
+			log.Printf("reaping timed-out/failed runner %s", instance.Name)
+			if err := r.ForceDeleteRunner(instance); err != nil {
 				log.Printf("failed to update runner %s status", instance.Name)
 				return errors.Wrap(err, "updating runner")
 			}
--- a/runner/pools.go
+++ b/runner/pools.go
@ -67,7 +67,7 @@ func (r *Runner) DeletePoolByID(ctx context.Context, poolID string) error {
 	}

 	if err := r.store.DeletePoolByID(ctx, poolID); err != nil {
-		return errors.Wrap(err, "fetching pool")
+		return errors.Wrap(err, "deleting pool")
 	}
 	return nil
 }
--- a/runner/pools_test.go
+++ b/runner/pools_test.go
@ -0,0 +1,223 @@
+// Copyright 2022 Cloudbase Solutions SRL
+//
+//    Licensed under the Apache License, Version 2.0 (the "License"); you may
+//    not use this file except in compliance with the License. You may obtain
+//    a copy of the License at
+//
+//         http://www.apache.org/licenses/LICENSE-2.0
+//
+//    Unless required by applicable law or agreed to in writing, software
+//    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+//    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+//    License for the specific language governing permissions and limitations
+//    under the License.
+
+package runner
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/cloudbase/garm/auth"
+	"github.com/cloudbase/garm/config"
+	"github.com/cloudbase/garm/database"
+	dbCommon "github.com/cloudbase/garm/database/common"
+	runnerErrors "github.com/cloudbase/garm/errors"
+	garmTesting "github.com/cloudbase/garm/internal/testing"
+	"github.com/cloudbase/garm/params"
+	"github.com/cloudbase/garm/runner/common"
+	"github.com/stretchr/testify/suite"
+)
+
+type PoolTestFixtures struct {
+	AdminContext         context.Context
+	Store                dbCommon.Store
+	Pools                []params.Pool
+	Providers            map[string]common.Provider
+	Credentials          map[string]config.Github
+	CreateInstanceParams params.CreateInstanceParams
+	UpdatePoolParams     params.UpdatePoolParams
+}
+
+type PoolTestSuite struct {
+	suite.Suite
+	Fixtures *PoolTestFixtures
+	Runner   *Runner
+}
+
+func (s *PoolTestSuite) SetupTest() {
+	adminCtx := auth.GetAdminContext()
+
+	// create testing sqlite database
+	dbCfg := garmTesting.GetTestSqliteDBConfig(s.T())
+	db, err := database.NewDatabase(adminCtx, dbCfg)
+	if err != nil {
+		s.FailNow(fmt.Sprintf("failed to create db connection: %s", err))
+	}
+
+	// create an organization for testing purposes
+	org, err := db.CreateOrganization(context.Background(), "test-org", "test-creds", "test-webhookSecret")
+	if err != nil {
+		s.FailNow(fmt.Sprintf("failed to create org: %s", err))
+	}
+
+	// create some pool objects in the database, for testing purposes
+	orgPools := []params.Pool{}
+	for i := 1; i <= 3; i++ {
+		pool, err := db.CreateOrganizationPool(
+			context.Background(),
+			org.ID,
+			params.CreatePoolParams{
+				ProviderName:           "test-provider",
+				MaxRunners:             4,
+				MinIdleRunners:         2,
+				Image:                  fmt.Sprintf("test-image-%d", i),
+				Flavor:                 "test-flavor",
+				OSType:                 "linux",
+				Tags:                   []string{"self-hosted", "amd64", "linux"},
+				RunnerBootstrapTimeout: 0,
+			},
+		)
+		if err != nil {
+			s.FailNow(fmt.Sprintf("cannot create org pool: %v", err))
+		}
+		orgPools = append(orgPools, pool)
+	}
+
+	// setup test fixtures
+	var maxRunners uint = 40
+	var minIdleRunners uint = 20
+	fixtures := &PoolTestFixtures{
+		AdminContext: adminCtx,
+		Store:        db,
+		Pools:        orgPools,
+		UpdatePoolParams: params.UpdatePoolParams{
+			MaxRunners:     &maxRunners,
+			MinIdleRunners: &minIdleRunners,
+			Image:          "test-images-updated",
+			Flavor:         "test-flavor-updated",
+		},
+		CreateInstanceParams: params.CreateInstanceParams{
+			Name:   "test-instance-name",
+			OSType: "linux",
+		},
+	}
+	s.Fixtures = fixtures
+
+	// setup test runner
+	runner := &Runner{
+		providers:   fixtures.Providers,
+		credentials: fixtures.Credentials,
+		store:       fixtures.Store,
+		ctx:         fixtures.AdminContext,
+	}
+	s.Runner = runner
+}
+
+func (s *PoolTestSuite) TestListAllPools() {
+	// call tested function
+	pools, err := s.Runner.ListAllPools(s.Fixtures.AdminContext)
+
+	// assertions
+	s.Require().Nil(err)
+	garmTesting.EqualDBEntityID(s.T(), s.Fixtures.Pools, pools)
+}
+
+func (s *PoolTestSuite) TestListAllPoolsErrUnauthorized() {
+	_, err := s.Runner.ListAllPools(context.Background())
+
+	s.Require().NotNil(err)
+	s.Require().Equal(runnerErrors.ErrUnauthorized, err)
+}
+
+func (s *PoolTestSuite) TestGetPoolByID() {
+	pool, err := s.Runner.GetPoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID)
+
+	s.Require().Nil(err)
+	s.Require().Equal(s.Fixtures.Pools[0].ID, pool.ID)
+}
+
+func (s *PoolTestSuite) TestGetPoolByIDErrUnauthorized() {
+	_, err := s.Runner.GetPoolByID(context.Background(), "dummy-pool-id")
+
+	s.Require().NotNil(err)
+	s.Require().Equal(runnerErrors.ErrUnauthorized, err)
+}
+
+func (s *PoolTestSuite) TestGetPoolByIDNotFound() {
+	err := s.Fixtures.Store.DeletePoolByID(context.Background(), s.Fixtures.Pools[0].ID)
+
+	s.Require().Nil(err)
+	_, err = s.Runner.GetPoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID)
+	s.Require().NotNil(err)
+	s.Require().Equal("fetching pool: fetching pool by ID: not found", err.Error())
+}
+
+func (s *PoolTestSuite) TestDeletePoolByID() {
+	err := s.Runner.DeletePoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID)
+
+	s.Require().Nil(err)
+	_, err = s.Fixtures.Store.GetPoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID)
+	s.Require().NotNil(err)
+	s.Require().Equal("fetching pool by ID: not found", err.Error())
+}
+
+func (s *PoolTestSuite) TestDeletePoolByIDErrUnauthorized() {
+	err := s.Runner.DeletePoolByID(context.Background(), "dummy-pool-id")
+
+	s.Require().NotNil(err)
+	s.Require().Equal(runnerErrors.ErrUnauthorized, err)
+}
+
+func (s *PoolTestSuite) TestDeletePoolByIDRunnersFailed() {
+	_, err := s.Fixtures.Store.CreateInstance(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID, s.Fixtures.CreateInstanceParams)
+	if err != nil {
+		s.FailNow(fmt.Sprintf("cannot create instance: %s", err))
+	}
+
+	err = s.Runner.DeletePoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID)
+	s.Require().NotNil(err)
+	s.Require().Equal(runnerErrors.NewBadRequestError("pool has runners"), err)
+}
+
+func (s *PoolTestSuite) TestUpdatePoolByIDErrUnauthorized() {
+	_, err := s.Runner.UpdatePoolByID(context.Background(), "dummy-pool-id", s.Fixtures.UpdatePoolParams)
+
+	s.Require().NotNil(err)
+	s.Require().Equal(runnerErrors.ErrUnauthorized, err)
+}
+
+func (s *PoolTestSuite) TestTestUpdatePoolByIDInvalidPoolID() {
+	_, err := s.Runner.UpdatePoolByID(s.Fixtures.AdminContext, "dummy-pool-id", s.Fixtures.UpdatePoolParams)
+
+	s.Require().NotNil(err)
+	s.Require().Equal("fetching pool: fetching pool by ID: parsing id: invalid request", err.Error())
+}
+
+func (s *PoolTestSuite) TestTestUpdatePoolByIDRunnerBootstrapTimeoutFailed() {
+	// this is already created in `SetupTest()`
+	var RunnerBootstrapTimeout uint = 0
+	s.Fixtures.UpdatePoolParams.RunnerBootstrapTimeout = &RunnerBootstrapTimeout
+
+	_, err := s.Runner.UpdatePoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID, s.Fixtures.UpdatePoolParams)
+
+	s.Require().NotNil(err)
+	s.Require().Equal(runnerErrors.NewBadRequestError("runner_bootstrap_timeout cannot be 0"), err)
+}
+
+func (s *PoolTestSuite) TestTestUpdatePoolByIDMinIdleGreaterThanMax() {
+	var maxRunners uint = 10
+	var minIdleRunners uint = 11
+	s.Fixtures.UpdatePoolParams.MaxRunners = &maxRunners
+	s.Fixtures.UpdatePoolParams.MinIdleRunners = &minIdleRunners
+
+	_, err := s.Runner.UpdatePoolByID(s.Fixtures.AdminContext, s.Fixtures.Pools[0].ID, s.Fixtures.UpdatePoolParams)
+
+	s.Require().NotNil(err)
+	s.Require().Equal(runnerErrors.NewBadRequestError("min_idle_runners cannot be larger than max_runners"), err)
+}
+
+func TestPoolTestSuite(t *testing.T) {
+	suite.Run(t, new(PoolTestSuite))
+}
--- a/runner/providers/common/common.go
+++ b/runner/providers/common/common.go
@ -35,6 +35,7 @@ const (
 	RunnerActive     RunnerStatus = "active"
 )

+// IsValidStatus checks if the given status is valid.
 func IsValidStatus(status InstanceStatus) bool {
 	switch status {
 	case InstanceRunning, InstanceError, InstancePendingCreate,
@ -46,3 +47,17 @@ func IsValidStatus(status InstanceStatus) bool {
 		return false
 	}
 }
+
+// IsProviderValidStatus checks if the given status is valid for the provider.
+// A provider should only return a status indicating that the instance is in a
+// lifecycle state that it can influence. The sole purpose of a provider is to
+// manage the lifecycle of an instance. Statuses that indicate an instance should
+// be created or removed, will be set by the controller.
+func IsValidProviderStatus(status InstanceStatus) bool {
+	switch status {
+	case InstanceRunning, InstanceError, InstanceStopped:
+		return true
+	default:
+		return false
+	}
+}
--- a/runner/providers/external/external.go
+++ b/runner/providers/external/external.go
@ -44,21 +44,21 @@ type external struct {
 	execPath     string
 }

-func (e *external) validateCreateResult(inst params.Instance) error {
+func (e *external) validateResult(inst params.Instance) error {
 	if inst.ProviderID == "" {
-		return garmErrors.NewProviderError("missing provider ID after create call")
+		return garmErrors.NewProviderError("missing provider ID")
 	}

 	if inst.Name == "" {
-		return garmErrors.NewProviderError("missing instance name after create call")
+		return garmErrors.NewProviderError("missing instance name")
 	}

 	if inst.OSName == "" || inst.OSArch == "" || inst.OSType == "" {
 		// we can still function without this info (I think)
-		log.Printf("WARNING: missing OS information after create call")
+		log.Printf("WARNING: missing OS information")
 	}
-	if !providerCommon.IsValidStatus(inst.Status) {
-		return garmErrors.NewProviderError("invalid status returned (%s) after create call", inst.Status)
+	if !providerCommon.IsValidProviderStatus(inst.Status) {
+		return garmErrors.NewProviderError("invalid status returned (%s)", inst.Status)
 	}

 	return nil
@ -88,7 +88,7 @@ func (e *external) CreateInstance(ctx context.Context, bootstrapParams params.Bo
 		return params.Instance{}, garmErrors.NewProviderError("failed to decode response from binary: %s", err)
 	}

-	if err := e.validateCreateResult(param); err != nil {
+	if err := e.validateResult(param); err != nil {
 		return params.Instance{}, garmErrors.NewProviderError("failed to validate result: %s", err)
 	}

@ -137,6 +137,11 @@ func (e *external) GetInstance(ctx context.Context, instance string) (params.Ins
 	if err := json.Unmarshal(out, &param); err != nil {
 		return params.Instance{}, garmErrors.NewProviderError("failed to decode response from binary: %s", err)
 	}
+
+	if err := e.validateResult(param); err != nil {
+		return params.Instance{}, garmErrors.NewProviderError("failed to validate result: %s", err)
+	}
+
 	return param, nil
 }

@ -158,6 +163,12 @@ func (e *external) ListInstances(ctx context.Context, poolID string) ([]params.I
 	if err := json.Unmarshal(out, &param); err != nil {
 		return []params.Instance{}, garmErrors.NewProviderError("failed to decode response from binary: %s", err)
 	}
+
+	for _, inst := range param {
+		if err := e.validateResult(inst); err != nil {
+			return []params.Instance{}, garmErrors.NewProviderError("failed to validate result: %s", err)
+		}
+	}
 	return param, nil
 }

--- a/runner/providers/lxd/lxd.go
+++ b/runner/providers/lxd/lxd.go
@ -236,6 +236,7 @@ func (l *LXD) getCreateInstanceArgs(bootstrapParams params.BootstrapInstance, sp
 	}

 	bootstrapParams.UserDataOptions.DisableUpdatesOnBoot = specs.DisableUpdates
+	bootstrapParams.UserDataOptions.ExtraPackages = specs.ExtraPackages
 	cloudCfg, err := util.GetCloudConfig(bootstrapParams, tools, bootstrapParams.Name)
 	if err != nil {
 		return api.InstancesPost{}, errors.Wrap(err, "generating cloud-config")
--- a/runner/providers/lxd/specs.go
+++ b/runner/providers/lxd/specs.go
@ -22,7 +22,8 @@ import (
 )

 type extraSpecs struct {
-	DisableUpdates bool `json:"disable_updates"`
+	DisableUpdates bool     `json:"disable_updates"`
+	ExtraPackages  []string `json:"extra_packages"`
 }

 func parseExtraSpecsFromBootstrapParams(bootstrapParams params.BootstrapInstance) (extraSpecs, error) {
--- a/util/util.go
+++ b/util/util.go
@ -271,6 +271,9 @@ func GetCloudConfig(bootstrapParams params.BootstrapInstance, tools github.Runne
 			cloudCfg.PackageUpgrade = false
 			cloudCfg.Packages = []string{}
 		}
+		for _, pkg := range bootstrapParams.UserDataOptions.ExtraPackages {
+			cloudCfg.AddPackage(pkg)
+		}

 		cloudCfg.AddSSHKey(bootstrapParams.SSHKeys...)
 		cloudCfg.AddFile(installScript, "/install_runner.sh", "root:root", "755")