(MODULES-6356) Fixes a problem still remaining from MODULES-2904 #259
Conversation
|
Hi @kreeuwijk. Thanks for the PR. Did you happen to try on an SQL edition other than 2016 ? |
|
I have not, but I can easily test this on other SQL versions for you. I'll do some tests today. |
|
Tested the following versions, all successfully:
|
|
Thanks @kreeuwijk . I've repro'd that issue too :-( Looks like we need to modify the spec tests as well. Should probably add an acceptance test as this should've been caught when MODULES-2904 was closed. |
Previously the acceptance test suite had been modified to use test tiering and test mode switcher. However the spec acceptance helper method still expected a master to be present. This commit modifies the helper to only execute master provisioning steps if there is indeed a master specified in the hosts array.
As described in MODULES-2904, installation fails with an array of SQL Admins. This was fixed in the use_discrete=true block (line 171-178. However the double quotes also needed to be removed from the list of accounts for the /SQLSYSADMINACCOUNTS switch parameter. This appears to be due to the arguments being interpretted literally, including the double quotes when doing account lookups. Without the fix, the only setting that works is /SQLSYSADMINACCOUNTS="BUILTIN\Administrators", all other settings (both local and domain accounts/groups) will fail, particularly those with whitespace.
glennsarti
force-pushed
the
kreeuwijk-patch-1
branch
from
1dc287a to
fcaea76
Compare
|
Updated the acceptance and unit tests for the change. CI is jammed up at the moment. Can't run through adhoc. Ran locally on my beaker testing ok. |
glennsarti
changed the title
|
@kreeuwijk In future can you please raise a MODULES ticket and perform the work on your personal fork rather than this repo. |
|
Ah ok, sorry that's due to my unfamiliarity with how this process works. |
|
Adhoc worked ok. This PR is ready for merge. |
|
@ThoughtCrhyme You wanna hit the merge button for me... |
As described in MODULES-2904, installation fails with an array of SQL Admins. This was fixed in the use_discrete=true block (line 171-178. However the double quotes also needed to be removed from the list of accounts for the /SQLSYSADMINACCOUNTS switch parameter.
This PR removes the double quotes in the use_discrete=true section (which is only used for /SQLSYSADMINACCOUNTS). I have tested this to sucessfully fix installation for the following scenario's:
Without the fix, the only setting that works is /SQLSYSADMINACCOUNTS="BUILTIN\Administrators", all other settings (both local and domain accounts/groups) will fail.