fix(init): fixed init logic #310

cboin1996 · 2022-03-30T14:05:09Z

cboin1996 commented

2022-03-30 14:05:09 +00:00

Description of the change

Checking the existence of the config directory should be done with the directory path itself. Not its parent directory.

This simple fix addresses that by using the config directory for its existence check.

Benefits

Prior to #337 there was no other way to install this helm chart using the extraVolumeMounts setting with these values:

replicaCount: %d  extraVolumes:  - name: config-volume  configMap:  name: %s  extraVolumeMounts:  - name: config-volume  mountPath: /data/gitea/templates/custom

Without this fix, the Gitea pod would never initialize, and would crashloop with the same error in #296.

Additional information

Mounting a configMap to /data/gitea/templates/custom causes the /data/gitea folder to exist even though the /data/gitea/conf had not been initialized yet. The initialization script saw that the /data/gitea dir existed and exited early without initializing /data/gitea/conf.

### Description of the change Checking the existence of the config directory should be done with the directory path itself. Not its parent directory. This simple fix addresses that by using the config directory for its existence check. ### Benefits Prior to #337 there was no other way to install this helm chart using the `extraVolumeMounts` setting with these values: ```yaml replicaCount: %d extraVolumes: - name: config-volume configMap: name: %s extraVolumeMounts: - name: config-volume mountPath: /data/gitea/templates/custom ``` Without this fix, the Gitea pod would never initialize, and would crashloop with the same error in #296. ### Additional information Mounting a configMap to `/data/gitea/templates/custom` causes the `/data/gitea` folder to exist even though the `/data/gitea/conf` had not been initialized yet. The initialization script saw that the `/data/gitea` dir existed and exited early without initializing `/data/gitea/conf`.

cboin1996 added 1 commit 2022-03-30 14:05:10 +00:00

fix(init): fixed init logic

continuous-integration/drone/pr Build is passing

Details

c59d776ad1

justusbunsi added the

kind

bug

label 2022-04-21 11:23:33 +00:00

justusbunsi commented

2022-04-21 11:27:56 +00:00

@viceice Does this break your environment regarding #210 and #211?

justusbunsi commented

2022-07-10 15:26:51 +00:00

Hi @cboin1996. Sorry for not responding earlier. I wasn't sure if it actually fixes the root cause and not only a symptom. IMO, the issue is that mount points which are only relevant during Gitea runtime are also mounted into the initialization phase of the Helm Chart. I've created PR #337 which splits the extraVolumeMounts into two different mount point settings. Doing so ensures that the init scripts are not interfered by customization mount points.

~~If it's OK for you, I'd like to close your PR in favor of #337.~~

Hi @cboin1996. Sorry for not responding earlier. I wasn't sure if it actually fixes the root cause and not only a symptom. IMO, the issue is that mount points which are only relevant during Gitea runtime are also mounted into the initialization phase of the Helm Chart. I've created PR #337 which splits the `extraVolumeMounts` into two different mount point settings. Doing so ensures that the init scripts are not interfered by customization mount points. ~~If it's OK for you, I'd like to close your PR in favor of #337.~~

viceice commented

2022-08-24 13:38:19 +00:00

Not sure if this breakes me. I've now migrated to chart v6 and don't see any issues yet.
My only last workaround is this:

 initPreScript: |  [ -d /data/gitea ] && chmod 700 /data/gitea

Not sure if it's still required.

Not sure if this breakes me. I've now migrated to chart v6 and don't see any issues yet. My only last workaround is this: ```yaml initPreScript: | [ -d /data/gitea ] && chmod 700 /data/gitea ``` Not sure if it's still required.

justusbunsi commented

2022-09-25 12:35:49 +00:00

@viceice

My only last workaround is this:
 initPreScript: |  [ -d /data/gitea ] && chmod 700 /data/gitea 
Not sure if it's still required.

I guess it shouldn't be necessary anymore when using the different volumeMount definitions. The need of such workaround makes sense when already having a directory structure next to the config directory. Which is the case when having template mounts or something similar.

@viceice > My only last workaround is this: > > ```yaml > > initPreScript: | > [ -d /data/gitea ] && chmod 700 /data/gitea > ``` > > Not sure if it's still required. I guess it shouldn't be necessary anymore when using the different volumeMount definitions. The need of such workaround makes sense when already having a directory structure next to the config directory. Which is the case when having template mounts or something similar.

justusbunsi approved these changes 2022-09-25 12:45:53 +00:00

justusbunsi left a comment

Thinking about the setup workflow and reviewing #337, your suggested change is still a valid one - LGTM.

👍 1

justusbunsi commented

2022-09-25 13:03:09 +00:00

I've restructured the description to take both the PR template and #337 into account.

👍 1

pat-s approved these changes 2022-09-25 20:06:10 +00:00

pat-s left a comment

This is also related to #296 and might have helped there by exiting early.

Checking for the final dir with test -d makes sense to me, don't see anything troublesome in this change.

This is also related to #296 and might have helped there by exiting early. Checking for the final dir with `test -d` makes sense to me, don't see anything troublesome in this change.

pat-s merged commit 0d1f748898 into main

2022-09-25 20:08:56 +00:00

pat-s referenced this issue from a commit

2022-09-25 20:08:57 +00:00

check existence of `/data/gitea/conf/` instead of `/data/gitea/` (#310)

cboin1996 commented

2022-09-26 01:31:31 +00:00

thanks guys!

Sign in to join this conversation.