fix(init): fixed init logic #310

2022-03-30T14:05:09Z

cboin1996 commented

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

(Migrated from gitea.com)

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`.

justusbunsi commented

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

(Migrated from gitea.com)

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

justusbunsi commented

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

(Migrated from gitea.com)

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

(Migrated from gitea.com)

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

(Migrated from gitea.com)

@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 (Migrated from gitea.com) approved these changes 2022-09-25 12:45:53 +00:00

justusbunsi (Migrated from gitea.com) left a comment

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

justusbunsi commented

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

(Migrated from gitea.com)

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

👍 1

pat-s (Migrated from gitea.com) approved these changes 2022-09-25 20:06:10 +00:00

pat-s (Migrated from gitea.com) 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.

cboin1996 commented

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

(Migrated from gitea.com)

thanks guys!

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#310