Honor --sql-host, --sql-port and --sql-db for SQL backup/restore (external PostgreSQL)#204
Open
edmharlaoag wants to merge 1 commit into
Open
Honor --sql-host, --sql-port and --sql-db for SQL backup/restore (external PostgreSQL)#204edmharlaoag wants to merge 1 commit into
edmharlaoag wants to merge 1 commit into
Conversation
b00c932 to
d7cc5b7
Compare
knife ec backup --with-user-sql ignored the configured PostgreSQL endpoint and database: it always connected to localhost:5432 and to a database named after the SQL user. External-PostgreSQL deployments failed with PG::ConnectionBad, and servers whose database name differs from the SQL user name could not be backed up at all. Two distinct problems in EcKeyBase: 1. Host/port were never derived from chef-server-running.json, and the --sql-host/--sql-port options carried hardcoded localhost/5432 defaults that masked any other value. Drop those defaults, source sql_host/sql_port from private_chef.postgresql.vip/port in load_config_from_file! (||=, so an explicit flag still wins), and fall back to localhost:5432 in #db when unset. 2. The database name was resolved (from --sql-db or autoconfigure) but never written to the connection URI, so it was silently ignored and PostgreSQL fell back to a user-named database. #db now sets the URI path from config[:sql_db]. This follows the one-line fix proposed by @ioSpark in chef#182. Adds specs for running-config host/port adoption, CLI precedence, and the database name appearing in the connection URI. Refs chef#181, chef#182, chef/chef-server#2907 Signed-off-by: edmharlaoag <laoagedmhar@gmail.com>
d7cc5b7 to
64d9ee5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
knife ec backup --with-user-sqldoes not honor the SQL connection flagsand silently targets the wrong PostgreSQL server and database:
localhost:5432and fails with
PG::ConnectionBad: Connection refused.named after the SQL user — servers whose DB name differs from the user
name can't be backed up at all (issue
--sql-dbdoes not take effect #181).Root cause
Both problems live in
EcKeyBase:Host/port —
load_config_from_file!autoconfiguressql_user/sql_password/sql_dbfrom/etc/opscode/chef-server-running.jsonbutnever reads the PostgreSQL host or port, while
--sql-host/--sql-portare declared with hardcodedlocalhost/5432defaults thatalways populate the config — so the target can never be derived from the
running config.
Database name —
#dbbuilds the connection URI from host, port, user,password and query params, but never writes
config[:sql_db]into the URIpath, so the resolved database name is dropped and PostgreSQL falls back to
a user-named database (
--sql-dbdoes not take effect #181).Fix
:default => "localhost"/5432from the--sql-host/--sql-portoptions (ec_base.rb,ec_key_base.rb).sql_host/sql_portfromprivate_chef.postgresql.vip/portinload_config_from_file!, using||=so an explicit--sql-host/--sql-portstill wins.#db(
server_uri.path = "/#{config[:sql_db]}" if config[:sql_db]) — the one-linefix originally proposed by @ioSpark in Pass
--sql-dbto URI builder if given #182.localhost:5432in#dbwhen host/port are unset, preservingembedded-PostgreSQL behavior.
Backward compatibility
Embedded-PostgreSQL installs report a loopback
vip, and the#dbfallbackcovers the Automate / no-running-config paths, so those deployments are
unaffected. Explicit CLI flags retain top precedence.
Tests
Adds specs for: host/port adoption from
chef-server-running.json, explicit--sql-host/--sql-portprecedence, and the database name appearing in theconnection URI.
Fixes #181. Refs #182, chef/chef-server#2907.