lunny/helm-chart
1
0
Fork 0
Code Issues 36 Pull Requests 14 Actions Packages Projects Releases 93 Wiki Activity

"Chaos user" test results #564

New Issue
Open
opened 2023-11-14 18:19:33 +00:00 by justusbunsi · 3 comments
justusbunsi commented 2023-11-14 18:19:33 +00:00 (Migrated from gitea.com)
Copy Link

One of my colleagues tried to use the recent Chart version (9.5.1) and got slapped in the face. There are several issues with the templating behavior we need to fix:

Mix 'n' Mesh the dependencies

(values.yaml)

postgresql:
  enabled: true
postgresql-ha:
  enabled: false
redis-cluster:
  enabled: false # <-- this is the most important part here

Disabling redis-cluster and don't change any other default values leads to a fresh install that stuck due to invalid session configuration:

(generated app.ini)

PROVIDER=redis
PROVIDER_CONFIG=

causes crash loops.

2023/11/14 18:08:22 cmd/web.go:212:func1() [F] PANIC: dial tcp 127.0.0.1:6379: connect: connection refused
/go/pkg/mod/gitea.com/go-chi/session@v0.0.0-20230415140235-3182bcc14852/session.go:239 (0x185e997)
/go/src/code.gitea.io/gitea/routers/common/middleware.go:104 (0x248f971)
/go/src/code.gitea.io/gitea/routers/web/web.go:147 (0x269f5cb)
/go/src/code.gitea.io/gitea/routers/init.go:176 (0x270c43c)
/go/src/code.gitea.io/gitea/cmd/web.go:192 (0x27fbb44)
/go/src/code.gitea.io/gitea/cmd/web.go:243 (0x27fc052)
/go/src/code.gitea.io/gitea/main.go:194 (0x283bc42)
/go/pkg/mod/github.com/urfave/cli@v1.22.13/app.go:524 (0x135e54f)
/go/pkg/mod/github.com/urfave/cli@v1.22.13/app.go:410 (0x135cebc)
/go/pkg/mod/github.com/urfave/cli@v1.22.13/command.go:380 (0x136135e)
/go/pkg/mod/github.com/urfave/cli@v1.22.13/command.go:103 (0x135f564)
/go/pkg/mod/github.com/urfave/cli@v1.22.13/app.go:277 (0x135c046)
/go/src/code.gitea.io/gitea/cmd/main.go:14 (0x27f33e6)
/go/src/code.gitea.io/gitea/main.go:155 (0x283b63e)
/usr/local/go/src/runtime/proc.go:250 (0x44c746)
/usr/local/go/src/runtime/asm_amd64.s:1598 (0x481340)

#356 kills any positive user-experience when trying things

Changing configuration settings and especially removing redis-related entries like HOST does not clean up the app.ini, causing an inconsistent configuration.

non-cluster Redis is useful

We don't hear the first time that a non-cluster redis can help users getting Gitea up and running in smaller environments where Kubernetes is still used. It also helps generating a consistent app.ini.

One of my colleagues tried to use the recent Chart version (9.5.1) and got slapped in the face. There are several issues with the templating behavior we need to fix: ## Mix 'n' Mesh the dependencies (values.yaml) ```yaml postgresql: enabled: true postgresql-ha: enabled: false redis-cluster: enabled: false # <-- this is the most important part here ``` Disabling redis-cluster and don't change any other default values leads to a fresh install that stuck due to invalid session configuration: (generated app.ini) ```ini PROVIDER=redis PROVIDER_CONFIG= ``` causes crash loops. ```text 2023/11/14 18:08:22 cmd/web.go:212:func1() [F] PANIC: dial tcp 127.0.0.1:6379: connect: connection refused /go/pkg/mod/gitea.com/go-chi/session@v0.0.0-20230415140235-3182bcc14852/session.go:239 (0x185e997) /go/src/code.gitea.io/gitea/routers/common/middleware.go:104 (0x248f971) /go/src/code.gitea.io/gitea/routers/web/web.go:147 (0x269f5cb) /go/src/code.gitea.io/gitea/routers/init.go:176 (0x270c43c) /go/src/code.gitea.io/gitea/cmd/web.go:192 (0x27fbb44) /go/src/code.gitea.io/gitea/cmd/web.go:243 (0x27fc052) /go/src/code.gitea.io/gitea/main.go:194 (0x283bc42) /go/pkg/mod/github.com/urfave/cli@v1.22.13/app.go:524 (0x135e54f) /go/pkg/mod/github.com/urfave/cli@v1.22.13/app.go:410 (0x135cebc) /go/pkg/mod/github.com/urfave/cli@v1.22.13/command.go:380 (0x136135e) /go/pkg/mod/github.com/urfave/cli@v1.22.13/command.go:103 (0x135f564) /go/pkg/mod/github.com/urfave/cli@v1.22.13/app.go:277 (0x135c046) /go/src/code.gitea.io/gitea/cmd/main.go:14 (0x27f33e6) /go/src/code.gitea.io/gitea/main.go:155 (0x283b63e) /usr/local/go/src/runtime/proc.go:250 (0x44c746) /usr/local/go/src/runtime/asm_amd64.s:1598 (0x481340) ``` ## #356 kills any positive user-experience when trying things Changing configuration settings and especially removing redis-related entries like `HOST` does not clean up the app.ini, causing an inconsistent configuration. ## non-cluster Redis is useful We don't hear the first time that a non-cluster redis can help users getting Gitea up and running in smaller environments where Kubernetes is still used. It also helps generating a consistent app.ini.
pat-s commented 2023-11-14 22:06:43 +00:00 (Migrated from gitea.com)
Copy Link

Thanks for the feedback.

Disabling redis-cluster and don't change any other default values leads to a fresh install that stuck due to invalid session configuration:

We are aware of this. I'd argue that when disabling any default of a chart one should not expect a working result without any custom input (no matter if #356 is solved or not).

In this case, it is expected that users define working definition for whatever session provider they want to use instead. I think this behavior was the same with memcache before, i.e. if you would have disabled it, you would have been in need for a proper alternative definition.

One way would be to fall back to a "in memory" one but given that this is not a suitable one to run the chart, I'd actually favor to fail in this case.

Second, the README explains this behavior and provides examples how to configure session & friends when turning off redis. If these are not clear enough, we can/should update it, but we need feedback on what parts are not clear enough.

Changing configuration settings and especially removing redis-related entries like HOST does not clean up the app.ini, causing an inconsistent configuration.

Yes, we know this. While this was present before the move to v9, this has been especially painful due to the config changes of v9.

non-cluster Redis is useful

I think the question is not so much "redis-cluster or non-cluster redis" but rather what could provide an alternative to redis in the first place. Redis is great for speed, efficiency and reliability for session, queue and cache, no matter if running in HA or not. A less performant but sufficient alternative can be memcached and my preference/suggestion would be to direct users to c/p example in the README rather than suggestion a one-pod-less non-cluster redis alternative.

Thanks for the feedback. > Disabling redis-cluster and don't change any other default values leads to a fresh install that stuck due to invalid session configuration: We are aware of this. I'd argue that when disabling any default of a chart one should not expect a working result without any custom input (no matter if #356 is solved or not). In this case, it is expected that users define working definition for whatever session provider they want to use instead. I think this behavior was the same with memcache before, i.e. if you would have disabled it, you would have been in need for a proper alternative definition. One way would be to fall back to a "in memory" one but given that this is not a suitable one to run the chart, I'd actually favor to fail in this case. Second, the README explains this behavior and provides examples how to configure `session` & friends when turning off redis. If these are not clear enough, we can/should update it, but we need feedback on what parts are not clear enough. > Changing configuration settings and especially removing redis-related entries like HOST does not clean up the app.ini, causing an inconsistent configuration. Yes, we know this. While this was present before the move to v9, this has been especially painful due to the config changes of v9. > non-cluster Redis is useful I think the question is not so much "redis-cluster or non-cluster redis" but rather what could provide an alternative to redis in the first place. Redis is great for speed, efficiency and reliability for session, queue and cache, no matter if running in HA or not. A less performant but sufficient alternative can be memcached and my preference/suggestion would be to direct users to c/p example in the README rather than suggestion a one-pod-less non-cluster redis alternative.
AntonOfTheWoods commented 2023-11-22 07:00:46 +00:00 (Migrated from gitea.com)
Copy Link

@pat-s the main issue is that the main container simply errors before giving any sort of error message at all, and at least I personally had the expectation that it would either have a fallback or a comprehensible error message.

Particularly given the README gives the following:

redis-cluster:
  enabled: false
postgresql:
  enabled: true
postgresql-ha:
  enabled: false

persistence:
  enabled: true

gitea:
  config:
    database:
      DB_TYPE: postgres
    session:
      PROVIDER: db
    cache:
      ADAPTER: memory
    queue:
      TYPE: level
    indexer:
      ISSUE_INDEXER_TYPE: bleve
      REPO_INDEXER_ENABLED: true

And claims "For a production-ready single-pod Gitea instance without external dependencies (using the chart dependency postgresql):"

I'm pretty sure a natural reading of that phrase suggests it will be a valid "production-ready" config... no?

@pat-s the main issue is that the main container simply errors before giving any sort of error message at all, and at least I personally had the expectation that it would either have a fallback or a comprehensible error message. Particularly given the `README` gives the following: ``` redis-cluster: enabled: false postgresql: enabled: true postgresql-ha: enabled: false persistence: enabled: true gitea: config: database: DB_TYPE: postgres session: PROVIDER: db cache: ADAPTER: memory queue: TYPE: level indexer: ISSUE_INDEXER_TYPE: bleve REPO_INDEXER_ENABLED: true ``` And claims "For a production-ready single-pod Gitea instance without external dependencies (using the chart dependency postgresql):" I'm pretty sure a natural reading of that phrase suggests it will be a valid "production-ready" config... no?
justusbunsi commented 2023-11-22 07:16:31 +00:00 (Migrated from gitea.com)
Copy Link

@AntonOfTheWoods

@pat-s the main issue is that the main container simply errors before giving any sort of error message at all, and at least I personally had the expectation that it would either have a fallback or a comprehensible error message.

Particularly given the README gives the following:

redis-cluster:
 enabled: false
postgresql:
 enabled: true
postgresql-ha:
 enabled: false

persistence:
 enabled: true

gitea:
 config:
   database:
     DB_TYPE: postgres
   session:
     PROVIDER: db
   cache:
     ADAPTER: memory
   queue:
     TYPE: level
   indexer:
     ISSUE_INDEXER_TYPE: bleve
     REPO_INDEXER_ENABLED: true

And claims "For a production-ready single-pod Gitea instance without external dependencies (using the chart dependency postgresql):"

I'm pretty sure a natural reading of that phrase suggests it will be a valid "production-ready" config... no?

Please see my comment in your issue. The sample from README should worked last time I checked. It might be due to the app.ini config bug #356. At least when you first tried the default values.

@AntonOfTheWoods >@pat-s the main issue is that the main container simply errors before giving any sort of error message at all, and at least I personally had the expectation that it would either have a fallback or a comprehensible error message. > >Particularly given the `README` gives the following: > >``` >redis-cluster: > enabled: false >postgresql: > enabled: true >postgresql-ha: > enabled: false > >persistence: > enabled: true > >gitea: > config: > database: > DB_TYPE: postgres > session: > PROVIDER: db > cache: > ADAPTER: memory > queue: > TYPE: level > indexer: > ISSUE_INDEXER_TYPE: bleve > REPO_INDEXER_ENABLED: true >``` > >And claims "For a production-ready single-pod Gitea instance without external dependencies (using the chart dependency postgresql):" > >I'm pretty sure a natural reading of that phrase suggests it will be a valid "production-ready" config... no? Please see my comment in your issue. The sample from README should worked last time I checked. It might be due to the app.ini config bug #356. At least when you first tried the default values.
👍 1
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
has/backport
in progress
invalid
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/proposal
kind/question
kind/refactor
kind/security
kind/testing
kind/translation
kind/ui
need/backport
priority/critical
priority/low
priority/maybe
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/other
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#564
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?