Skip to content

Conversation

@gossion
Copy link

@gossion gossion commented Feb 28, 2017

No description provided.

@cfdreddbot
Copy link

Hey gossion!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/140736235

The labels on this github issue will be updated when the story is started.

@gossion
Copy link
Author

gossion commented Feb 28, 2017

Hi,

I submit this PR in the purpose of supporting MSSQL (Microsoft / Azure SQL Server) for CF Diego.
Currently mysql and postgres are supported in Diego, in this PR I am trying to make mssql (especial for Azure PAAS - Azure SQL server) as another option for Diego databases.

There are 3 PRs to support this feature:

  1. On bbs: Support Microsoft SQL server on diego bbs #17 - Logic to make mssql available for bbs. (this PR)
  2. On locket: Support Microsoft SQL server (mssql) locket#2 - The change is mostly for making diego unit test pass, though I don't see how to use locket in a CF deployment.
  3. On diego-bbs: Support Microsoft Azure SQL Database diego-release#278 - Change job spec and package to support mssql.

Unit test and CF acceptance test were run for these 3 PRs, no issue found. Tests were done against diego v1.7.1 and CF v252

Notes: (If you are okay to do the merge, or if you want to rework these PRs)

  1. PR#1,2 need to be merged before PR#3.

  2. You need to prepare a SQL Server environment to make diego unit test passed when PR#3 is merged. there are two options to prepare the environment:

    A. Test against local SQL server: Setup a SQL server in localhost according to this guide(Guide has also been updated to diego-release/CONTRIBUTING.md). And export a SQL ConnectionString before launching the test, like this:

        export MSSQL_BASE_CONNECTION_STRING="server=localhost;user id=diego;password=Password-123;port=1433"; ./scripts/run-unit-tests     #this connection string should not contain 'database=..' because database will be created automatically when running unit test.
    

    B. Test against an Azure SQL server: You need to create a SQL server on Azure, set the firewall to allow connection from your test IP. And export a SQL ConnectionString before launching the test, like this:

        export MSSQL_BASE_CONNECTION_STRING="server={your_dbserver_name};user id={your_username};password={your_password};port=1433"; ./scripts/run-unit-tests     #this connection string should not contain 'database=..' because database will be created automatically when running unit test.
    
  3. To use on-premise SQL server in CF deployment Diego BBS, you can configure it on properties.diego.bbs.sql on your CF manifest, for example:

    sql:
      ca_cert: null
      db_connection_string: null
      db_driver: mssql
      db_host: {your_dbserver_name}
      db_password: {your_password}
      db_port: 1433
      db_schema: {your_dbname}
      db_username: {your_username}
      max_open_connections: null
      require_ssl: null
    

@emalm
Copy link
Contributor

emalm commented Mar 2, 2017

Hi, @gossion,

Thanks very much for the contribution. While this addition to the BBS to support MS SQL Server looks relatively manageable, maintaining official support for it will require non-trivial ongoing effort from the Diego team, primarily in the form of ensuring that we have adequate ongoing test coverage against this database in our CI pipelines. I understand from @zrob and @dieucao that there will be work happening within the next few weeks for Cloud Controller also to support MS SQL Server, so I'd prefer we learn from that effort before prioritizing similar changes in Diego. We'll leave the PRs open for the time being, of course.

Thanks again,
Eric, CF Diego PM

@emalm
Copy link
Contributor

emalm commented Nov 22, 2017

Hi, @gossion,

Sounds like the effort to add MS-SQL support to Cloud Controller has been stalled for some time. Since we have viewed that support as a prerequisite to supporting it for Diego components, I'm going to close these PRs as stale. If the effort resumes for Cloud Controller and the CAPI team, however, we'd consider new PRs here and on Locket and diego-release.

Best,
Eric, CF Diego PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants