-
Notifications
You must be signed in to change notification settings - Fork 1
Salmanmkc/1 cleanup #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: user/bosira/remote-powershell-script
Are you sure you want to change the base?
Salmanmkc/1 cleanup #2
Conversation
| $Version = $Version.TrimStart('v') | ||
| Write-Output "* Downloading and installing Containerd v$version at $InstallPath" | ||
| "Downloading and installing Containerd v$version at $InstallPath" >> logs | ||
| "{0} - Downloading and installing Containerd v$version at $InstallPath" -f (Get-Date) >> logs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timestamp is a good addition. However, we do not need to redirect each message to a logger, since we can do this: Install-Containerd >> logs.txt. This way, the code will be much cleaner and we'll not need to remember to add >> logs to every log.
| $dirPath = "C:\Users\$env:USERNAME\VirtualBox VMs\$VMName" | ||
| & VBoxManage controlvm $VMName poweroff --type headless | ||
| & VBoxManage unregistervm $VMName --delete | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log message for success and if the plaform is not supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a validated set to this file too
| } | ||
| } | ||
| else { | ||
| Write-Warning "The $dirPath directory doesn't exist." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya good point
| if ($VMName) { | ||
| try{ | ||
| if ($Platform -eq "Hyper-V") { | ||
| $dirPath = "C:\Users\$env:USERNAME\.minikube\machines\$VMName" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split HyperV and Virtual box to seperate functions to make it easier to mange each of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, eventually will do, this is in draft state, not finished
| if($Platform -eq "Hyper-V"){ | ||
| # set the vm switch first | ||
| $Switch = Set-VmSwitch -SwitchName $SwitchName | ||
| $VM = @{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split HyperV and Virtual box to seperate functions to make it easier to mange each of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya will do eventually, wanted to push code a bit earlier, will clean up eventually when finished with everything else on this PR
| # Add-VMDvdDrive -VMName $VMName -Path "$PSScriptRoot\auto-install.iso" -ControllerNumber 1 -ControllerLocation 1 | ||
| Start-VM -Name $VMName | Out-Null | ||
| if($Platform -eq "Hyper-V"){ | ||
| $VM = @{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split HyperV and Virtual box to seperate functions to make it easier to mange each of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup will do
DRAFT state, but just creating it early.
Changes