rebased: Add Gitea Actions act runner #666

2024-06-11T16:14:43Z

vjm commented

2024-06-11 16:14:43 +00:00

(Migrated from gitea.com)

Please see #596

pat-s (Migrated from gitea.com) reviewed 2024-06-11 16:14:44 +00:00

pat-s commented

2024-06-17 09:02:52 +00:00

(Migrated from gitea.com)

@vjm Thanks for continuing!

Did you check on all review requests by @justusbunsi in #596?

Did you check this branch on a cluster install of yours?

@vjm Thanks for continuing! Did you check on all review requests by @justusbunsi in #596? Did you check this branch on a cluster install of yours?

Dunky13 commented

2024-06-18 16:50:36 +00:00

(Migrated from gitea.com)

I've been running the previous codebase fom #596 but I can't get it to work on k8s gitea running on RPI5's for docker build & push.

At that step in the action it fails to connect to dockerd (either through tcp 2376 or unix docker.sock)
Any clues here?
I want to build & push to gitea package manager.

For the act runner I'm using: gitea/act_runner@0.2.10-dind-rootless
For the dind container I'm using: docker@dind-rootless

Most other steps I got working; just not push to package manger:

name: Build docker container
on:
  push:
    branches:
      - main

jobs:
  build:
    name: Build image
    runs-on: ubuntu-latest
    container: 
      image: ghcr.io/catthehacker/ubuntu:act-latest
      options: -v /certs/:/certs/
    env:
      IMAGE_NAME: action-tester
      REGISTRY: git.mydomain
      REPO_OWNER: owner
    steps:
      - name: Checkout
        uses: actions/checkout@v4
      - name: Login to Docker Hub
        uses: docker/login-action@v3
        with:
          registry: git.mydomain.com
          username: ${{ secrets.CI_REGISTRY_USER }}
          password: ${{ secrets.CI_REGISTRY_PASSWORD }}
      - name: Build and push Docker image
        uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25
        with:
          context: .
          push: true
          tags: |
            ${{ env.REGISTRY }}/${{ env.REPO_OWNER }}/${{ env.IMAGE_NAME }}:latest            
          addLatest: true
        env:
          DOCKER_HOST: unix:///var/run/docker.sock #"tcp://127.0.0.1:2376"
          # DOCKER_TLS_CERTDIR: "/certs"
          # DOCKER_TLS_VERIFY: 1
          # DOCKER_CERT_PATH: "/certs/server"

I've been running the previous codebase fom #596 but I can't get it to work on k8s gitea running on RPI5's for docker build & push. At that step in the action it fails to connect to dockerd (either through tcp 2376 or unix docker.sock) Any clues here? I want to build & push to gitea package manager. For the act runner I'm using: `gitea/act_runner@0.2.10-dind-rootless` For the dind container I'm using: `docker@dind-rootless` Most other steps I got working; just not push to package manger: ``` name: Build docker container on: push: branches: - main jobs: build: name: Build image runs-on: ubuntu-latest container: image: ghcr.io/catthehacker/ubuntu:act-latest options: -v /certs/:/certs/ env: IMAGE_NAME: action-tester REGISTRY: git.mydomain REPO_OWNER: owner steps: - name: Checkout uses: actions/checkout@v4 - name: Login to Docker Hub uses: docker/login-action@v3 with: registry: git.mydomain.com username: ${{ secrets.CI_REGISTRY_USER }} password: ${{ secrets.CI_REGISTRY_PASSWORD }} - name: Build and push Docker image uses: docker/build-push-action@ca052bb54ab0790a636c9b5f226502c73d547a25 with: context: . push: true tags: | ${{ env.REGISTRY }}/${{ env.REPO_OWNER }}/${{ env.IMAGE_NAME }}:latest addLatest: true env: DOCKER_HOST: unix:///var/run/docker.sock #"tcp://127.0.0.1:2376" # DOCKER_TLS_CERTDIR: "/certs" # DOCKER_TLS_VERIFY: 1 # DOCKER_CERT_PATH: "/certs/server" ```

vjm commented

2024-06-24 17:10:01 +00:00

(Migrated from gitea.com)

hi @pat-s -- yes, I reviewed and here is where I think we are:

clean decomissioned runners -- unable to do this because there is no API. using statefulset avoids over-clutter.
_helpers.tpl work is completed i believe
rebase with with main is completed

did i miss anything on this? happy to try and work on this if there is something specific you think we need to address to get this merged.

hi @pat-s -- yes, I reviewed and here is where I think we are: - [ ] clean decomissioned runners -- unable to do this because there is no API. using statefulset avoids over-clutter. - [x] `_helpers.tpl` work is completed i believe - [ ] rebase with with main is completed did i miss anything on this? happy to try and work on this if there is something specific you think we need to address to get this merged.

vjm commented

2024-06-24 17:11:31 +00:00

(Migrated from gitea.com)

@Dunky13, i have been running this in my cluster successfully for several weeks with no issues, but not the rootless image. also worth considering whether moving to the buildx builder approach is better than continuing with dind?

Can you try with the root image instead of rootless and see if that works first?

@Dunky13, i have been running this in my cluster successfully for several weeks with no issues, but not the rootless image. also worth considering whether moving to the buildx builder approach is better than continuing with dind? Can you try with the root image instead of rootless and see if that works first?

Dunky13 commented

2024-06-24 17:16:08 +00:00

(Migrated from gitea.com)

@Dunky13, i have been running this in my cluster successfully for several weeks with no issues, but not the rootless image. also worth considering whether moving to the buildx builder approach is better than continuing with dind?

Can you try with the root image instead of rootless and see if that works first?

Do you have snippets of your values.yaml and the action for me to recreate? I haven't been able to do so yet :-( I tried multiple root(less) tcp/Unix permutations but unsuccessfully do with buildx

> @Dunky13, i have been running this in my cluster successfully for several weeks with no issues, but not the rootless image. also worth considering whether moving to the buildx builder approach is better than continuing with dind? > > Can you try with the root image instead of rootless and see if that works first? Do you have snippets of your values.yaml and the action for me to recreate? I haven't been able to do so yet :-( I tried multiple root(less) tcp/Unix permutations but unsuccessfully do with buildx

Dunky13 commented

2024-06-24 17:21:14 +00:00

(Migrated from gitea.com)

It seems that https://gitea.com/vjm/helm-chart/src/branch/gitea-actions/templates/gitea/act_runner/statefulset.yaml#L46
And L75 are also causing (or are part of) the issue

It seems that https://gitea.com/vjm/helm-chart/src/branch/gitea-actions/templates/gitea/act_runner/statefulset.yaml#L46 And L75 are also causing (or are part of) the issue

justusbunsi commented

2024-06-24 17:38:29 +00:00

(Migrated from gitea.com)

Sorry for the silence from my side. I am not sure if I can review earlier than this weekend.

vjm commented

2024-06-24 18:21:12 +00:00

(Migrated from gitea.com)

@Dunky13

values snippet is very straightforward:

# values.yaml
actions:
      enabled: true
      provisioning:
        enabled: true

current action:

# action.yml
on:
  push:
    tags:
      - '*'

env:
  DOCKER_REPO_NAME: "docker-repo-here"
  VERSION: "0.1.0"

jobs:
  build-and-push-latest:
    runs-on: ubuntu-latest
    container:
      image: "catthehacker/ubuntu:act-22.04"
    steps:
      - name: install tools
        run: |
          apt-get -yq update
          apt-get -yq install make zsh yamllint curl
      - name: Set up Docker BuildX
        uses: docker/setup-buildx-action@v3
      - uses: actions/checkout@v3
      - uses: actions/setup-node@v4
      - name: Setup Helm
        uses: azure/setup-helm@v3
      - uses: docker/login-action@v2
        with:
          registry: ${{ secrets.DOCKER_REGISTRY }}
          username: ${{ secrets.DOCKER_USERNAME }}
          password: ${{ secrets.DOCKER_PASSWORD }}
      - name: Build and push
        uses: docker/build-push-action@v6
        with:
          push: true
          context: .
          tags: ${{ secrets.DOCKER_REGISTRY }}/${{ secrets.DOCKER_USERNAME }}/${{ env.DOCKER_REPO_NAME }}:latest,${{ secrets.DOCKER_REGISTRY }}/${{ secrets.DOCKER_USERNAME }}/${{ env.DOCKER_REPO_NAME }}:${{ env.VERSION }}

@Dunky13 values snippet is very straightforward: ``` # values.yaml actions: enabled: true provisioning: enabled: true ``` current action: ``` # action.yml on: push: tags: - '*' env: DOCKER_REPO_NAME: "docker-repo-here" VERSION: "0.1.0" jobs: build-and-push-latest: runs-on: ubuntu-latest container: image: "catthehacker/ubuntu:act-22.04" steps: - name: install tools run: | apt-get -yq update apt-get -yq install make zsh yamllint curl - name: Set up Docker BuildX uses: docker/setup-buildx-action@v3 - uses: actions/checkout@v3 - uses: actions/setup-node@v4 - name: Setup Helm uses: azure/setup-helm@v3 - uses: docker/login-action@v2 with: registry: ${{ secrets.DOCKER_REGISTRY }} username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} - name: Build and push uses: docker/build-push-action@v6 with: push: true context: . tags: ${{ secrets.DOCKER_REGISTRY }}/${{ secrets.DOCKER_USERNAME }}/${{ env.DOCKER_REPO_NAME }}:latest,${{ secrets.DOCKER_REGISTRY }}/${{ secrets.DOCKER_USERNAME }}/${{ env.DOCKER_REPO_NAME }}:${{ env.VERSION }} ```

vjm commented

2024-06-29 15:11:02 +00:00

(Migrated from gitea.com)

@Dunky13 hopefully the above helped?

@justusbunsi hopefully the current state of things meets the bar to merge and we can address further enhancements as we go?

@Dunky13 hopefully the above helped? @justusbunsi hopefully the current state of things meets the bar to merge and we can address further enhancements as we go?

👍 1

justusbunsi (Migrated from gitea.com) reviewed 2024-06-30 12:32:46 +00:00

justusbunsi (Migrated from gitea.com) left a comment

@vjm Thanks for continuing the original approach. Please see my comments below.

I consider following topics blocking the PR merge:

Environment variable related handling in various files
Everything related to app.ini creation in _helpers.tpl
Handling of static values that should be in values.yaml

I consider following things not really blocking but it helping the Chart integrity and is therefore highly appreciated:

Unittests for new templates not only to check that the resources are rendered, but also verify that dynamic parts are rendered correctly (e.g. secret reference in act_runner/statefulset.yaml, placeholders for URLs, ...).

Following things are worth to talk about:

Helm Chart Hook mechanism vs. while-sleep loop
actions.provisioning.token object in values.yaml

@vjm Thanks for continuing the original approach. Please see my comments below. I consider following topics **blocking the PR merge**: - Environment variable related handling in various files - Everything related to app.ini creation in `_helpers.tpl` - Handling of static values that should be in values.yaml I consider following things **not really blocking but it helping the Chart integrity** and is therefore highly appreciated: - Unittests for new templates not only to check that the resources are rendered, but also verify that dynamic parts are rendered correctly (e.g. secret reference in act_runner/statefulset.yaml, placeholders for URLs, ...). Following things are **worth to talk about**: - Helm Chart Hook mechanism vs. while-sleep loop - `actions.provisioning.token` object in values.yaml

templates/_helpers.tpl

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:32:46 +00:00

https://gitea.com/gitea/helm-chart/pulls/596#issuecomment-813114 is still an open issue. If I use the PR changes and run helm template my-release ., I get following rendered diff compared to the current main branch:

diff --git a/rendered.yaml b/rendered.yaml
index 9027273..47d36a4 100644
--- a/rendered.yaml
+++ b/rendered.yaml
@@ -165,6 +165,10 @@ metadata:
 type: Opaque
 stringData:
   _generals_: ""
+  actions: |-
+    GITEA__ACTIONS__ENABLED=true
+    GITEA__INSTANCE__URL=http://my-release-gitea-http:3000
+    GITEA__SERVER__LOCAL_ROOT_URL=http://my-release-gitea-http:3000
   cache: |-
     ADAPTER=redis
     HOST=redis+cluster://:@my-release-redis-cluster-headless.default.svc.cluster.local:6379/0?pool_size=100&idle_timeout=180s&
@@ -3391,7 +3395,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: b10c6eb905bab8d363558afa9da4128cbe67821c4dd0f95f7ccc1ad5e4fd397c
+        checksum/config: 6fbc88429b6a693f40b4ee660bc273865d16f2027f32574cc5626a3ec45ad74e
       labels:
         helm.sh/chart: gitea-0.0.0
         app: gitea

This means, a default Helm release without enabled actions will somewhat modify the app.ini for actions.

Despite that, the given values are not valid in the app.ini structure; see https://docs.gitea.com/administration/config-cheat-sheet#actions-actions for valid options.
If I would add --set actions.enabled=true to the above helm command, it should set the following app.ini settings:

diff --git a/rendered.yaml b/rendered.yaml
index 9027273..47d36a4 100644
--- a/rendered.yaml
+++ b/rendered.yaml
@@ -165,6 +165,10 @@ metadata:
 type: Opaque
 stringData:
   _generals_: ""
+  actions: |-
+    ENABLED=true
+  server: |-
+    LOCAL_ROOT_URL=http://my-release-gitea-http:3000
   cache: |-
     ADAPTER=redis
     HOST=redis+cluster://:@my-release-redis-cluster-headless.default.svc.cluster.local:6379/0?pool_size=100&idle_timeout=180s&
@@ -3391,7 +3395,7 @@ spec:
   template:
     metadata:
       annotations:
-        checksum/config: b10c6eb905bab8d363558afa9da4128cbe67821c4dd0f95f7ccc1ad5e4fd397c
+        checksum/config: 6fbc88429b6a693f40b4ee660bc273865d16f2027f32574cc5626a3ec45ad74e
       labels:
         helm.sh/chart: gitea-0.0.0
         app: gitea

The GITEA__INSTANCE__URL environment variable is actually GITEA_INSTANCE_URL and only valid within templates/gitea/act_runner/statefulset.yaml.

https://gitea.com/gitea/helm-chart/pulls/596#issuecomment-813114 is still an open issue. If I use the PR changes and run `helm template my-release .`, I get following rendered diff compared to the current main branch: ```diff diff --git a/rendered.yaml b/rendered.yaml index 9027273..47d36a4 100644 --- a/rendered.yaml +++ b/rendered.yaml @@ -165,6 +165,10 @@ metadata: type: Opaque stringData: _generals_: "" + actions: |- + GITEA__ACTIONS__ENABLED=true + GITEA__INSTANCE__URL=http://my-release-gitea-http:3000 + GITEA__SERVER__LOCAL_ROOT_URL=http://my-release-gitea-http:3000 cache: |- ADAPTER=redis HOST=redis+cluster://:@my-release-redis-cluster-headless.default.svc.cluster.local:6379/0?pool_size=100&idle_timeout=180s& @@ -3391,7 +3395,7 @@ spec: template: metadata: annotations: - checksum/config: b10c6eb905bab8d363558afa9da4128cbe67821c4dd0f95f7ccc1ad5e4fd397c + checksum/config: 6fbc88429b6a693f40b4ee660bc273865d16f2027f32574cc5626a3ec45ad74e labels: helm.sh/chart: gitea-0.0.0 app: gitea ``` This means, a default Helm release without enabled actions will somewhat modify the app.ini for actions. Despite that, the given values are not valid in the app.ini structure; see https://docs.gitea.com/administration/config-cheat-sheet#actions-actions for valid options. If I would add `--set actions.enabled=true` to the above helm command, it should set the following app.ini settings: ```diff diff --git a/rendered.yaml b/rendered.yaml index 9027273..47d36a4 100644 --- a/rendered.yaml +++ b/rendered.yaml @@ -165,6 +165,10 @@ metadata: type: Opaque stringData: _generals_: "" + actions: |- + ENABLED=true + server: |- + LOCAL_ROOT_URL=http://my-release-gitea-http:3000 cache: |- ADAPTER=redis HOST=redis+cluster://:@my-release-redis-cluster-headless.default.svc.cluster.local:6379/0?pool_size=100&idle_timeout=180s& @@ -3391,7 +3395,7 @@ spec: template: metadata: annotations: - checksum/config: b10c6eb905bab8d363558afa9da4128cbe67821c4dd0f95f7ccc1ad5e4fd397c + checksum/config: 6fbc88429b6a693f40b4ee660bc273865d16f2027f32574cc5626a3ec45ad74e labels: helm.sh/chart: gitea-0.0.0 app: gitea ``` The `GITEA__INSTANCE__URL` environment variable is actually `GITEA_INSTANCE_URL` and only valid within `templates/gitea/act_runner/statefulset.yaml`.

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:03:01 +00:00

fixed!

diff official-helm-chart/helm-output.yaml helm-chart/helm-output.yaml
type: Opaque
 stringData:
   _generals_: ""
+   actions: ENABLED=false
+     LOCAL_ROOT_URL=http://my-release-gitea-http:3000
-         checksum/config: 0cfe1485ad6f53623b735da62dedbd132cd46abb2a2aa920a433ab8dde9499a7
template:
     metadata:
       annotations:
+         checksum/config: 8cf8240db21d9804533046d01b69bc77313df0f5362cd019e77d20b8902f0a06

fixed! ```diff diff official-helm-chart/helm-output.yaml helm-chart/helm-output.yaml type: Opaque stringData: _generals_: "" + actions: ENABLED=false + LOCAL_ROOT_URL=http://my-release-gitea-http:3000 - checksum/config: 0cfe1485ad6f53623b735da62dedbd132cd46abb2a2aa920a433ab8dde9499a7 template: metadata: annotations: + checksum/config: 8cf8240db21d9804533046d01b69bc77313df0f5362cd019e77d20b8902f0a06 ```

templates/gitea/act_runner/job.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:36:11 +00:00

We should allow customization via actions.provisioning.ttlSecondsAfterFinished

We should allow customization via `actions.provisioning.ttlSecondsAfterFinished`

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:06:42 +00:00

done!

templates/gitea/act_runner/job.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 13:01:54 +00:00

I am not fully sure, but are we able to wait with provisioning activity via Helm Chart hook mechanism? Instead of waiting for eternity? Or would that confuse tools like ArgoCD?

I imagine a flow like below:

Deploy the Gitea application itself
Wait for the application itself being healthy and ready to accept connections
Apply the hook-based provisioning job and being able to immediately contact Gitea

I am not fully sure, but are we able to wait with provisioning activity via Helm Chart hook mechanism? Instead of waiting for eternity? Or would that confuse tools like ArgoCD? I imagine a flow like below: - Deploy the Gitea application itself - Wait for the application itself being healthy and ready to accept connections - Apply the hook-based provisioning job and being able to immediately contact Gitea

justusbunsi (Migrated from gitea.com) commented

2024-07-24 21:02:21 +00:00

i just tried to test this out and i had some issues -- the post-install hook waited to be applied as expected, but the gitea application was still not ready, even though helm and kubernetes had marked it as ready -- this might still be possible but i'd prefer to push this to a separate PR if possible.

templates/gitea/act_runner/statefulset.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:52:39 +00:00

Please move this hard-coded image/tag reference into values.yaml. That way users can customize, and we can properly let Renovate update it.

Please move this hard-coded image/tag reference into `values.yaml`. That way users can customize, and we can properly let Renovate update it.

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:06:32 +00:00

done!

templates/gitea/act_runner/statefulset.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 13:05:31 +00:00

I guess we could reuse the already configured LOCAL_ROOT_URL from app.ini rendering above?

I guess we could reuse the already configured `LOCAL_ROOT_URL` from app.ini rendering above?

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:04:31 +00:00

fixed!

templates/gitea/deployment.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:54:07 +00:00

This block shouldn't be necessary.

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:04:48 +00:00

removed!

templates/gitea/deployment.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:54:20 +00:00

This block shouldn't be necessary.

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:05:35 +00:00

removed!

templates/gitea/deployment.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:54:29 +00:00

This block shouldn't be necessary.

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:06:20 +00:00

removed!

templates/gitea/deployment.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:54:36 +00:00

This block shouldn't be necessary.

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:05:40 +00:00

removed!

unittests/act_runner/config-act-runner.yaml

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:47:11 +00:00

We should also ensure the content of the ConfigMap is correct.

justusbunsi (Migrated from gitea.com) commented

2024-07-24 17:30:26 +00:00

added unit tests!

values.yaml

						
				@ -410,17 +409,12 @@ actions:

				      repository: docker

				      tag: 25.0.2-dind

justusbunsi (Migrated from gitea.com) commented

2024-06-30 12:45:21 +00:00

Do we really need the actions.provisioning.token object? Isn't the crafted image reference identical to what {{ include "gitea.image" . }} from _helpers.tpl would already provide?

Do we really need the `actions.provisioning.token` object? Isn't the crafted image reference identical to what `{{ include "gitea.image" . }}` from `_helpers.tpl` would already provide?

justusbunsi (Migrated from gitea.com) commented

2024-07-24 03:05:57 +00:00

removed!

vjm commented

2024-07-23 11:41:57 +00:00

(Migrated from gitea.com)

I am hoping to pick this back up this week! FYI.

👍 3 🎉 4

vjm commented

2024-07-24 21:04:30 +00:00

(Migrated from gitea.com)

@justusbunsi -- i think we're ready to merge!

Environment variable related handling in various files
Everything related to app.ini creation in _helpers.tpl
Handling of static values that should be in values.yaml
Unittests for new templates not only to check that the resources are rendered, but also verify that dynamic parts are rendered correctly (e.g. secret reference in act_runner/statefulset.yaml, placeholders for URLs, ...).
actions.provisioning.token object in values.yaml

could we merge this and talk separately about the Helm Chart Hook mechanism vs. while-sleep loop? i wasn't able to get it working quickly per my other comment.

@justusbunsi -- i think we're ready to merge! - [x] Environment variable related handling in various files - [x] Everything related to app.ini creation in _helpers.tpl - [x] Handling of static values that should be in values.yaml - [x] Unittests for new templates not only to check that the resources are rendered, but also verify that dynamic parts are rendered correctly (e.g. secret reference in act_runner/statefulset.yaml, placeholders for URLs, ...). - [x] actions.provisioning.token object in values.yaml could we merge this and talk separately about the Helm Chart Hook mechanism vs. while-sleep loop? i wasn't able to get it working quickly per my other comment.

🎉 2

CompPhy commented

2024-07-30 14:07:03 +00:00

(Migrated from gitea.com)

REDACTED.... I answered my own question... You're using DinD configuration so that runner can run build jobs for images, and other similar container based actions.

👍 1

justusbunsi commented

2024-07-31 17:16:25 +00:00

(Migrated from gitea.com)

@vjm Thanks for the follow-up. I am not able to re-review until weekend due to time constraints.

Regarding hook mechanism vs. sleep loop: I have nothing against addressing that in another PR.

By the way, anyone else having issues running the recent dind images on containerd? For me, the last working image is 23.x.x-dind. I always had to modify the values to test it locally. Any tips from those following this PR?

@vjm Thanks for the follow-up. I am not able to re-review until weekend due to time constraints. Regarding hook mechanism vs. sleep loop: I have nothing against addressing that in another PR. By the way, anyone else having issues running the recent dind images on containerd? For me, the last working image is 23.x.x-dind. I always had to modify the values to test it locally. Any tips from those following this PR?

👍 1

vjm commented

2024-07-31 20:43:21 +00:00

(Migrated from gitea.com)

I actually have been using buildx instead of the dind, but i purposely did not remove dind in case others still want that approach. see my example action above for the buildx approach!

vjm commented

2024-08-12 16:31:06 +00:00

(Migrated from gitea.com)

hi @justusbunsi , just nudging this to make sure we review before it becomes out of date!

👀 2

DominicMCN commented

2024-08-14 10:29:14 +00:00

(Migrated from gitea.com)

I've been eyeing at this PR for months. Is it moving forward or should I just copy the templates manually now.

justusbunsi commented

2024-08-14 17:53:32 +00:00

(Migrated from gitea.com)

hi @justusbunsi , just nudging this to make sure we review before it becomes out of date!

@vjm Apologies for not reacting earlier, and thanks for nudging 🙂. I'll review tomorrow as for today, I have no brain energy left after work. If it is in a functioning state I won't block a merge.

I've been eyeing at this PR for months. Is it moving forward or should I just copy the templates manually now.

@DominicMCN, that's no helpful comment. I am aware that this is highly requested. But sometimes real life exists. Keep in mind that we all do this in our spare time. Next time, maybe ask how you can help or test changes in your environment to confirm it from your perspective. The larger the feedback pool, the better.

>hi @justusbunsi , just nudging this to make sure we review before it becomes out of date! @vjm Apologies for not reacting earlier, and thanks for nudging 🙂. I'll review tomorrow as for today, I have no brain energy left after work. If it is in a functioning state I won't block a merge. > I've been eyeing at this PR for months. Is it moving forward or should I just copy the templates manually now. @DominicMCN, that's no helpful comment. I am aware that this is **highly** requested. But sometimes real life exists. Keep in mind that we all do this in our spare time. Next time, maybe ask how you can help or test changes in your environment to confirm it from your perspective. The larger the feedback pool, the better.

❤️ 2

mlhynfield commented

2024-08-15 19:02:52 +00:00

(Migrated from gitea.com)

Hi! I'm a relatively new user myself, and I already have Gitea deployed via Helm chart in my homelab RKE2 cluster, and I decided to test this branch out. I have a few notes, but I will preface this by saying I don't think any of these necessarily block a merge, but may warrant subsequent PRs.

With .Values.config.server.LOCAL_ROOT_URL not explicitly specified in my values file, I got a blank GITEA_INSTANCE_URL which prevented the StatefulSet from starting up
Assumedly since the Gitea PersistentVolume is RWO, I was unable to mount the PV to the Job created when .Values.actions.provisioning.enabled is true, so I wasn't able to test out automated provisioning
Is the volumeClaimTemplate in the StatefulSet necessary? I'd ideally like to see more granular control over the size, StorageClass, and accessModes, maybe under something like .Values.actions.persistence
Would it be possible to add a replicas option for the StatefulSet? I'm not sure exactly how the runner architecture works but theoretically having the option to host multiple runners would be nice

Overall though this works great, I was able to manually create a Secret with a manually generated token and my new runner works like a charm!

Hi! I'm a relatively new user myself, and I already have Gitea deployed via Helm chart in my homelab RKE2 cluster, and I decided to test this branch out. I have a few notes, but I will preface this by saying I don't think any of these necessarily block a merge, but may warrant subsequent PRs. - With `.Values.config.server.LOCAL_ROOT_URL` not explicitly specified in my values file, I got a blank GITEA_INSTANCE_URL which prevented the StatefulSet from starting up - Assumedly since the Gitea PersistentVolume is RWO, I was unable to mount the PV to the Job created when `.Values.actions.provisioning.enabled` is `true`, so I wasn't able to test out automated provisioning - Is the volumeClaimTemplate in the StatefulSet necessary? I'd ideally like to see more granular control over the size, StorageClass, and accessModes, maybe under something like `.Values.actions.persistence` - Would it be possible to add a replicas option for the StatefulSet? I'm not sure exactly how the runner architecture works but theoretically having the option to host multiple runners would be nice Overall though this works great, I was able to manually create a Secret with a manually generated token and my new runner works like a charm!

justusbunsi commented

2024-08-15 20:51:38 +00:00

(Migrated from gitea.com)

Hi! I'm a relatively new user myself, and I already have Gitea deployed via Helm chart in my homelab RKE2 cluster, and I decided to test this branch out. I have a few notes, but I will preface this by saying I don't think any of these necessarily block a merge, but may warrant subsequent PRs.

With .Values.config.server.LOCAL_ROOT_URL not explicitly specified in my values file, I got a blank GITEA_INSTANCE_URL which prevented the StatefulSet from starting up

Assumedly since the Gitea PersistentVolume is RWO, I was unable to mount the PV to the Job created when .Values.actions.provisioning.enabled is true, so I wasn't able to test out automated provisioning

Is the volumeClaimTemplate in the StatefulSet necessary? I'd ideally like to see more granular control over the size, StorageClass, and accessModes, maybe under something like .Values.actions.persistence

Would it be possible to add a replicas option for the StatefulSet? I'm not sure exactly how the runner architecture works but theoretically having the option to host multiple runners would be nice

Overall though this works great, I was able to manually create a Secret with a manually generated token and my new runner works like a charm!

Thanks for your feedback.

I am currently reviewing it and provide a git patch for the author to be applied. Thanks for confirming the LOCAL_ROOT_URL issue.

The volumeClaimTemplate in StatefulSet is a requirement from Kubernetes. Without such claim template, the resource would not be valid.

Regarding replicas in StatefulSet: Let's first get this over the finish line and collect some user insights how stable it is. Replica > 1 would probably mean mutliple register tokens...

> Hi! I'm a relatively new user myself, and I already have Gitea deployed via Helm chart in my homelab RKE2 cluster, and I decided to test this branch out. I have a few notes, but I will preface this by saying I don't think any of these necessarily block a merge, but may warrant subsequent PRs. > > - With `.Values.config.server.LOCAL_ROOT_URL` not explicitly specified in my values file, I got a blank GITEA_INSTANCE_URL which prevented the StatefulSet from starting up > - Assumedly since the Gitea PersistentVolume is RWO, I was unable to mount the PV to the Job created when `.Values.actions.provisioning.enabled` is `true`, so I wasn't able to test out automated provisioning > - Is the volumeClaimTemplate in the StatefulSet necessary? I'd ideally like to see more granular control over the size, StorageClass, and accessModes, maybe under something like `.Values.actions.persistence` > - Would it be possible to add a replicas option for the StatefulSet? I'm not sure exactly how the runner architecture works but theoretically having the option to host multiple runners would be nice > > Overall though this works great, I was able to manually create a Secret with a manually generated token and my new runner works like a charm! Thanks for your feedback. I am currently reviewing it and provide a git patch for the author to be applied. Thanks for confirming the `LOCAL_ROOT_URL` issue. The `volumeClaimTemplate` in `StatefulSet` is a requirement from Kubernetes. Without such claim template, the resource would not be valid. Regarding replicas in StatefulSet: Let's first get this over the finish line and collect some user insights how stable it is. Replica > 1 would probably mean mutliple register tokens...

👍 2 🎉 2

justusbunsi (Migrated from gitea.com) reviewed 2024-08-15 21:51:23 +00:00

justusbunsi (Migrated from gitea.com) left a comment

@vjm Unfortunately, the PR was not in a functioning state. Please review below diff and consider applying it.

It fixes several issues with overwriting settings, preserving default behavior, ensures configuration consistency for actions.* settings, adds many unittests.
With these changes, no Chart user should be able to apply a release with incompatible values. To everyone following this PR: If I missed something, let us know.

📢 EXPAND ME to see my proposed git diff

diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl
index b84c93b..9fa86df 100644
--- a/templates/_helpers.tpl
+++ b/templates/_helpers.tpl
@@ -220,6 +220,15 @@ https
 {{- end -}}
 {{- end -}}
 
+{{- define "gitea.act_runner.local_root_url" -}}
+{{- if not .Values.gitea.config.server.LOCAL_ROOT_URL -}}
+    {{- printf "http://%s-http:%.0f" (include "gitea.fullname" .) .Values.service.http.port -}}
+{{- else -}}
+  {{/* fallback for allowing to overwrite this value via inline config */}}
+  {{- .Values.gitea.config.server.LOCAL_ROOT_URL -}}
+{{- end -}}
+{{- end -}}
+
 {{- define "gitea.inline_configuration" -}}
   {{- include "gitea.inline_configuration.init" . -}}
   {{- include "gitea.inline_configuration.defaults" . -}}
@@ -334,7 +343,7 @@ https
      {{- $_ := set .Values.gitea.config.indexer "ISSUE_INDEXER_TYPE" "db" -}}
   {{- end -}}
   {{- if not .Values.gitea.config.actions.ENABLED -}}
-     {{- $_ := set .Values.gitea.config.actions "ENABLED" "false" -}}
+     {{- $_ := set .Values.gitea.config.actions "ENABLED" (ternary "true" "false" .Values.actions.enabled) -}}
   {{- end -}}
 {{- end -}}
 
@@ -355,8 +364,8 @@ https
   {{- if not .Values.gitea.config.server.ROOT_URL -}}
     {{- $_ := set .Values.gitea.config.server "ROOT_URL" (printf "%s://%s" (include "gitea.public_protocol" .) .Values.gitea.config.server.DOMAIN) -}}
   {{- end -}}
-  {{- if not .Values.gitea.config.server.LOCAL_ROOT_URL -}}
-     {{- $_ := set .Values.gitea.config.server "LOCAL_ROOT_URL" (printf "http://%s-http:%.0f" (include "gitea.fullname" .) .Values.service.http.port) -}}
+  {{- if .Values.actions.enabled -}}
+     {{- $_ := set .Values.gitea.config.server "LOCAL_ROOT_URL" (include "gitea.act_runner.local_root_url" .) -}}
   {{- end -}}
   {{- if not .Values.gitea.config.server.SSH_DOMAIN -}}
     {{- $_ := set .Values.gitea.config.server "SSH_DOMAIN" .Values.gitea.config.server.DOMAIN -}}
diff --git a/templates/gitea/act_runner/01-consistency-checks.yaml b/templates/gitea/act_runner/01-consistency-checks.yaml
new file mode 100644
index 0000000..25ae556
--- /dev/null
+++ b/templates/gitea/act_runner/01-consistency-checks.yaml
@@ -0,0 +1,15 @@
+{{- if .Values.actions.enabled -}}
+    {{- if .Values.actions.provisioning.enabled -}}
+        {{- if not (and .Values.persistence.enabled .Values.persistence.mount) -}}
+            {{- fail "persistence.enabled and persistence.mount are required when provisioning is enabled" -}}
+        {{- end -}}
+        {{- if and .Values.persistence.enabled .Values.persistence.mount -}}
+            {{- if .Values.actions.existingSecret -}}
+                {{- fail "Can't specify both actions.provisioning.enabled and actions.existingSecret" -}}
+            {{- end -}}
+        {{- end -}}
+    {{- end -}}
+    {{- if and (not .Values.actions.provisioning.enabled) (or (empty .Values.actions.existingSecret) (empty .Values.actions.existingSecretKey)) -}}
+        {{- fail "actions.existingSecret and actions.existingSecretKey are required when provisioning is disabled" -}}
+    {{- end -}}
+{{- end -}}
diff --git a/templates/gitea/act_runner/config-scripts.yaml b/templates/gitea/act_runner/config-scripts.yaml
index 778b1c9..688bd20 100644
--- a/templates/gitea/act_runner/config-scripts.yaml
+++ b/templates/gitea/act_runner/config-scripts.yaml
@@ -1,3 +1,4 @@
+{{- if .Values.actions.enabled }}
 {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }}
 ---
 apiVersion: v1
@@ -9,3 +10,4 @@ metadata:
 data:
 {{ (.Files.Glob "scripts/*.sh").AsConfig | indent 2 }}
 {{- end }}
+{{- end }}
diff --git a/templates/gitea/act_runner/job.yaml b/templates/gitea/act_runner/job.yaml
index 795809f..032671f 100644
--- a/templates/gitea/act_runner/job.yaml
+++ b/templates/gitea/act_runner/job.yaml
@@ -1,7 +1,5 @@
+{{- if .Values.actions.enabled }}
 {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }}
-{{- if .Values.actions.existingSecret }}
-{{- fail "Can't specify both actions.provisioning.enabled and actions.existingSecret" }}
-{{- end }}
 {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }}
 {{- $secretName := include "gitea.workername" (dict "global" . "worker" "actions-token") }}
 ---
@@ -113,3 +111,4 @@ spec:
   completions: 1
   backoffLimit: 1
 {{- end }}
+{{- end }}
diff --git a/templates/gitea/act_runner/role-job.yaml b/templates/gitea/act_runner/role-job.yaml
index 3b21cea..b06c18d 100644
--- a/templates/gitea/act_runner/role-job.yaml
+++ b/templates/gitea/act_runner/role-job.yaml
@@ -1,3 +1,4 @@
+{{- if .Values.actions.enabled }}
 {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }}
 {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }}
 {{- $secretName := include "gitea.workername" (dict "global" . "worker" "actions-token") }}
@@ -21,3 +22,4 @@ rules:
       - update
       - patch
 {{- end }}
+{{- end }}
diff --git a/templates/gitea/act_runner/rolebinding-job.yaml b/templates/gitea/act_runner/rolebinding-job.yaml
index 74f68e6..c80bd3e 100644
--- a/templates/gitea/act_runner/rolebinding-job.yaml
+++ b/templates/gitea/act_runner/rolebinding-job.yaml
@@ -1,3 +1,4 @@
+{{- if .Values.actions.enabled }}
 {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }}
 {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }}
 {{- $secretName := include "gitea.workername" (dict "global" . "worker" "actions-token") }}
@@ -18,3 +19,4 @@ subjects:
     name: {{ $name }}
     namespace: {{ .Release.Namespace }}
 {{- end }}
+{{- end }}
diff --git a/templates/gitea/act_runner/secret-token.yaml b/templates/gitea/act_runner/secret-token.yaml
index 5b57f3a..e6ee325 100644
--- a/templates/gitea/act_runner/secret-token.yaml
+++ b/templates/gitea/act_runner/secret-token.yaml
@@ -1,3 +1,4 @@
+{{- if .Values.actions.enabled }}
 {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }}
 {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }}
 {{- $secretName := include "gitea.workername" (dict "global" . "worker" "actions-token") }}
@@ -15,3 +16,4 @@ data:
   token: {{ (b64dec (index $secret.data "token")) | b64enc }}
 {{ end -}}
 {{- end }}
+{{- end }}
diff --git a/templates/gitea/act_runner/serviceaccount-job.yaml b/templates/gitea/act_runner/serviceaccount-job.yaml
index 4e1738c..e2c0fb4 100644
--- a/templates/gitea/act_runner/serviceaccount-job.yaml
+++ b/templates/gitea/act_runner/serviceaccount-job.yaml
@@ -1,3 +1,4 @@
+{{- if .Values.actions.enabled }}
 {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }}
 {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }}
 ---
@@ -9,3 +10,4 @@ metadata:
     {{- include "gitea.labels" . | nindent 4 }}
     app.kubernetes.io/component: token-job
 {{- end }}
+{{- end }}
diff --git a/templates/gitea/act_runner/statefulset.yaml b/templates/gitea/act_runner/statefulset.yaml
index c4d2375..7d5d096 100644
--- a/templates/gitea/act_runner/statefulset.yaml
+++ b/templates/gitea/act_runner/statefulset.yaml
@@ -54,7 +54,7 @@ spec:
                   name: "{{ .Values.actions.existingSecret | default $secretName }}"
                   key: "{{ .Values.actions.existingSecretKey | default "token" }}"
             - name: GITEA_INSTANCE_URL
-              value: {{ .Values.gitea.config.server.LOCAL_ROOT_URL | quote }}
+              value: {{ include "gitea.act_runner.local_root_url" . }}
             - name: CONFIG_FILE
               value: /actrunner/config.yaml
           resources:
diff --git a/unittests/act_runner/01-consistency-checks.yaml b/unittests/act_runner/01-consistency-checks.yaml
new file mode 100644
index 0000000..1c30924
--- /dev/null
+++ b/unittests/act_runner/01-consistency-checks.yaml
@@ -0,0 +1,69 @@
+suite: actions template | consistency checks
+release:
+  name: gitea-unittests
+  namespace: testing
+templates:
+  - templates/gitea/act_runner/01-consistency-checks.yaml
+tests:
+  - it: fails when provisioning is enabled BUT persistence is completely disabled
+    set:
+      persistence:
+        enabled: false
+      actions:
+        enabled: true
+        provisioning:
+          enabled: true
+    asserts:
+      - failedTemplate:
+          errorMessage: "persistence.enabled and persistence.mount are required when provisioning is enabled"
+  - it: fails when provisioning is enabled BUT mount is disabled, although persistence is enabled
+    set:
+      persistence:
+        enabled: true
+        mount: false
+      actions:
+        enabled: true
+        provisioning:
+          enabled: true
+    asserts:
+      - failedTemplate:
+          errorMessage: "persistence.enabled and persistence.mount are required when provisioning is enabled"
+  - it: fails when provisioning is enabled AND existingSecret is given
+    set:
+      actions:
+        enabled: true
+        provisioning:
+          enabled: true
+        existingSecret: "secret-reference"
+    asserts:
+      - failedTemplate:
+          errorMessage: "Can't specify both actions.provisioning.enabled and actions.existingSecret"
+  - it: fails when provisioning is disabled BUT existingSecret and existingSecretKey are missing
+    set:
+      actions:
+        enabled: true
+        provisioning:
+          enabled: false
+    asserts:
+      - failedTemplate:
+          errorMessage: "actions.existingSecret and actions.existingSecretKey are required when provisioning is disabled"
+  - it: fails when provisioning is disabled BUT existingSecretKey is missing
+    set:
+      actions:
+        enabled: true
+        provisioning:
+          enabled: false
+        existingSecret: "my-secret"
+    asserts:
+      - failedTemplate:
+          errorMessage: "actions.existingSecret and actions.existingSecretKey are required when provisioning is disabled"
+  - it: fails when provisioning is disabled BUT existingSecret is missing
+    set:
+      actions:
+        enabled: true
+        provisioning:
+          enabled: false
+        existingSecretKey: "my-secret-key"
+    asserts:
+      - failedTemplate:
+          errorMessage: "actions.existingSecret and actions.existingSecretKey are required when provisioning is disabled"
diff --git a/unittests/act_runner/config-act-runner.yaml b/unittests/act_runner/config-act-runner.yaml
index 3a78787..2cba6bc 100644
--- a/unittests/act_runner/config-act-runner.yaml
+++ b/unittests/act_runner/config-act-runner.yaml
@@ -6,6 +6,11 @@ release:
 templates:
   - templates/gitea/act_runner/config-act-runner.yaml
 tests:
+  - it: doesn't renders a ConfigMap by default
+    template: templates/gitea/act_runner/config-act-runner.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
   - it: renders a ConfigMap
     template: templates/gitea/act_runner/config-act-runner.yaml
     set:
diff --git a/unittests/act_runner/config-scripts.yaml b/unittests/act_runner/config-scripts.yaml
index c8754ea..da6d9aa 100644
--- a/unittests/act_runner/config-scripts.yaml
+++ b/unittests/act_runner/config-scripts.yaml
@@ -5,10 +5,11 @@ release:
 templates:
   - templates/gitea/act_runner/config-scripts.yaml
 tests:
-  - it: renders a ConfigMap
+  - it: renders a ConfigMap when all criteria are met
     template: templates/gitea/act_runner/config-scripts.yaml
     set:
       actions:
+        enabled: true
         provisioning:
           enabled: true
       persistence:
@@ -23,3 +24,26 @@ tests:
           name: gitea-unittests-scripts
       - isNotNullOrEmpty:
           path: data["token.sh"]
+  - it: doesn't renders a ConfigMap by default
+    template: templates/gitea/act_runner/config-scripts.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: doesn't renders a ConfigMap with disabled actions but enabled provisioning
+    template: templates/gitea/act_runner/config-scripts.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: doesn't renders a ConfigMap with disabled actions but otherwise met criteria
+    template: templates/gitea/act_runner/config-scripts.yaml
+    set:
+      actions:
+        enabled: false
+        provisioning:
+          enabled: true
+      persistence:
+        enabled: true
+        mount: true
+    asserts:
+      - hasDocuments:
+          count: 0
diff --git a/unittests/act_runner/job.yaml b/unittests/act_runner/job.yaml
index c6fe62e..c1d32e2 100644
--- a/unittests/act_runner/job.yaml
+++ b/unittests/act_runner/job.yaml
@@ -3,7 +3,7 @@ release:
   name: gitea-unittests
   namespace: testing
 chart:
-  # Override appVersion to be consistent with used digest :)
+  # Override appVersion to have a pinned version for comparison
   appVersion: 1.19.3
 templates:
   - templates/gitea/act_runner/job.yaml
@@ -12,6 +12,7 @@ tests:
     template: templates/gitea/act_runner/job.yaml
     set:
       actions:
+        enabled: true
         provisioning:
           enabled: true
       persistence:
@@ -32,6 +33,7 @@ tests:
     set:
       image.tag: "1.19.4"
       actions:
+        enabled: true
         provisioning:
           enabled: true
           publish:
@@ -46,3 +48,18 @@ tests:
       - equal:
           path: spec.template.spec.containers[1].image
           value: "bitnami/kubectl:1.29.0"
+  - it: doesn't renders a Job by default
+    template: templates/gitea/act_runner/job.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: doesn't renders a Job when provisioning is enabled BUT actions are not enabled
+    template: templates/gitea/act_runner/job.yaml
+    set:
+      actions:
+        enabled: false
+        provisioning:
+          enabled: true
+    asserts:
+      - hasDocuments:
+          count: 0
diff --git a/unittests/act_runner/role-job.yaml b/unittests/act_runner/role-job.yaml
index 69bf54c..8c511d8 100644
--- a/unittests/act_runner/role-job.yaml
+++ b/unittests/act_runner/role-job.yaml
@@ -5,10 +5,16 @@ release:
 templates:
   - templates/gitea/act_runner/role-job.yaml
 tests:
+  - it: doesn't renders a Role by default
+    template: templates/gitea/act_runner/role-job.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
   - it: renders a Role
     template: templates/gitea/act_runner/role-job.yaml
     set:
       actions:
+        enabled: true
         provisioning:
           enabled: true
       persistence:
@@ -21,3 +27,16 @@ tests:
           kind: Role
           apiVersion: rbac.authorization.k8s.io/v1
           name: gitea-unittests-actions-token-job
+  - it: doesn't renders a Role when criteria met BUT actions are not enabled
+    template: templates/gitea/act_runner/role-job.yaml
+    set:
+      actions:
+        enabled: false
+        provisioning:
+          enabled: true
+      persistence:
+        enabled: true
+        mount: true
+    asserts:
+      - hasDocuments:
+          count: 0
diff --git a/unittests/act_runner/rolebinding-job.yaml b/unittests/act_runner/rolebinding-job.yaml
index 6c08aa5..2073bfc 100644
--- a/unittests/act_runner/rolebinding-job.yaml
+++ b/unittests/act_runner/rolebinding-job.yaml
@@ -5,10 +5,16 @@ release:
 templates:
   - templates/gitea/act_runner/rolebinding-job.yaml
 tests:
+  - it: doesn't renders a RoleBinding by default
+    template: templates/gitea/act_runner/rolebinding-job.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
   - it: renders a RoleBinding
     template: templates/gitea/act_runner/rolebinding-job.yaml
     set:
       actions:
+        enabled: true
         provisioning:
           enabled: true
       persistence:
@@ -21,3 +27,16 @@ tests:
           kind: RoleBinding
           apiVersion: rbac.authorization.k8s.io/v1
           name: gitea-unittests-actions-token-job
+  - it: doesn't renders a RoleBinding when criteria met BUT actions are not enabled
+    template: templates/gitea/act_runner/rolebinding-job.yaml
+    set:
+      actions:
+        enabled: false
+        provisioning:
+          enabled: true
+      persistence:
+        enabled: true
+        mount: true
+    asserts:
+      - hasDocuments:
+          count: 0
diff --git a/unittests/act_runner/secret-token.yaml b/unittests/act_runner/secret-token.yaml
index bca06a5..b5054f3 100644
--- a/unittests/act_runner/secret-token.yaml
+++ b/unittests/act_runner/secret-token.yaml
@@ -5,10 +5,16 @@ release:
 templates:
   - templates/gitea/act_runner/secret-token.yaml
 tests:
+  - it: doesn't renders a Secret by default
+    template: templates/gitea/act_runner/secret-token.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
   - it: renders a Secret
     template: templates/gitea/act_runner/secret-token.yaml
     set:
       actions:
+        enabled: true
         provisioning:
           enabled: true
       persistence:
@@ -21,3 +27,16 @@ tests:
           kind: Secret
           apiVersion: v1
           name: gitea-unittests-actions-token
+  - it: doesn't renders a Secret when criteria met BUT actions are not enabled
+    template: templates/gitea/act_runner/secret-token.yaml
+    set:
+      actions:
+        enabled: false
+        provisioning:
+          enabled: true
+      persistence:
+        enabled: true
+        mount: true
+    asserts:
+      - hasDocuments:
+          count: 0
diff --git a/unittests/act_runner/serviceaccount-job.yaml b/unittests/act_runner/serviceaccount-job.yaml
index 6e759b9..bf8f0c8 100644
--- a/unittests/act_runner/serviceaccount-job.yaml
+++ b/unittests/act_runner/serviceaccount-job.yaml
@@ -5,10 +5,16 @@ release:
 templates:
   - templates/gitea/act_runner/serviceaccount-job.yaml
 tests:
+  - it: doesn't renders a ServiceAccount by default
+    template: templates/gitea/act_runner/serviceaccount-job.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
   - it: renders a ServiceAccount
     template: templates/gitea/act_runner/serviceaccount-job.yaml
     set:
       actions:
+        enabled: true
         provisioning:
           enabled: true
       persistence:
@@ -21,3 +27,16 @@ tests:
           kind: ServiceAccount
           apiVersion: v1
           name: gitea-unittests-actions-token-job
+  - it: doesn't renders a ServiceAccount when criteria met BUT actions are not enabled
+    template: templates/gitea/act_runner/serviceaccount-job.yaml
+    set:
+      actions:
+        enabled: false
+        provisioning:
+          enabled: true
+      persistence:
+        enabled: true
+        mount: true
+    asserts:
+      - hasDocuments:
+          count: 0
diff --git a/unittests/act_runner/statefulset.yaml b/unittests/act_runner/statefulset.yaml
index 017ddc5..cc10157 100644
--- a/unittests/act_runner/statefulset.yaml
+++ b/unittests/act_runner/statefulset.yaml
@@ -5,11 +5,40 @@ release:
 templates:
   - templates/gitea/act_runner/statefulset.yaml
 tests:
-  - it: renders a StatefulSet
+  - it: doesn't renders a StatefulSet by default
+    template: templates/gitea/act_runner/statefulset.yaml
+    asserts:
+      - hasDocuments:
+          count: 0
+  - it: renders a StatefulSet (with given existingSecret/existingSecretKey)
     template: templates/gitea/act_runner/statefulset.yaml
     set:
       actions:
         enabled: true
+        existingSecret: "my-secret"
+        existingSecretKey: "my-secret-key"
+    asserts:
+      - hasDocuments:
+          count: 1
+      - containsDocument:
+          kind: StatefulSet
+          apiVersion: apps/v1
+          name: gitea-unittests-act-runner
+      - equal:
+          path: spec.template.spec.containers[0].env[3]
+          value:
+            name: GITEA_RUNNER_REGISTRATION_TOKEN
+            valueFrom:
+              secretKeyRef:
+                name: "my-secret"
+                key: "my-secret-key"
+  - it: renders a StatefulSet (with secret reference defaults for enabled provisioning)
+    template: templates/gitea/act_runner/statefulset.yaml
+    set:
+      actions:
+        enabled: true
+        provisioning:
+          enabled: true
     asserts:
       - hasDocuments:
           count: 1
@@ -25,3 +54,42 @@ tests:
               secretKeyRef:
                 name: "gitea-unittests-actions-token"
                 key: "token"
+  - it: renders a StatefulSet (with correct GITEA_INSTANCE_URL env with default act-runner specific LOCAL_ROOT_URL)
+    template: templates/gitea/act_runner/statefulset.yaml
+    set:
+      actions:
+        enabled: true
+        existingSecret: "my-secret"
+        existingSecretKey: "my-secret-key"
+    asserts:
+      - hasDocuments:
+          count: 1
+      - containsDocument:
+          kind: StatefulSet
+          apiVersion: apps/v1
+          name: gitea-unittests-act-runner
+      - equal:
+          path: spec.template.spec.containers[0].env[4]
+          value:
+            name: GITEA_INSTANCE_URL
+            value: "http://gitea-unittests-http:3000"
+  - it: renders a StatefulSet (with correct GITEA_INSTANCE_URL env from customized LOCAL_ROOT_URL)
+    template: templates/gitea/act_runner/statefulset.yaml
+    set:
+      gitea.config.server.LOCAL_ROOT_URL: "http://git.example.com"
+      actions:
+        enabled: true
+        existingSecret: "my-secret"
+        existingSecretKey: "my-secret-key"
+    asserts:
+      - hasDocuments:
+          count: 1
+      - containsDocument:
+          kind: StatefulSet
+          apiVersion: apps/v1
+          name: gitea-unittests-act-runner
+      - equal:
+          path: spec.template.spec.containers[0].env[4]
+          value:
+            name: GITEA_INSTANCE_URL
+            value: "http://git.example.com"
diff --git a/unittests/config/actions-config.yaml b/unittests/config/actions-config.yaml
new file mode 100644
index 0000000..ada9694
--- /dev/null
+++ b/unittests/config/actions-config.yaml
@@ -0,0 +1,61 @@
+suite: config template | actions config
+release:
+  name: gitea-unittests
+  namespace: testing
+templates:
+  - templates/gitea/config.yaml
+tests:
+  - it: "actions are not enabled by default"
+    template: templates/gitea/config.yaml
+    asserts:
+      - documentIndex: 0
+        equal:
+          path: stringData.actions
+          value: |-
+            ENABLED=false
+
+  - it: "actions can be enabled via inline config"
+    template: templates/gitea/config.yaml
+    set:
+      gitea.config.actions.ENABLED: true
+    asserts:
+      - documentIndex: 0
+        equal:
+          path: stringData.actions
+          value: |-
+            ENABLED=true
+
+  - it: "actions can be enabled via dedicated values object"
+    template: templates/gitea/config.yaml
+    set:
+      actions:
+        enabled: true
+    asserts:
+      - documentIndex: 0
+        equal:
+          path: stringData.actions
+          value: |-
+            ENABLED=true
+
+  - it: "defines LOCAL_ROOT_URL when actions are enabled"
+    template: templates/gitea/config.yaml
+    set:
+      actions:
+        enabled: true
+    asserts:
+      - documentIndex: 0
+        matchRegex:
+          path: stringData.server
+          pattern: \nLOCAL_ROOT_URL=http://gitea-unittests-http:3000
+
+  - it: "respects custom LOCAL_ROOT_URL, even when actions are enabled"
+    template: templates/gitea/config.yaml
+    set:
+      actions:
+        enabled: true
+      gitea.config.server.LOCAL_ROOT_URL: "http://git.example.com"
+    asserts:
+      - documentIndex: 0
+        matchRegex:
+          path: stringData.server
+          pattern: \nLOCAL_ROOT_URL=http://git.example.com

@vjm Unfortunately, the PR was not in a functioning state. Please review below diff and consider applying it. It fixes several issues with overwriting settings, preserving default behavior, ensures configuration consistency for `actions.*` settings, adds many unittests. With these changes, no Chart user should be able to apply a release with incompatible values. To everyone following this PR: If I missed something, let us know. <details> <summary>📢 EXPAND ME to see my proposed git diff</summary> ```diff diff --git a/templates/_helpers.tpl b/templates/_helpers.tpl index b84c93b..9fa86df 100644 --- a/templates/_helpers.tpl +++ b/templates/_helpers.tpl @@ -220,6 +220,15 @@ https {{- end -}} {{- end -}} +{{- define "gitea.act_runner.local_root_url" -}} +{{- if not .Values.gitea.config.server.LOCAL_ROOT_URL -}} + {{- printf "http://%s-http:%.0f" (include "gitea.fullname" .) .Values.service.http.port -}} +{{- else -}} + {{/* fallback for allowing to overwrite this value via inline config */}} + {{- .Values.gitea.config.server.LOCAL_ROOT_URL -}} +{{- end -}} +{{- end -}} + {{- define "gitea.inline_configuration" -}} {{- include "gitea.inline_configuration.init" . -}} {{- include "gitea.inline_configuration.defaults" . -}} @@ -334,7 +343,7 @@ https {{- $_ := set .Values.gitea.config.indexer "ISSUE_INDEXER_TYPE" "db" -}} {{- end -}} {{- if not .Values.gitea.config.actions.ENABLED -}} - {{- $_ := set .Values.gitea.config.actions "ENABLED" "false" -}} + {{- $_ := set .Values.gitea.config.actions "ENABLED" (ternary "true" "false" .Values.actions.enabled) -}} {{- end -}} {{- end -}} @@ -355,8 +364,8 @@ https {{- if not .Values.gitea.config.server.ROOT_URL -}} {{- $_ := set .Values.gitea.config.server "ROOT_URL" (printf "%s://%s" (include "gitea.public_protocol" .) .Values.gitea.config.server.DOMAIN) -}} {{- end -}} - {{- if not .Values.gitea.config.server.LOCAL_ROOT_URL -}} - {{- $_ := set .Values.gitea.config.server "LOCAL_ROOT_URL" (printf "http://%s-http:%.0f" (include "gitea.fullname" .) .Values.service.http.port) -}} + {{- if .Values.actions.enabled -}} + {{- $_ := set .Values.gitea.config.server "LOCAL_ROOT_URL" (include "gitea.act_runner.local_root_url" .) -}} {{- end -}} {{- if not .Values.gitea.config.server.SSH_DOMAIN -}} {{- $_ := set .Values.gitea.config.server "SSH_DOMAIN" .Values.gitea.config.server.DOMAIN -}} diff --git a/templates/gitea/act_runner/01-consistency-checks.yaml b/templates/gitea/act_runner/01-consistency-checks.yaml new file mode 100644 index 0000000..25ae556 --- /dev/null +++ b/templates/gitea/act_runner/01-consistency-checks.yaml @@ -0,0 +1,15 @@ +{{- if .Values.actions.enabled -}} + {{- if .Values.actions.provisioning.enabled -}} + {{- if not (and .Values.persistence.enabled .Values.persistence.mount) -}} + {{- fail "persistence.enabled and persistence.mount are required when provisioning is enabled" -}} + {{- end -}} + {{- if and .Values.persistence.enabled .Values.persistence.mount -}} + {{- if .Values.actions.existingSecret -}} + {{- fail "Can't specify both actions.provisioning.enabled and actions.existingSecret" -}} + {{- end -}} + {{- end -}} + {{- end -}} + {{- if and (not .Values.actions.provisioning.enabled) (or (empty .Values.actions.existingSecret) (empty .Values.actions.existingSecretKey)) -}} + {{- fail "actions.existingSecret and actions.existingSecretKey are required when provisioning is disabled" -}} + {{- end -}} +{{- end -}} diff --git a/templates/gitea/act_runner/config-scripts.yaml b/templates/gitea/act_runner/config-scripts.yaml index 778b1c9..688bd20 100644 --- a/templates/gitea/act_runner/config-scripts.yaml +++ b/templates/gitea/act_runner/config-scripts.yaml @@ -1,3 +1,4 @@ +{{- if .Values.actions.enabled }} {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }} --- apiVersion: v1 @@ -9,3 +10,4 @@ metadata: data: {{ (.Files.Glob "scripts/*.sh").AsConfig | indent 2 }} {{- end }} +{{- end }} diff --git a/templates/gitea/act_runner/job.yaml b/templates/gitea/act_runner/job.yaml index 795809f..032671f 100644 --- a/templates/gitea/act_runner/job.yaml +++ b/templates/gitea/act_runner/job.yaml @@ -1,7 +1,5 @@ +{{- if .Values.actions.enabled }} {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }} -{{- if .Values.actions.existingSecret }} -{{- fail "Can't specify both actions.provisioning.enabled and actions.existingSecret" }} -{{- end }} {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }} {{- $secretName := include "gitea.workername" (dict "global" . "worker" "actions-token") }} --- @@ -113,3 +111,4 @@ spec: completions: 1 backoffLimit: 1 {{- end }} +{{- end }} diff --git a/templates/gitea/act_runner/role-job.yaml b/templates/gitea/act_runner/role-job.yaml index 3b21cea..b06c18d 100644 --- a/templates/gitea/act_runner/role-job.yaml +++ b/templates/gitea/act_runner/role-job.yaml @@ -1,3 +1,4 @@ +{{- if .Values.actions.enabled }} {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }} {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }} {{- $secretName := include "gitea.workername" (dict "global" . "worker" "actions-token") }} @@ -21,3 +22,4 @@ rules: - update - patch {{- end }} +{{- end }} diff --git a/templates/gitea/act_runner/rolebinding-job.yaml b/templates/gitea/act_runner/rolebinding-job.yaml index 74f68e6..c80bd3e 100644 --- a/templates/gitea/act_runner/rolebinding-job.yaml +++ b/templates/gitea/act_runner/rolebinding-job.yaml @@ -1,3 +1,4 @@ +{{- if .Values.actions.enabled }} {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }} {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }} {{- $secretName := include "gitea.workername" (dict "global" . "worker" "actions-token") }} @@ -18,3 +19,4 @@ subjects: name: {{ $name }} namespace: {{ .Release.Namespace }} {{- end }} +{{- end }} diff --git a/templates/gitea/act_runner/secret-token.yaml b/templates/gitea/act_runner/secret-token.yaml index 5b57f3a..e6ee325 100644 --- a/templates/gitea/act_runner/secret-token.yaml +++ b/templates/gitea/act_runner/secret-token.yaml @@ -1,3 +1,4 @@ +{{- if .Values.actions.enabled }} {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }} {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }} {{- $secretName := include "gitea.workername" (dict "global" . "worker" "actions-token") }} @@ -15,3 +16,4 @@ data: token: {{ (b64dec (index $secret.data "token")) | b64enc }} {{ end -}} {{- end }} +{{- end }} diff --git a/templates/gitea/act_runner/serviceaccount-job.yaml b/templates/gitea/act_runner/serviceaccount-job.yaml index 4e1738c..e2c0fb4 100644 --- a/templates/gitea/act_runner/serviceaccount-job.yaml +++ b/templates/gitea/act_runner/serviceaccount-job.yaml @@ -1,3 +1,4 @@ +{{- if .Values.actions.enabled }} {{- if and (and .Values.actions.provisioning.enabled .Values.persistence.enabled) .Values.persistence.mount }} {{- $name := include "gitea.workername" (dict "global" . "worker" "actions-token-job") }} --- @@ -9,3 +10,4 @@ metadata: {{- include "gitea.labels" . | nindent 4 }} app.kubernetes.io/component: token-job {{- end }} +{{- end }} diff --git a/templates/gitea/act_runner/statefulset.yaml b/templates/gitea/act_runner/statefulset.yaml index c4d2375..7d5d096 100644 --- a/templates/gitea/act_runner/statefulset.yaml +++ b/templates/gitea/act_runner/statefulset.yaml @@ -54,7 +54,7 @@ spec: name: "{{ .Values.actions.existingSecret | default $secretName }}" key: "{{ .Values.actions.existingSecretKey | default "token" }}" - name: GITEA_INSTANCE_URL - value: {{ .Values.gitea.config.server.LOCAL_ROOT_URL | quote }} + value: {{ include "gitea.act_runner.local_root_url" . }} - name: CONFIG_FILE value: /actrunner/config.yaml resources: diff --git a/unittests/act_runner/01-consistency-checks.yaml b/unittests/act_runner/01-consistency-checks.yaml new file mode 100644 index 0000000..1c30924 --- /dev/null +++ b/unittests/act_runner/01-consistency-checks.yaml @@ -0,0 +1,69 @@ +suite: actions template | consistency checks +release: + name: gitea-unittests + namespace: testing +templates: + - templates/gitea/act_runner/01-consistency-checks.yaml +tests: + - it: fails when provisioning is enabled BUT persistence is completely disabled + set: + persistence: + enabled: false + actions: + enabled: true + provisioning: + enabled: true + asserts: + - failedTemplate: + errorMessage: "persistence.enabled and persistence.mount are required when provisioning is enabled" + - it: fails when provisioning is enabled BUT mount is disabled, although persistence is enabled + set: + persistence: + enabled: true + mount: false + actions: + enabled: true + provisioning: + enabled: true + asserts: + - failedTemplate: + errorMessage: "persistence.enabled and persistence.mount are required when provisioning is enabled" + - it: fails when provisioning is enabled AND existingSecret is given + set: + actions: + enabled: true + provisioning: + enabled: true + existingSecret: "secret-reference" + asserts: + - failedTemplate: + errorMessage: "Can't specify both actions.provisioning.enabled and actions.existingSecret" + - it: fails when provisioning is disabled BUT existingSecret and existingSecretKey are missing + set: + actions: + enabled: true + provisioning: + enabled: false + asserts: + - failedTemplate: + errorMessage: "actions.existingSecret and actions.existingSecretKey are required when provisioning is disabled" + - it: fails when provisioning is disabled BUT existingSecretKey is missing + set: + actions: + enabled: true + provisioning: + enabled: false + existingSecret: "my-secret" + asserts: + - failedTemplate: + errorMessage: "actions.existingSecret and actions.existingSecretKey are required when provisioning is disabled" + - it: fails when provisioning is disabled BUT existingSecret is missing + set: + actions: + enabled: true + provisioning: + enabled: false + existingSecretKey: "my-secret-key" + asserts: + - failedTemplate: + errorMessage: "actions.existingSecret and actions.existingSecretKey are required when provisioning is disabled" diff --git a/unittests/act_runner/config-act-runner.yaml b/unittests/act_runner/config-act-runner.yaml index 3a78787..2cba6bc 100644 --- a/unittests/act_runner/config-act-runner.yaml +++ b/unittests/act_runner/config-act-runner.yaml @@ -6,6 +6,11 @@ release: templates: - templates/gitea/act_runner/config-act-runner.yaml tests: + - it: doesn't renders a ConfigMap by default + template: templates/gitea/act_runner/config-act-runner.yaml + asserts: + - hasDocuments: + count: 0 - it: renders a ConfigMap template: templates/gitea/act_runner/config-act-runner.yaml set: diff --git a/unittests/act_runner/config-scripts.yaml b/unittests/act_runner/config-scripts.yaml index c8754ea..da6d9aa 100644 --- a/unittests/act_runner/config-scripts.yaml +++ b/unittests/act_runner/config-scripts.yaml @@ -5,10 +5,11 @@ release: templates: - templates/gitea/act_runner/config-scripts.yaml tests: - - it: renders a ConfigMap + - it: renders a ConfigMap when all criteria are met template: templates/gitea/act_runner/config-scripts.yaml set: actions: + enabled: true provisioning: enabled: true persistence: @@ -23,3 +24,26 @@ tests: name: gitea-unittests-scripts - isNotNullOrEmpty: path: data["token.sh"] + - it: doesn't renders a ConfigMap by default + template: templates/gitea/act_runner/config-scripts.yaml + asserts: + - hasDocuments: + count: 0 + - it: doesn't renders a ConfigMap with disabled actions but enabled provisioning + template: templates/gitea/act_runner/config-scripts.yaml + asserts: + - hasDocuments: + count: 0 + - it: doesn't renders a ConfigMap with disabled actions but otherwise met criteria + template: templates/gitea/act_runner/config-scripts.yaml + set: + actions: + enabled: false + provisioning: + enabled: true + persistence: + enabled: true + mount: true + asserts: + - hasDocuments: + count: 0 diff --git a/unittests/act_runner/job.yaml b/unittests/act_runner/job.yaml index c6fe62e..c1d32e2 100644 --- a/unittests/act_runner/job.yaml +++ b/unittests/act_runner/job.yaml @@ -3,7 +3,7 @@ release: name: gitea-unittests namespace: testing chart: - # Override appVersion to be consistent with used digest :) + # Override appVersion to have a pinned version for comparison appVersion: 1.19.3 templates: - templates/gitea/act_runner/job.yaml @@ -12,6 +12,7 @@ tests: template: templates/gitea/act_runner/job.yaml set: actions: + enabled: true provisioning: enabled: true persistence: @@ -32,6 +33,7 @@ tests: set: image.tag: "1.19.4" actions: + enabled: true provisioning: enabled: true publish: @@ -46,3 +48,18 @@ tests: - equal: path: spec.template.spec.containers[1].image value: "bitnami/kubectl:1.29.0" + - it: doesn't renders a Job by default + template: templates/gitea/act_runner/job.yaml + asserts: + - hasDocuments: + count: 0 + - it: doesn't renders a Job when provisioning is enabled BUT actions are not enabled + template: templates/gitea/act_runner/job.yaml + set: + actions: + enabled: false + provisioning: + enabled: true + asserts: + - hasDocuments: + count: 0 diff --git a/unittests/act_runner/role-job.yaml b/unittests/act_runner/role-job.yaml index 69bf54c..8c511d8 100644 --- a/unittests/act_runner/role-job.yaml +++ b/unittests/act_runner/role-job.yaml @@ -5,10 +5,16 @@ release: templates: - templates/gitea/act_runner/role-job.yaml tests: + - it: doesn't renders a Role by default + template: templates/gitea/act_runner/role-job.yaml + asserts: + - hasDocuments: + count: 0 - it: renders a Role template: templates/gitea/act_runner/role-job.yaml set: actions: + enabled: true provisioning: enabled: true persistence: @@ -21,3 +27,16 @@ tests: kind: Role apiVersion: rbac.authorization.k8s.io/v1 name: gitea-unittests-actions-token-job + - it: doesn't renders a Role when criteria met BUT actions are not enabled + template: templates/gitea/act_runner/role-job.yaml + set: + actions: + enabled: false + provisioning: + enabled: true + persistence: + enabled: true + mount: true + asserts: + - hasDocuments: + count: 0 diff --git a/unittests/act_runner/rolebinding-job.yaml b/unittests/act_runner/rolebinding-job.yaml index 6c08aa5..2073bfc 100644 --- a/unittests/act_runner/rolebinding-job.yaml +++ b/unittests/act_runner/rolebinding-job.yaml @@ -5,10 +5,16 @@ release: templates: - templates/gitea/act_runner/rolebinding-job.yaml tests: + - it: doesn't renders a RoleBinding by default + template: templates/gitea/act_runner/rolebinding-job.yaml + asserts: + - hasDocuments: + count: 0 - it: renders a RoleBinding template: templates/gitea/act_runner/rolebinding-job.yaml set: actions: + enabled: true provisioning: enabled: true persistence: @@ -21,3 +27,16 @@ tests: kind: RoleBinding apiVersion: rbac.authorization.k8s.io/v1 name: gitea-unittests-actions-token-job + - it: doesn't renders a RoleBinding when criteria met BUT actions are not enabled + template: templates/gitea/act_runner/rolebinding-job.yaml + set: + actions: + enabled: false + provisioning: + enabled: true + persistence: + enabled: true + mount: true + asserts: + - hasDocuments: + count: 0 diff --git a/unittests/act_runner/secret-token.yaml b/unittests/act_runner/secret-token.yaml index bca06a5..b5054f3 100644 --- a/unittests/act_runner/secret-token.yaml +++ b/unittests/act_runner/secret-token.yaml @@ -5,10 +5,16 @@ release: templates: - templates/gitea/act_runner/secret-token.yaml tests: + - it: doesn't renders a Secret by default + template: templates/gitea/act_runner/secret-token.yaml + asserts: + - hasDocuments: + count: 0 - it: renders a Secret template: templates/gitea/act_runner/secret-token.yaml set: actions: + enabled: true provisioning: enabled: true persistence: @@ -21,3 +27,16 @@ tests: kind: Secret apiVersion: v1 name: gitea-unittests-actions-token + - it: doesn't renders a Secret when criteria met BUT actions are not enabled + template: templates/gitea/act_runner/secret-token.yaml + set: + actions: + enabled: false + provisioning: + enabled: true + persistence: + enabled: true + mount: true + asserts: + - hasDocuments: + count: 0 diff --git a/unittests/act_runner/serviceaccount-job.yaml b/unittests/act_runner/serviceaccount-job.yaml index 6e759b9..bf8f0c8 100644 --- a/unittests/act_runner/serviceaccount-job.yaml +++ b/unittests/act_runner/serviceaccount-job.yaml @@ -5,10 +5,16 @@ release: templates: - templates/gitea/act_runner/serviceaccount-job.yaml tests: + - it: doesn't renders a ServiceAccount by default + template: templates/gitea/act_runner/serviceaccount-job.yaml + asserts: + - hasDocuments: + count: 0 - it: renders a ServiceAccount template: templates/gitea/act_runner/serviceaccount-job.yaml set: actions: + enabled: true provisioning: enabled: true persistence: @@ -21,3 +27,16 @@ tests: kind: ServiceAccount apiVersion: v1 name: gitea-unittests-actions-token-job + - it: doesn't renders a ServiceAccount when criteria met BUT actions are not enabled + template: templates/gitea/act_runner/serviceaccount-job.yaml + set: + actions: + enabled: false + provisioning: + enabled: true + persistence: + enabled: true + mount: true + asserts: + - hasDocuments: + count: 0 diff --git a/unittests/act_runner/statefulset.yaml b/unittests/act_runner/statefulset.yaml index 017ddc5..cc10157 100644 --- a/unittests/act_runner/statefulset.yaml +++ b/unittests/act_runner/statefulset.yaml @@ -5,11 +5,40 @@ release: templates: - templates/gitea/act_runner/statefulset.yaml tests: - - it: renders a StatefulSet + - it: doesn't renders a StatefulSet by default + template: templates/gitea/act_runner/statefulset.yaml + asserts: + - hasDocuments: + count: 0 + - it: renders a StatefulSet (with given existingSecret/existingSecretKey) template: templates/gitea/act_runner/statefulset.yaml set: actions: enabled: true + existingSecret: "my-secret" + existingSecretKey: "my-secret-key" + asserts: + - hasDocuments: + count: 1 + - containsDocument: + kind: StatefulSet + apiVersion: apps/v1 + name: gitea-unittests-act-runner + - equal: + path: spec.template.spec.containers[0].env[3] + value: + name: GITEA_RUNNER_REGISTRATION_TOKEN + valueFrom: + secretKeyRef: + name: "my-secret" + key: "my-secret-key" + - it: renders a StatefulSet (with secret reference defaults for enabled provisioning) + template: templates/gitea/act_runner/statefulset.yaml + set: + actions: + enabled: true + provisioning: + enabled: true asserts: - hasDocuments: count: 1 @@ -25,3 +54,42 @@ tests: secretKeyRef: name: "gitea-unittests-actions-token" key: "token" + - it: renders a StatefulSet (with correct GITEA_INSTANCE_URL env with default act-runner specific LOCAL_ROOT_URL) + template: templates/gitea/act_runner/statefulset.yaml + set: + actions: + enabled: true + existingSecret: "my-secret" + existingSecretKey: "my-secret-key" + asserts: + - hasDocuments: + count: 1 + - containsDocument: + kind: StatefulSet + apiVersion: apps/v1 + name: gitea-unittests-act-runner + - equal: + path: spec.template.spec.containers[0].env[4] + value: + name: GITEA_INSTANCE_URL + value: "http://gitea-unittests-http:3000" + - it: renders a StatefulSet (with correct GITEA_INSTANCE_URL env from customized LOCAL_ROOT_URL) + template: templates/gitea/act_runner/statefulset.yaml + set: + gitea.config.server.LOCAL_ROOT_URL: "http://git.example.com" + actions: + enabled: true + existingSecret: "my-secret" + existingSecretKey: "my-secret-key" + asserts: + - hasDocuments: + count: 1 + - containsDocument: + kind: StatefulSet + apiVersion: apps/v1 + name: gitea-unittests-act-runner + - equal: + path: spec.template.spec.containers[0].env[4] + value: + name: GITEA_INSTANCE_URL + value: "http://git.example.com" diff --git a/unittests/config/actions-config.yaml b/unittests/config/actions-config.yaml new file mode 100644 index 0000000..ada9694 --- /dev/null +++ b/unittests/config/actions-config.yaml @@ -0,0 +1,61 @@ +suite: config template | actions config +release: + name: gitea-unittests + namespace: testing +templates: + - templates/gitea/config.yaml +tests: + - it: "actions are not enabled by default" + template: templates/gitea/config.yaml + asserts: + - documentIndex: 0 + equal: + path: stringData.actions + value: |- + ENABLED=false + + - it: "actions can be enabled via inline config" + template: templates/gitea/config.yaml + set: + gitea.config.actions.ENABLED: true + asserts: + - documentIndex: 0 + equal: + path: stringData.actions + value: |- + ENABLED=true + + - it: "actions can be enabled via dedicated values object" + template: templates/gitea/config.yaml + set: + actions: + enabled: true + asserts: + - documentIndex: 0 + equal: + path: stringData.actions + value: |- + ENABLED=true + + - it: "defines LOCAL_ROOT_URL when actions are enabled" + template: templates/gitea/config.yaml + set: + actions: + enabled: true + asserts: + - documentIndex: 0 + matchRegex: + path: stringData.server + pattern: \nLOCAL_ROOT_URL=http://gitea-unittests-http:3000 + + - it: "respects custom LOCAL_ROOT_URL, even when actions are enabled" + template: templates/gitea/config.yaml + set: + actions: + enabled: true + gitea.config.server.LOCAL_ROOT_URL: "http://git.example.com" + asserts: + - documentIndex: 0 + matchRegex: + path: stringData.server + pattern: \nLOCAL_ROOT_URL=http://git.example.com ``` </details>

lreading commented

2024-08-15 21:58:09 +00:00

(Migrated from gitea.com)

Regarding replicas in StatefulSet: Let's first get this over the finish line and collect some user insights how stable it is. Replica > 1 would probably mean mutliple register tokens...

When I tested this, it worked fine by manually updating the replica count after I deployed it. I changed the PV to RWX by defining it in my values.yaml. Just as a warning to future readers, there was a significant performance impact when adding multiple runners. That performance hit has nothing to do with the helm chart. :)

> Regarding replicas in StatefulSet: Let's first get this over the finish line and collect some user insights how stable it is. Replica > 1 would probably mean mutliple register tokens... When I tested this, it worked fine by manually updating the replica count after I deployed it. I changed the PV to RWX by defining it in my values.yaml. Just as a warning to future readers, there was a significant performance impact when adding multiple runners. That performance hit has _nothing_ to do with the helm chart. :)

👍 1 🎉 1

mlhynfield commented

2024-08-16 16:54:49 +00:00

(Migrated from gitea.com)

Hi again! One more minor thing I'm noticing: When the Pod restarts, intermittently there will be several errors/restarts due to dind not yet being completely initialized. You might consider adding a check to ensure the docker daemon is reachable before the runner comes online. Offhand I'm not exactly sure what that would look like.

javiertury commented

2024-08-20 16:02:12 +00:00

(Migrated from gitea.com)

Recently I configured a gitea instance with an act runner. I looked into this PR for inspiration but I found the token management very complicated.

After fiddling with gitea for a while, I came up with a simpler alternative. Perhaps it is not as robust as the one proposed in this PR, but I will leave the idea here just in case.

      initContainers:
        - name: gitea-act-runner-init
          image: gitea/gitea:1.22.1-rootless
          # Creates temporary gitea instance, generates token and saves it to act-runner
          command:
            - bash
            - -exc
            - |
              sed '/[server]/a LOCAL_ROOT_URL = http://<gitea-domain>:<gitea-port>/' /data/gitea/conf/app.ini > /tmp/app.ini; test -e /act-runner-data/.runner || gitea actions generate-runner-token > /act-runner-data/token                                          
          env:
            - name: GITEA_APP_INI
              value: /tmp/app.ini
            - name: GITEA_CUSTOM
              value: /data/gitea
            - name: GITEA_WORK_DIR
              value: /data

          volumeMounts:
            - name: gitea-shared-storage
              mountPath: /data
              readOnly: true
            - name: gitea-act-runner-data
              mountPath: /act-runner-data

The overall idea is very similar to this PR. Before creating the act runner, we create an initContainer that spins up a temporary gitea container, generate a token and save it inside the act runner volume. The temporary gitea container is created on the fly by attaching the main gitea storage as readOnly and using a patched gitea configuration file. The LOCAL_ROOT_URL setting makes gitea cli connect to an already running instance at http://<gitea-domain>:<gitea-port>/ to obtain the token. Why is it necessary to obtain the token from an already running instance? Because if we use sqlite on a RWO filesystem, only one instance can be running at a time, and we don't want to kill the main instance.

The full act runner recipe can be found in https://gitea.com/gitea/act_runner/issues/280#issuecomment-898726.

Recently I configured a gitea instance with an act runner. I looked into this PR for inspiration but I found the token management very complicated. After fiddling with gitea for a while, I came up with a simpler alternative. Perhaps it is not as robust as the one proposed in this PR, but I will leave the idea here just in case. ```yaml initContainers: - name: gitea-act-runner-init image: gitea/gitea:1.22.1-rootless # Creates temporary gitea instance, generates token and saves it to act-runner command: - bash - -exc - | sed '/[server]/a LOCAL_ROOT_URL = http://<gitea-domain>:<gitea-port>/' /data/gitea/conf/app.ini > /tmp/app.ini; test -e /act-runner-data/.runner || gitea actions generate-runner-token > /act-runner-data/token env: - name: GITEA_APP_INI value: /tmp/app.ini - name: GITEA_CUSTOM value: /data/gitea - name: GITEA_WORK_DIR value: /data volumeMounts: - name: gitea-shared-storage mountPath: /data readOnly: true - name: gitea-act-runner-data mountPath: /act-runner-data ``` The overall idea is very similar to this PR. Before creating the act runner, we create an initContainer that spins up a temporary gitea container, generate a token and save it inside the act runner volume. The temporary gitea container is created on the fly by attaching the main gitea storage as `readOnly` and using a patched gitea configuration file. The `LOCAL_ROOT_URL` setting makes gitea cli connect to an already running instance at `http://<gitea-domain>:<gitea-port>/` to obtain the token. Why is it necessary to obtain the token from an already running instance? Because if we use sqlite on a RWO filesystem, only one instance can be running at a time, and we don't want to kill the main instance. The full act runner recipe can be found in https://gitea.com/gitea/act_runner/issues/280#issuecomment-898726.

vjm commented

2024-11-04 22:00:41 +00:00

(Migrated from gitea.com)

I just applied this patch, merged the latest from main -- it looks like this is a good patch @justusbunsi !

I will continue to test in day to day usage in my cluster, but this looks good to merge to me.

I just applied this patch, merged the latest from `main` -- it looks like this is a good patch @justusbunsi ! I will continue to test in day to day usage in my cluster, but this looks good to merge to me.

justusbunsi commented

2024-11-05 06:06:39 +00:00

(Migrated from gitea.com)

Awesome. Thanks for applying the patch and updating the branch. I'll do my very best giving it a final check today or tomorrow.

❤️ 3 🎉 3

vjm commented

2024-11-08 19:09:06 +00:00

(Migrated from gitea.com)

@justusbunsi, this is still good to merge, just upgraded the act runner image tag: https://gitea.com/gitea/act_runner/releases/tag/v0.2.11

❤️ 1

justusbunsi commented

2024-11-10 13:34:41 +00:00

(Migrated from gitea.com)

Awesome. Thanks for applying the patch and updating the branch. I'll do my very best giving it a final check today or tomorrow.

Ok, this obviously didn't make out. My apologies for not letting you know. 😅
I have now re-reviewed the changes. This PR is good to merge as-is. Will merge it in a second...

Thank you again @vjm for your commitment to get this Pull Request over the finish line. Highly appreciate it.

By the way, anyone else having issues running the recent dind images on containerd? For me, the last working image is 23.x.x-dind. I always had to modify the values to test it locally. Any tips from those following this PR?

(I posted this with https://gitea.com/gitea/helm-chart/pulls/666#issuecomment-844643 some time ago) For me, this is still an issue. I did some research and for whatever reason I have to add DOCKER_IPTABLES_LEGACY=1 as environment variable for the dind image¹. Don't know what's the issue on my side. But I seem not the only person having these issues.

So I am merging this PR now, then implement a way to allow custom envs for the dind image. Which makes it more flexible for different environments.

After that, I'll create a release so that all of you can use this long-awaited Chart feature.

https://github.com/docker-library/docker/issues/463#issuecomment-1881909456 ↩︎

> Awesome. Thanks for applying the patch and updating the branch. I'll do my very best giving it a final check today or tomorrow. Ok, this obviously didn't make out. My apologies for not letting you know. 😅 I have now re-reviewed the changes. This PR is good to merge as-is. Will merge it in a second... Thank you again @vjm for your commitment to get this Pull Request over the finish line. Highly appreciate it. --- > By the way, anyone else having issues running the recent dind images on containerd? For me, the last working image is 23.x.x-dind. I always had to modify the values to test it locally. Any tips from those following this PR? (I posted this with https://gitea.com/gitea/helm-chart/pulls/666#issuecomment-844643 some time ago) For me, this is still an issue. I did some research and for whatever reason I have to add `DOCKER_IPTABLES_LEGACY=1` as environment variable for the `dind` image[^1]. Don't know what's the issue on my side. But I seem not the only person having these issues. So I am merging this PR now, then implement a way to allow custom envs for the `dind` image. Which makes it more flexible for different environments. After that, I'll create a release so that all of you can use this long-awaited Chart feature. [^1]: https://github.com/docker-library/docker/issues/463#issuecomment-1881909456

justusbunsi (Migrated from gitea.com) approved these changes 2024-11-10 13:35:05 +00:00

justusbunsi (Migrated from gitea.com) left a comment

🚀

justusbunsi commented

2024-11-10 14:08:49 +00:00

(Migrated from gitea.com)

Released as https://gitea.com/gitea/helm-chart/releases/tag/v10.6.0

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#666