[bookstack] README does not mention that apparently a database is required?

kastl-ars commented 1 month ago

Chart Name

bookstack

Chart Version

0.17.2

Bug Description

I used the installation instructions from the README, without any values.yaml file.

helm install bookstack gabe565/bookstack

The bookstack pod threw a warning in the logs that no database could be reached.

Setting mariadb.enabled to true added a database instance, and after deleting the pods the new pod came up fine and started the database migrations.

I would propose to add a --set mariadb.enabled=true to the example instructions.

values.yaml

No response

Relevant log output

************************************************************************                                                      
Waiting for DB to be available                                                                                                

   Illuminate\Database\QueryException                                                                                         

  SQLSTATE[HY000] [2002] Connection refused (Connection: mysql, SQL: select table_name as `name`, (data_length + index_length) as `size`, table_comment as `comment`, engine as `engine`, table_collation as `collation` from information_schema.tables where
 table_schema = 'bookstack' and table_type in ('BASE TABLE', 'SYSTEM VERSIONED') order by table_name)

  at /app/www/vendor/laravel/framework/src/Illuminate/Database/Connection.php:829
    825▕                     $this->getName(), $query, $this->prepareBindings($bindings), $e
    826▕                 );                                                                                                   
    827▕             }                                                                                                        
    828▕                                                                                                                      
  ➜ 829▕             throw new QueryException(                                                                                
    830▕                 $this->getName(), $query, $this->prepareBindings($bindings), $e
    831▕             );                                                                                                       
    832▕         }                                                                                                            
    833▕     }                                                                                                                

      +39 vendor frames 

  40  /app/www/artisan:35
      Illuminate\Foundation\Console\Kernel::handle()

[custom-init] No custom files found, skipping...
[ls.io-init] done.

gabe565 commented 1 month ago

@kastl-ars Sorry for the trouble! I've gotten a few issues about this now, so I'll go ahead and enable it by default. Also note that persistence is disabled by default! Make sure you set the following values:

persistence:
  config:
    enabled: true

mariadb:
  primary:
    persistence:
      enabled: true

Do you think those should also be enabled by default? Without them, data will not be persisted between container restarts.

kastl-ars commented 1 month ago

Thanks for taking care, @gabe565.

I think having "sane" defaults, aka a fully working setup, is a good idea. So I would vote for enabling both mariadb and persistence.

And I would add a stanza to the README to describe the defaults, i.e. that persistence is enabled and can be tweaked by setting e.g. the storageClass. And that a database is required, the default is to use mariadb, but you can bring your own if you wish.

gabe565 / charts