Alon Bar-Lev has posted comments on this change.
Change subject: packaging: Allow restore to different db location
......................................................................
Patch Set 5:
(13 comments)
....................................................
File packaging/bin/engine-backup.sh
Line 111
Line 112
Line 113
Line 114
Line 115
here you should verify args... if we change database credentials we should have
non empty host, database, user, password.
all the other should have sane defaults.
Line 75:
Line 76: MODE=
Line 77: SCOPE=all
Line 78: START_SERVICES=0
Line 79: CHANGE_DB_CREDENTIALS=0
= empty is false and ask for [ -n ] or [ -z ]
Line 80: MY_ENGINE_DB_HOST=
Line 81: MY_ENGINE_DB_PORT=
Line 82: MY_ENGINE_DB_USER=
Line 83: MY_ENGINE_DB_PASSWORD=
Line 77: SCOPE=all
Line 78: START_SERVICES=0
Line 79: CHANGE_DB_CREDENTIALS=0
Line 80: MY_ENGINE_DB_HOST=
Line 81: MY_ENGINE_DB_PORT=
set default psql port
Line 82: MY_ENGINE_DB_USER=
Line 83: MY_ENGINE_DB_PASSWORD=
Line 84: MY_ENGINE_DB_DATABASE=
Line 85: MY_ENGINE_DB_SECURED=
Line 81: MY_ENGINE_DB_PORT=
Line 82: MY_ENGINE_DB_USER=
Line 83: MY_ENGINE_DB_PASSWORD=
Line 84: MY_ENGINE_DB_DATABASE=
Line 85: MY_ENGINE_DB_SECURED=
=False
Line 86: MY_ENGINE_DB_SECURED_VALIDATION=
Line 87:
Line 88: parseArgs() {
Line 89: while [ -n "$1" ]; do
Line 82: MY_ENGINE_DB_USER=
Line 83: MY_ENGINE_DB_PASSWORD=
Line 84: MY_ENGINE_DB_DATABASE=
Line 85: MY_ENGINE_DB_SECURED=
Line 86: MY_ENGINE_DB_SECURED_VALIDATION=
=False
Line 87:
Line 88: parseArgs() {
Line 89: while [ -n "$1" ]; do
Line 90: local x="$1"
Line 141: ;;
Line 142: --db-secured)
Line 143: MY_ENGINE_DB_SECURED="True"
Line 144: ;;
Line 145: --no-db-secured)
why do we need both? default is unsecured and override if it does.
Line 146: MY_ENGINE_DB_SECURED="False"
Line 147: ;;
Line 148: --db-sec-validation)
Line 149: MY_ENGINE_DB_SECURED_VALIDATION="True"
Line 147: ;;
Line 148: --db-sec-validation)
Line 149: MY_ENGINE_DB_SECURED_VALIDATION="True"
Line 150: ;;
Line 151: --no-db-sec-validation)
same
Line 152: MY_ENGINE_DB_SECURED_VALIDATION="False"
Line 153: ;;
Line 154: --help)
Line 155: usage
Line 262: fi
Line 263:
Line 264: log "Reloading configuration"
Line 265: load_config
Line 266: [ "${CHANGE_DB_CREDENTIALS}" = 1 ] && changeDBConf
this is like writing xxx == True in code... while only false value is actually
known.
Line 267: log "Generating pgpass"
Line 268: generatePgPass # Must run after configuration reload
Line 269: log "Verifying connection"
Line 270: verifyConnection
Line 325: done || logdie "Cannot read ${paths}"
Line 326: }
Line 327:
Line 328: changeDBConf() {
Line 329: local
conf="/etc/ovirt-engine/engine.conf.d/10-setup-database.conf"
please use engine-prolog variables
Line 330: [ -f "${conf}" ] || logdie "Can not find ${conf}"
Line 331:
Line 332: [ "${MY_ENGINE_DB_HOST}" ] && \
Line 333: ENGINE_DB_HOST="${MY_ENGINE_DB_HOST}"
Line 341: ENGINE_DB_DATABASE="${MY_ENGINE_DB_DATABASE}"
Line 342: [ "${MY_ENGINE_DB_SECURED}" ] && \
Line 343: ENGINE_DB_SECURED="${MY_ENGINE_DB_SECURED}"
Line 344: [ "${MY_ENGINE_DB_SECURED_VALIDATION}" ] && \
Line 345:
ENGINE_DB_SECURED_VALIDATION="${MY_ENGINE_DB_SECURED_VALIDATION}"
not sure why the above is needed.
anyway, even if it does, please declare all local variables.
but I am almost sure they are not needed.
Line 346:
Line 347: local jdbcTlsOptions1=''
Line 348: local jdbcTlsOptions2=''
Line 349: [ "${ENGINE_DB_SECURED}" = "True" ] && \
Line 346:
Line 347: local jdbcTlsOptions1=''
Line 348: local jdbcTlsOptions2=''
Line 349: [ "${ENGINE_DB_SECURED}" = "True" ] && \
Line 350: jdbcTlsOptions1='ssl=true'
why don't you just concat to options?
local options
[ xxx ] && options="${options}&ssl=true"
...
[ -n "${options}" ] && options="${x#&}"
Line 351: [ "${ENGINE_DB_SECURED_VALIDATION}" != "True" ] && \
Line 352:
jdbcTlsOptions2='&sslfactory=org.postgresql.ssl.NonValidatingFactory'
Line 353: local jdbcTlsOptions="${jdbcTlsOptions1}${jdbcTlsOptions2}"
Line 354:
Line 351: [ "${ENGINE_DB_SECURED_VALIDATION}" != "True" ] && \
Line 352:
jdbcTlsOptions2='&sslfactory=org.postgresql.ssl.NonValidatingFactory'
Line 353: local jdbcTlsOptions="${jdbcTlsOptions1}${jdbcTlsOptions2}"
Line 354:
Line 355: escpass="$(echo ${ENGINE_DB_PASSWORD} | sed
's;\(["\$]\);\\\1;g')"
local
Line 356:
Line 357: log "Backing up ${conf} to ${backup}"
Line 358: local backup="${conf}.$(date +"%Y%m%d%H%M%S")"
Line 359: cp -a "${conf}" "${backup}" || die "Failed to backup ${conf}"
Line 361: cat << __EOF__ > "${conf}"
Line 362: ENGINE_DB_HOST=${ENGINE_DB_HOST}
Line 363: ENGINE_DB_PORT=${ENGINE_DB_PORT}
Line 364: ENGINE_DB_USER=${ENGINE_DB_USER}
Line 365: ENGINE_DB_PASSWORD="${escpass}"
you can put the sed here
Line 366: ENGINE_DB_DATABASE=${ENGINE_DB_DATABASE}
Line 367: ENGINE_DB_SECURED=${ENGINE_DB_SECURED}
Line 368: ENGINE_DB_SECURED_VALIDATION=${ENGINE_DB_SECURED_VALIDATION}
Line 369: ENGINE_DB_DRIVER=org.postgresql.Driver
--
To view, visit http://gerrit.ovirt.org/20258
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2c84111808c638a1c589cf3c14c271184a0a7c2
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
