add ssh option for all servers linux and windows #2
Conversation
morsh
left a comment
There was a problem hiding this comment.
@jackyalbo - I wrote some comments on the changes + it's a little hard to review since those changes include code structure changes, so in many of the changes I'm not sure if they're structure or logic.
| var self = this; | ||
|
|
||
| self.login(function (err) { | ||
| self.login(err => { |
There was a problem hiding this comment.
This format is not supported in the "declared" version of this repo (node 0.10 or something) so it has to be function (err) {...}
[Here and everywhere]
| }, | ||
| 'createOption': 'Empty' | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Why did you remove those lines?
Those lines enable adding multiple data disks when creating/updating a vm
There was a problem hiding this comment.
it was for testing - but i don't want data disk by default? can we make it optional?
There was a problem hiding this comment.
they are optional:
if (options.storageDataDiskNames > 0) ...
| 'https://raw.githubusercontent.com/Azure/azure-quickstart-templates/master/201-vm-winrm-windows/makecert.exe', | ||
| 'https://raw.githubusercontent.com/Azure/azure-quickstart-templates/master/201-vm-winrm-windows/winrmconf.cmd' | ||
| ], | ||
| 'commandToExecute': '[concat(\'powershell -ExecutionPolicy Unrestricted -file ConfigureWinRM.ps1 \',variables(\'hostDNSNameScriptArgument\'))]' |
There was a problem hiding this comment.
Also not clear - for windows you need an extension to enable remote connect - why not add this as part of the ARM deployment and create a separate deployment?
There was a problem hiding this comment.
so the thing is i want to use ssh instead of winRM... I changed this lines with ones that adds ssh support.
There was a problem hiding this comment.
Right, sounds great, but this means two things:
- Would be better to add this using
template.resources[vmIndex].resources = [{... - For this to work E2E you'll need to change cloud-cd to only support ssh (for windows/linux) - but this is not in the scope of this repo
| }, | ||
| "protectedSettings": { | ||
| "storageAccountName": 'pluginsstorage', | ||
| "storageAccountKey": 'bHabDjY34dXwITjXEasmQxI84QinJqiBZHiU+Vc1dqLNSKQxvFrZbVsfDshPriIB+XIaFVaQ2R3ua1YMDYYfHw==' |
There was a problem hiding this comment.
Not sure what this is for, but you can't add constant personal information like:
https://pluginsstorage.blob.core.windows.net/agentscripts/ssh.ps1
pluginsstorage
bHabDjY34dXwITjXEasmQxI84QinJqiBZHiU+Vc1dqLNSKQxvFrZbVsfDshPriIB+XIaFVaQ2R3ua1YMDYYfHw==
[Same for Linux]
There was a problem hiding this comment.
you are correct - my bad - I'll need to find public repo to use instead...
No description provided.