-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable binding of postgres CF db service to CF hosted console #1231
Conversation
- TODO Tidy - TODO Exit code & stop 'process has crashed with type: "web"' - TODO Use db from service env var in backend
- Also in ALL cases app must be restart via cf push -c "null"
- with persistant storage code to automatically register hosting cf endpoint fails
components/app-core/backend/cnsi.go
Outdated
func (p *portalProxy) cnsiRecordExists(endpoint string) bool { | ||
log.Debug("cnsiRecordExists") | ||
|
||
if _, err := p.GetCNSIRecordByEndpoint(endpoint); err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the if:
_, err := p.GetCNSIRecordByEndpoint(endpoint)
return err == nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few things to consider
@@ -0,0 +1,68 @@ | |||
package datastore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in the cloud-foundry-hosting component
@@ -312,7 +312,10 @@ func loadPortalConfig(pc interfaces.PortalConfig) (interfaces.PortalConfig, erro | |||
|
|||
func loadDatabaseConfig(dc datastore.DatabaseConfig) (datastore.DatabaseConfig, error) { | |||
log.Debug("loadDatabaseConfig") | |||
if err := config.Load(&dc); err != nil { | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a plugin extension point so that the CF-specific hosting code can go in the cloud foundry hosting component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a look and also talked to Irfan, it's not something we can do at the moment. This is called right at the beginning of the PP start up process before the portal proxy object and plugins are initialised. Fixing this would require a bit of a rework.
|
||
set -e | ||
|
||
echo "Attempting to migrating database" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bad grammar - "Attempting to migrate database"
@@ -0,0 +1,35 @@ | |||
# Associated a Cloud Foundry database service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be "Associate"
echo "Migrating postgresql instance on $DB_HOST" | ||
$GOBIN/goose -env cf_postgres up | ||
if [ $? -eq 0 ]; then | ||
while sleep 60; do echo "Database successfully migrated. Please restart the application via 'cf push -c \"null\"'"; done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to print the message immediately and then every 60 seconds - at present, you don't get the initial message until after 60 seconds.
|
||
for (var y = 0; y < serviceInstance.tags.length; y++) { | ||
var tag = serviceInstance.tags[y]; | ||
if (tag === 'postgresql') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if "stratos_postgresql" would be a better tag name?
@@ -23,7 +23,12 @@ case $DB_TYPE in | |||
echo "Migrating postgresql instance on $DB_HOST" | |||
$GOBIN/goose -env cf_postgres up | |||
if [ $? -eq 0 ]; then | |||
while sleep 60; do echo "Database successfully migrated. Please restart the application via 'cf push -c \"null\"'"; done | |||
while |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its more common to put:
while true; do
echo "Database successfully migrated. Please restart the application via 'cf push -c "null"'";
sleep 60
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
See README in deploy/cloud-foundry/db-migration/README.md, but roughly..