Skip to content
This repository has been archived by the owner on Jan 24, 2023. It is now read-only.

Enable binding of postgres CF db service to CF hosted console #1231

Merged
merged 14 commits into from
Sep 13, 2017

Conversation

richard-cox
Copy link
Contributor

See README in deploy/cloud-foundry/db-migration/README.md, but roughly..

  • Added script to execute goose against db with settings found in VCAP_SERVICES
  • Enable console to pick up db settings from VCAP_SERVICES
  • Only auto-register + connect to hosting CF if CF has not already been registered

- 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
@richard-cox richard-cox changed the title Enable binding of CF db service to Console Enable binding of postgres CF db service to Console Aug 24, 2017
@richard-cox richard-cox changed the title Enable binding of postgres CF db service to Console Enable binding of postgres CF db service to CF hosted console Aug 24, 2017
func (p *portalProxy) cnsiRecordExists(endpoint string) bool {
log.Debug("cnsiRecordExists")

if _, err := p.GetCNSIRecordByEndpoint(endpoint); err == nil {
Copy link
Contributor

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

Copy link
Contributor

@nwmac nwmac left a 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
Copy link
Contributor

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 {

Copy link
Contributor

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.

Copy link
Contributor Author

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"
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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') {
Copy link
Contributor

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
Copy link
Contributor

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

Copy link
Contributor

@nwmac nwmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nwmac nwmac merged commit 01bd945 into master Sep 13, 2017
@irfanhabib irfanhabib deleted the cf-db-migrate branch November 22, 2017 14:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants