From f7c336f9ddb7b7f4cb59e689b07da54c8b308c32 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 25 Aug 2022 04:41:55 +0000 Subject: [PATCH 01/13] e2e: Mention maintained versions of cert-manager for reference --- test/e2e/e2e_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 36faf6bd..750296cb 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -25,6 +25,8 @@ const ( ) var ( + // See the below link for maintained versions of cert-manager + // https://cert-manager.io/docs/installation/supported-releases/ certManagerVersion = "v1.8.2" images = []testing.ContainerImage{ From f70f325f488e3d827538c356aa5a67c9dc0dafdc Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 25 Aug 2022 04:43:03 +0000 Subject: [PATCH 02/13] e2e: Set ARC_E2E_DO_DOCKER_BUILD to verify docker-build --- test/e2e/e2e_test.go | 114 ++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 55 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 750296cb..66b3af4d 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -121,6 +121,7 @@ func TestE2E(t *testing.T) { t.Fatalf("Failed to parse duration %q: %v", vt, err) } } + env.doDockerBuild = os.Getenv("ARC_E2E_DO_DOCKER_BUILD") != "" t.Run("build and load images", func(t *testing.T) { env.buildAndLoadImages(t) @@ -317,6 +318,7 @@ type env struct { minReplicas int64 dockerdWithinRunnerContainer bool rootlessDocker bool + doDockerBuild bool containerMode string remoteKubeconfig string imagePullSecretName string @@ -732,7 +734,7 @@ func (e *env) createControllerNamespaceAndServiceAccount(t *testing.T) { func (e *env) installActionsWorkflow(t *testing.T, kind DeployKind, testID string) { t.Helper() - installActionsWorkflow(t, e.testName+" "+testID, e.runnerLabel(testID), testResultCMNamePrefix, e.repoToCommit, kind, e.testJobs(testID), !e.rootlessDocker) + installActionsWorkflow(t, e.testName+" "+testID, e.runnerLabel(testID), testResultCMNamePrefix, e.repoToCommit, kind, e.testJobs(testID), !e.rootlessDocker, e.doDockerBuild) } func (e *env) testJobs(testID string) []job { @@ -774,7 +776,7 @@ func createTestJobs(id, testResultCMNamePrefix string, numJobs int) []job { const Branch = "main" // useSudo also implies rootful docker and the use of buildx cache export/import -func installActionsWorkflow(t *testing.T, testName, runnerLabel, testResultCMNamePrefix, testRepo string, kind DeployKind, testJobs []job, useSudo bool) { +func installActionsWorkflow(t *testing.T, testName, runnerLabel, testResultCMNamePrefix, testRepo string, kind DeployKind, testJobs []job, useSudo, doDockerBuild bool) { t.Helper() ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) @@ -873,75 +875,77 @@ func installActionsWorkflow(t *testing.T, testName, runnerLabel, testResultCMNam }, ) - if !kubernetesContainerMode { - setupBuildXActionWith := &testing.With{ - BuildkitdFlags: "--debug", - Endpoint: "mycontext", - // As the consequence of setting `install: false`, it doesn't install buildx as an alias to `docker build` - // so we need to use `docker buildx build` in the next step - Install: false, - } - var dockerBuildCache, dockerfile string - if useSudo { - // This needs to be set only when rootful docker mode. - // When rootless, we need to use the `docker` buildx driver, which doesn't support cache export - // so we end up with the below error on docker-build: - // error: cache export feature is currently not supported for docker driver. Please switch to a different driver (eg. "docker buildx create --use") - dockerBuildCache = "--cache-from=type=local,src=/home/runner/.cache/buildx " + - "--cache-to=type=local,dest=/home/runner/.cache/buildx-new,mode=max " - dockerfile = "Dockerfile" - } else { - setupBuildXActionWith.Driver = "docker" - dockerfile = "Dockerfile.nocache" - } - steps = append(steps, - testing.Step{ - // https://github.com/docker/buildx/issues/413#issuecomment-710660155 - // To prevent setup-buildx-action from failing with: - // error: could not create a builder instance with TLS data loaded from environment. Please use `docker context create ` to create a context for current environment and then create a builder instance with `docker buildx create ` - Run: "docker context create mycontext", - }, - testing.Step{ - Run: "docker context use mycontext", - }, - testing.Step{ - Name: "Set up Docker Buildx", - Uses: "docker/setup-buildx-action@v1", - With: setupBuildXActionWith, - }, - testing.Step{ - Run: "docker buildx build --platform=linux/amd64 " + - dockerBuildCache + - fmt.Sprintf("-f %s .", dockerfile), - }, - ) - - if useSudo { + if doDockerBuild { + if !kubernetesContainerMode { + setupBuildXActionWith := &testing.With{ + BuildkitdFlags: "--debug", + Endpoint: "mycontext", + // As the consequence of setting `install: false`, it doesn't install buildx as an alias to `docker build` + // so we need to use `docker buildx build` in the next step + Install: false, + } + var dockerBuildCache, dockerfile string + if useSudo { + // This needs to be set only when rootful docker mode. + // When rootless, we need to use the `docker` buildx driver, which doesn't support cache export + // so we end up with the below error on docker-build: + // error: cache export feature is currently not supported for docker driver. Please switch to a different driver (eg. "docker buildx create --use") + dockerBuildCache = "--cache-from=type=local,src=/home/runner/.cache/buildx " + + "--cache-to=type=local,dest=/home/runner/.cache/buildx-new,mode=max " + dockerfile = "Dockerfile" + } else { + setupBuildXActionWith.Driver = "docker" + dockerfile = "Dockerfile.nocache" + } steps = append(steps, testing.Step{ - // https://github.com/docker/build-push-action/blob/master/docs/advanced/cache.md#local-cache - // See https://github.com/moby/buildkit/issues/1896 for why this is needed - Run: "rm -rf /home/runner/.cache/buildx && mv /home/runner/.cache/buildx-new /home/runner/.cache/buildx", + // https://github.com/docker/buildx/issues/413#issuecomment-710660155 + // To prevent setup-buildx-action from failing with: + // error: could not create a builder instance with TLS data loaded from environment. Please use `docker context create ` to create a context for current environment and then create a builder instance with `docker buildx create ` + Run: "docker context create mycontext", }, testing.Step{ - Run: "ls -lah /home/runner/.cache/*", + Run: "docker context use mycontext", + }, + testing.Step{ + Name: "Set up Docker Buildx", + Uses: "docker/setup-buildx-action@v1", + With: setupBuildXActionWith, + }, + testing.Step{ + Run: "docker buildx build --platform=linux/amd64 " + + dockerBuildCache + + fmt.Sprintf("-f %s .", dockerfile), }, ) } + } + if useSudo { steps = append(steps, testing.Step{ - Uses: "azure/setup-kubectl@v1", - With: &testing.With{ - Version: "v1.20.2", - }, + // https://github.com/docker/build-push-action/blob/master/docs/advanced/cache.md#local-cache + // See https://github.com/moby/buildkit/issues/1896 for why this is needed + Run: "rm -rf /home/runner/.cache/buildx && mv /home/runner/.cache/buildx-new /home/runner/.cache/buildx", }, testing.Step{ - Run: fmt.Sprintf("./test.sh %s %s", t.Name(), j.testArg), + Run: "ls -lah /home/runner/.cache/*", }, ) } + steps = append(steps, + testing.Step{ + Uses: "azure/setup-kubectl@v1", + With: &testing.With{ + Version: "v1.20.2", + }, + }, + testing.Step{ + Run: fmt.Sprintf("./test.sh %s %s", t.Name(), j.testArg), + }, + ) + wf.Jobs[j.name] = testing.Job{ RunsOn: runnerLabel, Container: container, From 6ef276b239be16767bc515a67542e5c8b9feaef8 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 25 Aug 2022 04:44:22 +0000 Subject: [PATCH 03/13] e2e: Custom RBAC resources for make test success reporting work when k8s container mode or runner update hook is enabled --- acceptance/deploy_runners.sh | 2 ++ .../testdata/kubernetes_container_mode.envsubst.yaml | 10 +++++++--- acceptance/testdata/runnerdeploy.envsubst.yaml | 1 + acceptance/testdata/runnerset.envsubst.yaml | 1 + test/e2e/e2e_test.go | 6 ++++++ 5 files changed, 17 insertions(+), 3 deletions(-) diff --git a/acceptance/deploy_runners.sh b/acceptance/deploy_runners.sh index 0c0ecfbe..bb8a21d4 100755 --- a/acceptance/deploy_runners.sh +++ b/acceptance/deploy_runners.sh @@ -6,6 +6,8 @@ OP=${OP:-apply} RUNNER_LABEL=${RUNNER_LABEL:-self-hosted} +cat acceptance/testdata/kubernetes_container_mode.envsubst.yaml | NAMESPACE=${RUNNER_NAMESPACE} envsubst | kubectl apply -f - + if [ -n "${TEST_REPO}" ]; then if [ "${USE_RUNNERSET}" != "false" ]; then cat acceptance/testdata/runnerset.envsubst.yaml | TEST_ENTERPRISE= TEST_ORG= RUNNER_MIN_REPLICAS=${REPO_RUNNER_MIN_REPLICAS} NAME=repo-runnerset envsubst | kubectl ${OP} -f - diff --git a/acceptance/testdata/kubernetes_container_mode.envsubst.yaml b/acceptance/testdata/kubernetes_container_mode.envsubst.yaml index 6c7b4465..43d63c0e 100644 --- a/acceptance/testdata/kubernetes_container_mode.envsubst.yaml +++ b/acceptance/testdata/kubernetes_container_mode.envsubst.yaml @@ -20,6 +20,10 @@ rules: - apiGroups: [""] resources: ["secrets"] verbs: ["get", "list", "create", "delete"] +# Needed to report test success by crating a cm from within workflow job step +- apiGroups: [""] + resources: ["configmaps"] + verbs: ["create", "delete"] --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole @@ -33,7 +37,7 @@ rules: apiVersion: v1 kind: ServiceAccount metadata: - name: runner + name: ${RUNNER_SERVICE_ACCOUNT_NAME} namespace: ${NAMESPACE} --- # To verify it's working, try: @@ -50,7 +54,7 @@ metadata: namespace: ${NAMESPACE} subjects: - kind: ServiceAccount - name: runner + name: ${RUNNER_SERVICE_ACCOUNT_NAME} namespace: ${NAMESPACE} roleRef: kind: ClusterRole @@ -64,7 +68,7 @@ metadata: namespace: ${NAMESPACE} subjects: - kind: ServiceAccount - name: runner + name: ${RUNNER_SERVICE_ACCOUNT_NAME} namespace: ${NAMESPACE} roleRef: kind: ClusterRole diff --git a/acceptance/testdata/runnerdeploy.envsubst.yaml b/acceptance/testdata/runnerdeploy.envsubst.yaml index a76f38d3..f99e39d3 100644 --- a/acceptance/testdata/runnerdeploy.envsubst.yaml +++ b/acceptance/testdata/runnerdeploy.envsubst.yaml @@ -64,6 +64,7 @@ spec: resources: requests: storage: 10Gi + serviceAccountName: ${RUNNER_SERVICE_ACCOUNT_NAME} --- apiVersion: actions.summerwind.dev/v1alpha1 kind: HorizontalRunnerAutoscaler diff --git a/acceptance/testdata/runnerset.envsubst.yaml b/acceptance/testdata/runnerset.envsubst.yaml index 1809f483..b54a5d63 100644 --- a/acceptance/testdata/runnerset.envsubst.yaml +++ b/acceptance/testdata/runnerset.envsubst.yaml @@ -112,6 +112,7 @@ spec: labels: app: ${NAME} spec: + serviceAccountName: ${RUNNER_SERVICE_ACCOUNT_NAME} containers: - name: runner imagePullPolicy: IfNotPresent diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 66b3af4d..45fd43ae 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -320,6 +320,8 @@ type env struct { rootlessDocker bool doDockerBuild bool containerMode string + runnerServiceAccuontName string + runnerNamespace string remoteKubeconfig string imagePullSecretName string imagePullPolicy string @@ -448,6 +450,8 @@ func initTestEnv(t *testing.T, k8sMinorVer string, vars vars) *env { e.testOrgRepo = testing.Getenv(t, "TEST_ORG_REPO", "") e.testEnterprise = testing.Getenv(t, "TEST_ENTERPRISE", "") e.testEphemeral = testing.Getenv(t, "TEST_EPHEMERAL", "") + e.runnerServiceAccuontName = testing.Getenv(t, "TEST_RUNNER_SERVICE_ACCOUNT_NAME", "") + e.runnerNamespace = testing.Getenv(t, "TEST_RUNNER_NAMESPACE", "default") e.remoteKubeconfig = testing.Getenv(t, "ARC_E2E_REMOTE_KUBECONFIG", "") e.imagePullSecretName = testing.Getenv(t, "ARC_E2E_IMAGE_PULL_SECRET_NAME", "") e.vars = vars @@ -642,6 +646,8 @@ func (e *env) do(t *testing.T, op string, kind DeployKind, testID string) { scriptEnv := []string{ "KUBECONFIG=" + e.Kubeconfig, "OP=" + op, + "RUNNER_NAMESPACE=" + e.runnerNamespace, + "RUNNER_SERVICE_ACCOUNT_NAME=" + e.runnerServiceAccuontName, } switch kind { From ebcd83850173f82ee0a96090bb35c5b2fc27eafe Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 26 Aug 2022 01:28:00 +0000 Subject: [PATCH 04/13] e2e: Continuous rolling-update of runners while workflow jobs are running This should help revealing issues like https://github.com/actions-runner-controller/actions-runner-controller/issues/1535 if any. --- .../testdata/runnerdeploy.envsubst.yaml | 4 ++ acceptance/testdata/runnerset.envsubst.yaml | 2 + test/e2e/e2e_test.go | 49 +++++++++++++++++-- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/acceptance/testdata/runnerdeploy.envsubst.yaml b/acceptance/testdata/runnerdeploy.envsubst.yaml index f99e39d3..69402111 100644 --- a/acceptance/testdata/runnerdeploy.envsubst.yaml +++ b/acceptance/testdata/runnerdeploy.envsubst.yaml @@ -49,6 +49,10 @@ spec: labels: - "${RUNNER_LABEL}" + env: + - name: ROLLING_UPDATE_PHASE + value: "${ROLLING_UPDATE_PHASE}" + # # Non-standard working directory # diff --git a/acceptance/testdata/runnerset.envsubst.yaml b/acceptance/testdata/runnerset.envsubst.yaml index b54a5d63..9a315a0d 100644 --- a/acceptance/testdata/runnerset.envsubst.yaml +++ b/acceptance/testdata/runnerset.envsubst.yaml @@ -121,6 +121,8 @@ spec: value: "${RUNNER_FEATURE_FLAG_EPHEMERAL}" - name: GOMODCACHE value: "/home/runner/.cache/go-mod" + - name: ROLLING_UPDATE_PHASE + value: "${ROLLING_UPDATE_PHASE}" # PV-backed runner work dir volumeMounts: # Comment out the ephemeral work volume if you're going to test the kubernetes container mode diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 45fd43ae..c82f5cf6 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -205,6 +205,27 @@ func TestE2E(t *testing.T) { } } + ctx, cancel := context.WithCancel(context.Background()) + go func() { + for i := 1; ; i++ { + select { + case _, ok := <-ctx.Done(): + if !ok { + t.Logf("Stopping the continuous rolling-update of runners") + } + default: + time.Sleep(10 * time.Second) + + t.Run(fmt.Sprintf("update runners attempt %d", i), func(t *testing.T) { + env.deploy(t, RunnerSets, testID, fmt.Sprintf("ROLLING_UPDATE_PHASE=%d", i)) + }) + } + } + }() + t.Cleanup(func() { + cancel() + }) + t.Run("Install workflow", func(t *testing.T) { env.installActionsWorkflow(t, RunnerSets, testID) }) @@ -280,6 +301,27 @@ func TestE2E(t *testing.T) { } } + ctx, cancel := context.WithCancel(context.Background()) + go func() { + for i := 1; ; i++ { + select { + case _, ok := <-ctx.Done(): + if !ok { + t.Logf("Stopping the continuous rolling-update of runners") + } + default: + time.Sleep(10 * time.Second) + + t.Run(fmt.Sprintf("update runners - attempt %d", i), func(t *testing.T) { + env.deploy(t, RunnerDeployments, testID, fmt.Sprintf("ROLLING_UPDATE_PHASE=%d", i)) + }) + } + } + }() + t.Cleanup(func() { + cancel() + }) + t.Run("Install workflow", func(t *testing.T) { env.installActionsWorkflow(t, RunnerDeployments, testID) }) @@ -628,9 +670,9 @@ func (e *env) installActionsRunnerController(t *testing.T, repo, tag, testID, ch e.RunScript(t, "../../acceptance/deploy.sh", testing.ScriptConfig{Dir: "../..", Env: scriptEnv}) } -func (e *env) deploy(t *testing.T, kind DeployKind, testID string) { +func (e *env) deploy(t *testing.T, kind DeployKind, testID string, env ...string) { t.Helper() - e.do(t, "apply", kind, testID) + e.do(t, "apply", kind, testID, env...) } func (e *env) undeploy(t *testing.T, kind DeployKind, testID string) { @@ -638,7 +680,7 @@ func (e *env) undeploy(t *testing.T, kind DeployKind, testID string) { e.do(t, "delete", kind, testID) } -func (e *env) do(t *testing.T, op string, kind DeployKind, testID string) { +func (e *env) do(t *testing.T, op string, kind DeployKind, testID string, env ...string) { t.Helper() e.createControllerNamespaceAndServiceAccount(t) @@ -649,6 +691,7 @@ func (e *env) do(t *testing.T, op string, kind DeployKind, testID string) { "RUNNER_NAMESPACE=" + e.runnerNamespace, "RUNNER_SERVICE_ACCOUNT_NAME=" + e.runnerServiceAccuontName, } + scriptEnv = append(scriptEnv, env...) switch kind { case RunnerSets: From 5c1be3265b083d1df311d333f916c870e31e0fde Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 26 Aug 2022 01:48:36 +0000 Subject: [PATCH 05/13] e2e: Fix the token check to actually fail on expiration --- test/e2e/e2e_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index c82f5cf6..81ae3889 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -557,9 +557,9 @@ func (e *env) checkGitHubToken(t *testing.T, tok string) error { c := github.NewClient(&http.Client{Transport: transport}) aa, res, err := c.Octocat(context.Background(), "hello") if err != nil { - b, err := io.ReadAll(res.Body) - if err != nil { - t.Logf("%v", err) + b, ioerr := io.ReadAll(res.Body) + if ioerr != nil { + t.Logf("%v", ioerr) return err } t.Logf(string(b)) @@ -569,9 +569,9 @@ func (e *env) checkGitHubToken(t *testing.T, tok string) error { t.Logf("%s", aa) if _, res, err := c.Actions.CreateRegistrationToken(ctx, e.testOrg, e.testOrgRepo); err != nil { - b, err := io.ReadAll(res.Body) - if err != nil { - t.Logf("%v", err) + b, ioerr := io.ReadAll(res.Body) + if ioerr != nil { + t.Logf("%v", ioerr) return err } t.Logf(string(b)) From dbd668ae2d3b67a6f742b5c34e92a28a4ebbe6a8 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Fri, 26 Aug 2022 01:48:54 +0000 Subject: [PATCH 06/13] e2e: Set ARC_E2E_SKIP_RUNNERDEPLOYMENT to skip RunnerDeployment test --- test/e2e/e2e_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 81ae3889..ff12912e 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -240,6 +240,10 @@ func TestE2E(t *testing.T) { }) t.Run("RunnerDeployments", func(t *testing.T) { + if os.Getenv("ARC_E2E_SKIP_RUNNERDEPLOYMENT") != "" { + t.Skip("RunnerSets test has been skipped due to ARC_E2E_SKIP_RUNNERSETS") + } + var ( testID string ) From c143fd50b5a27de11639c2f045a34af96c13f3a9 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 27 Aug 2022 07:07:56 +0000 Subject: [PATCH 07/13] e2e: Use newer version of actions/runner(0.296.0) --- test/e2e/e2e_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index ff12912e..0d3b70b8 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -38,6 +38,8 @@ var ( } testResultCMNamePrefix = "test-result-" + + RunnerVersion = "2.296.0" ) // If you're willing to run this test via VS Code "run test" or "debug test", @@ -435,7 +437,7 @@ func buildVars(repo string) vars { Args: []testing.BuildArg{ { Name: "RUNNER_VERSION", - Value: "2.294.0", + Value: RunnerVersion, }, }, Image: runnerImage, @@ -446,7 +448,7 @@ func buildVars(repo string) vars { Args: []testing.BuildArg{ { Name: "RUNNER_VERSION", - Value: "2.294.0", + Value: RunnerVersion, }, }, Image: runnerDindImage, @@ -457,7 +459,7 @@ func buildVars(repo string) vars { Args: []testing.BuildArg{ { Name: "RUNNER_VERSION", - Value: "2.294.0", + Value: RunnerVersion, }, }, Image: runnerRootlessDindImage, From 4925880e5e2b2f06230d3a93774a5120d60bb0a4 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 27 Aug 2022 07:08:56 +0000 Subject: [PATCH 08/13] e2e: Install workflow before starting continuous rolling-updates of runners --- test/e2e/e2e_test.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 0d3b70b8..85c7df19 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -207,6 +207,14 @@ func TestE2E(t *testing.T) { } } + t.Run("Install workflow", func(t *testing.T) { + env.installActionsWorkflow(t, RunnerSets, testID) + }) + + if t.Failed() { + return + } + ctx, cancel := context.WithCancel(context.Background()) go func() { for i := 1; ; i++ { @@ -228,14 +236,6 @@ func TestE2E(t *testing.T) { cancel() }) - t.Run("Install workflow", func(t *testing.T) { - env.installActionsWorkflow(t, RunnerSets, testID) - }) - - if t.Failed() { - return - } - t.Run("Verify workflow run result", func(t *testing.T) { env.verifyActionsWorkflowRun(t, testID) }) @@ -307,6 +307,14 @@ func TestE2E(t *testing.T) { } } + t.Run("Install workflow", func(t *testing.T) { + env.installActionsWorkflow(t, RunnerDeployments, testID) + }) + + if t.Failed() { + return + } + ctx, cancel := context.WithCancel(context.Background()) go func() { for i := 1; ; i++ { @@ -328,14 +336,6 @@ func TestE2E(t *testing.T) { cancel() }) - t.Run("Install workflow", func(t *testing.T) { - env.installActionsWorkflow(t, RunnerDeployments, testID) - }) - - if t.Failed() { - return - } - t.Run("Verify workflow run result", func(t *testing.T) { env.verifyActionsWorkflowRun(t, testID) }) From 915739b9721c36dcd23275a49647803e6d81c959 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 27 Aug 2022 07:10:10 +0000 Subject: [PATCH 09/13] e2e: Fix broken token expiration checks --- test/e2e/e2e_test.go | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 85c7df19..32576da0 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "time" "github.com/actions-runner-controller/actions-runner-controller/testing" @@ -574,14 +575,42 @@ func (e *env) checkGitHubToken(t *testing.T, tok string) error { t.Logf("%s", aa) - if _, res, err := c.Actions.CreateRegistrationToken(ctx, e.testOrg, e.testOrgRepo); err != nil { - b, ioerr := io.ReadAll(res.Body) - if ioerr != nil { - t.Logf("%v", ioerr) + if e.testEnterprise != "" { + if _, res, err := c.Enterprise.CreateRegistrationToken(ctx, e.testEnterprise); err != nil { + b, ioerr := io.ReadAll(res.Body) + if ioerr != nil { + t.Logf("%v", ioerr) + return err + } + t.Logf(string(b)) + return err + } + } + + if e.testOrg != "" { + if _, res, err := c.Actions.CreateOrganizationRegistrationToken(ctx, e.testOrg); err != nil { + b, ioerr := io.ReadAll(res.Body) + if ioerr != nil { + t.Logf("%v", ioerr) + return err + } + t.Logf(string(b)) + return err + } + } + + if e.testRepo != "" { + s := strings.Split(e.testRepo, "/") + owner, repo := s[0], s[1] + if _, res, err := c.Actions.CreateRegistrationToken(ctx, owner, repo); err != nil { + b, ioerr := io.ReadAll(res.Body) + if ioerr != nil { + t.Logf("%v", ioerr) + return err + } + t.Logf(string(b)) return err } - t.Logf(string(b)) - return err } return nil From e0a7be253e447ee5637b406a091c35192b0ec2fb Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 27 Aug 2022 07:11:17 +0000 Subject: [PATCH 10/13] e2e: Change the default runner rolling-update interval from 10s to 60s to let the runners actually get jobs assigned by GitHub Actions --- test/e2e/e2e_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 32576da0..0e4073ae 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -225,7 +225,7 @@ func TestE2E(t *testing.T) { t.Logf("Stopping the continuous rolling-update of runners") } default: - time.Sleep(10 * time.Second) + time.Sleep(60 * time.Second) t.Run(fmt.Sprintf("update runners attempt %d", i), func(t *testing.T) { env.deploy(t, RunnerSets, testID, fmt.Sprintf("ROLLING_UPDATE_PHASE=%d", i)) From f73713859c1c1307874a86f3edbaf76133aee174 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 27 Aug 2022 07:12:06 +0000 Subject: [PATCH 11/13] e2e: Fix workflow for rootless-dind test to actually pass --- test/e2e/e2e_test.go | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 0e4073ae..4e21edb9 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -915,32 +915,30 @@ func installActionsWorkflow(t *testing.T, testName, runnerLabel, testResultCMNam } } - steps = append(steps, - testing.Step{ - // This might be the easiest way to handle permissions without use of securityContext - // https://stackoverflow.com/questions/50156124/kubernetes-nfs-persistent-volumes-permission-denied#comment107483717_53186320 - Run: sudo + "chmod 777 -R \"${RUNNER_TOOL_CACHE}\" \"${HOME}/.cache\"", - }, - ) if useSudo { steps = append(steps, + testing.Step{ + // This might be the easiest way to handle permissions without use of securityContext + // https://stackoverflow.com/questions/50156124/kubernetes-nfs-persistent-volumes-permission-denied#comment107483717_53186320 + Run: sudo + "chmod 777 -R \"${RUNNER_TOOL_CACHE}\" \"${HOME}/.cache\"", + }, testing.Step{ Run: sudo + "chmod 777 -R \"/var/lib/docker\"", }, + testing.Step{ + // This might be the easiest way to handle permissions without use of securityContext + // https://stackoverflow.com/questions/50156124/kubernetes-nfs-persistent-volumes-permission-denied#comment107483717_53186320 + Run: "ls -lah \"${RUNNER_TOOL_CACHE}\" \"${HOME}/.cache\"", + }, + testing.Step{ + // This might be the easiest way to handle permissions without use of securityContext + // https://stackoverflow.com/questions/50156124/kubernetes-nfs-persistent-volumes-permission-denied#comment107483717_53186320 + Run: "ls -lah \"/var/lib/docker\" || echo ls failed.", + }, ) } steps = append(steps, - testing.Step{ - // This might be the easiest way to handle permissions without use of securityContext - // https://stackoverflow.com/questions/50156124/kubernetes-nfs-persistent-volumes-permission-denied#comment107483717_53186320 - Run: "ls -lah \"${RUNNER_TOOL_CACHE}\" \"${HOME}/.cache\"", - }, - testing.Step{ - // This might be the easiest way to handle permissions without use of securityContext - // https://stackoverflow.com/questions/50156124/kubernetes-nfs-persistent-volumes-permission-denied#comment107483717_53186320 - Run: "ls -lah \"/var/lib/docker\" || echo ls failed.", - }, testing.Step{ Uses: "actions/setup-go@v3", With: &testing.With{ From f8e07c7fe4edf3c20b6655e21c0732fc1dc95a80 Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 27 Aug 2022 07:12:55 +0000 Subject: [PATCH 12/13] e2e: Update RunnerSet template for rootless-dind test --- acceptance/testdata/runnerset.envsubst.yaml | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/acceptance/testdata/runnerset.envsubst.yaml b/acceptance/testdata/runnerset.envsubst.yaml index 9a315a0d..f9e6147e 100644 --- a/acceptance/testdata/runnerset.envsubst.yaml +++ b/acceptance/testdata/runnerset.envsubst.yaml @@ -155,19 +155,19 @@ spec: # https://github.com/actions/setup-go/blob/56a61c9834b4a4950dbbf4740af0b8a98c73b768/src/installer.ts#L144 mountPath: "/opt/hostedtoolcache" # Valid only when dockerdWithinRunnerContainer=false - - name: docker - # PV-backed runner work dir - volumeMounts: - - name: work - mountPath: /runner/_work - # Cache docker image layers, in case dockerdWithinRunnerContainer=false - - name: var-lib-docker - mountPath: /var/lib/docker - # image: mumoshu/actions-runner-dind:dev + # - name: docker + # # PV-backed runner work dir + # volumeMounts: + # - name: work + # mountPath: /runner/_work + # # Cache docker image layers, in case dockerdWithinRunnerContainer=false + # - name: var-lib-docker + # mountPath: /var/lib/docker + # # image: mumoshu/actions-runner-dind:dev - # For buildx cache - - name: cache - mountPath: "/home/runner/.cache" + # # For buildx cache + # - name: cache + # mountPath: "/home/runner/.cache" # Comment out the ephemeral work volume if you're going to test the kubernetes container mode # volumes: # - name: work From d4fb6204cb366821d6d7b62f1fb24905eb13e4df Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Sat, 27 Aug 2022 07:14:16 +0000 Subject: [PATCH 13/13] Add TODO comment to the PVC reconciler --- controllers/sync_volumes.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/controllers/sync_volumes.go b/controllers/sync_volumes.go index eb959338..9419c27f 100644 --- a/controllers/sync_volumes.go +++ b/controllers/sync_volumes.go @@ -75,6 +75,10 @@ func syncPVC(ctx context.Context, c client.Client, log logr.Logger, ns string, p log.V(2).Info("Reconciling runner PVC") + // TODO: Probably we'd better remove PVCs related to the RunnetSet that is nowhere now? + // Otherwise, a bunch of continuously recreated StatefulSet + // can leave dangling PVCs forever, which might stress the cluster. + var sts appsv1.StatefulSet if err := c.Get(ctx, types.NamespacedName{Namespace: ns, Name: stsName}, &sts); err != nil { if !kerrors.IsNotFound(err) {