setup option is SQL_INSTANCE_NAME instead of SQLINSTANCE_NAME

[jmunnik]

When installing wsus on Windows Server 2012 R2 we are receiving the following error;

the WID role service is not installed, the instance name must be specified on the commandline

After investigation it seems like a wrong setup_option is used;

recipes/install.rb:setup_options << " SQLINSTANCE_NAME=\"#{setup_conf['sqlinstance_name']}\""

instead of ;

recipes/install.rb:setup_options << " SQL_INSTANCE_NAME=\"#{setup_conf['sqlinstance_name']}\""

Could you please have a look and merge this pull request. Maybe it's cleaner for consistency to use sql_instance_name in all files;

spec/recipes/install_spec.rb: chef_run = windows2012_chef_run(wsus_server: { setup: { sqlinstance_name: nil } }) spec/recipes/install_spec.rb: chef_run = windows2012_chef_run(wsus_server: { setup: { sqlinstance_name: 'instance' } }) attributes/install.rb:default['wsus_server']['setup']['sqlinstance_name'] = nil attributes/install.rb: # Frontend server shares the configuration of the main server, using the value of above attribute sqlinstance_name. recipes/install.rb:setup_options << " SQLINSTANCE_NAME=\"#{setup_conf['sqlinstance_name']}\"" if setup_conf['sqlinstance_name'] recipes/install.rb: action setup_conf['sqlinstance_name'] ? :install : :remove recipes/install.rb: action setup_conf['sqlinstance_name'] ? :remove : :install README.md:sqlinstance_name| Local or remote SQL instance for WSUS configuration| String | nil

With kind regards,

Jos Munnik

[Annih]

hello @jmunnik Thanks for your contribution! You are right the option for the postinstall script on Windows Server 2012R2 is now SQL_INSTANCE_NAME, but it's still SQLINSTANCE_NAME for WSUS 3.0. So the patch is a bit more complex than the one you did.

I'll try to look at it this week, feel free to update your patch :)

[jmunnik]

Hi @Annih , Sorry but I didn't tested this on older versions of windows 2012 r2 but bizar that the options are changed. It's not really consistent but I think the best way then is to define this via an attribute or os version right ? I think this is relatively quick to sort out.

[jmunnik]

Hi @Annih did you already had time to look into this ? pls let me know.. Regards, Jos

[jmunnik]

Hi @Annih, can you have a look at my new pull request and merge it if you agree. I made a distinction now between 2012_r2 SQL_INSTANCE_NAME and for all other OS versions to use SQLINSTANCE_NAME if setup_conf['sqlinstance_name'] is true off course

Regards, Jos

[Annih]

Hello @jmunnik

Nice PR, I commented a bit your commit just to be sure we keep coherence. So instead of using the method helper from win32 version module, I would prefere that you use the platform_version attribute, even if I use the module to determine if the server is a CORE server.

Please also rebase your PR on master branch to get the latest travis config, and pass the test :)

[Annih]

Could you also squash your commits or at least rename the last one.

[Annih]

Thanks @jmunnik, I pushed your fix in my commit.