Added easyappointments catalog item to cattle #804
Conversation
| label: 'Mysql Host' | ||
| description: 'Leave empty to spin up a mysql docker container' | ||
| type: string | ||
| required: false |
There was a problem hiding this comment.
What do you think to replace mysql_host type string to type service??
- variable: mysql_host
label: 'Mysql Host'
description: 'Leave empty to spin up a mysql docker container'
- type: string
+ type: service
required: false
There was a problem hiding this comment.
I'm not familiar with type service
There was a problem hiding this comment.
type service shows combo box wit a list of stack/services running at rancher.. could be exposed at service as link or external_link... Useful when you have more than one ip to expose and it's resilient to service update/upgrade...
There was a problem hiding this comment.
Um, yeah that's not what MySQL host is for. You only fill it out if you're doing an external connection
| - 1 | ||
| - 1.2.0 | ||
| - 1.2 | ||
| - latest |
There was a problem hiding this comment.
Use generic docker image tags, implicit or explicit, is a bad practice due to is not reproducible. Otherwise, catalog packages manage versions, update,...
Could you please add one catalog package version for each docker image tag, one for 1.2.0 other for 1.3.0 and remove latest??
|
|
||
| catalog: | ||
| name: 'Easy!Appointments' | ||
| version: 0.1.0 |
There was a problem hiding this comment.
What do you think if you match this version with service version??
- version: 0.1.0
+ version: 1.3.0
There was a problem hiding this comment.
Trying to make it standard due to it's more clear.. Package version 1.3.0 install service version 1.3.0
| name: 'Easy!Appointments' | ||
| description: | | ||
| Open source appointment scheduler | ||
| version: 0.1.0 |
There was a problem hiding this comment.
What do you think if you match this version with service version??
-version: 0.1.0
+version: 1.3.0
| - {{.Stack.Name}}--mysql-data:/var/lib/mysql | ||
| labels: | ||
| io.rancher.container.hostname_override: container_name | ||
| {{- end}} |
There was a problem hiding this comment.
To use golang templating, you need to rename templates/easyappointments/0/docker-compose.yml to templates/easyappointments/0/docker-compose.yml.tpl. Build is failing due to that.
| DB_USERNAME: ${mysql_user} | ||
| {{- if (.Values.mysql_host)}} | ||
| DB_HOST: ${mysql_host} | ||
| {{- end}} |
There was a problem hiding this comment.
What is the default value of env var DB_HOST?? What do you think to define like that??
{{- if not (.Values.mysql_host)}}
links:
- mysql:db
+{{- else }}
+ external_links:
+ - ${mysql_host}:db
{{- end }}
environment:
BASE_URL: ${base_url}
DB_NAME: ${mysql_database}
DB_PASSWORD: ${mysql_password}
DB_USERNAME: ${mysql_user}
+ DB_HOST: db
-{{- if (.Values.mysql_host)}}
- DB_HOST: ${mysql_host}
-{{- end}}
There was a problem hiding this comment.
That is so they can connect with a remote database if they so choose to. Read the description of the msyql_host question in the rancher config. Also, the default value of DB_HOST is db, hence the reason I link to db when mysql_host is not set.
There was a problem hiding this comment.
Yeah, i know what you are doing here and it works. Just a proposal that may be more clear.
|
Hi @codejamninja . Thanks for the PR and contribute. Could you please take a look to changes requested?? |
|
Closing this due to staleness. Feel free to reopen or open a new PR if there's still a relevant change to be made. Thanks! |
Fixes issue #803