WahlNetwork / vester

Easily validate and remediate your vSphere configuration
https://wahlnetwork.com
Apache License 2.0
146 stars 45 forks source link

Problems with NTP-Servers.vester.ps1 #195

Closed h0bbel closed 6 years ago

h0bbel commented 7 years ago

Expected Behavior

When the NTP Server remediation runs, it should replace whatever values are currently present with the correct settings.

Current Behavior

The test currently it fails if the NTP setting for a host is completely empty. If the NTP setting has a wrong value (eg. 1.1.1.1), it works as expected.

Possible Solution

It seems like the current check doesn't like empty values for NTP servers.

Steps to Reproduce (for bugs)

  1. Create configuration from reference host
  2. Empty the NTP settings for a host in the cluster (Host->Configure->System->Time Configuration->NTP Servers
  3. Run Invoke-Vester -Remediate, which returns: WARNING: Expected: value to be empty but it was { } WARNING: Remediating esx3.bgolab.local [-] Host <hostname> - NTP Servers 63ms ValidationMetadataException: The argument is null or empty. Provide an arg ument that is not null or empty, and then try the command again. ParameterBindingValidationException: Cannot validate argument on parameter 'NtpServer'. The argument is null or empty. Provide an argument that is not nul l or empty, and then try the command again. at <ScriptBlock>, C:\Program Files\WindowsPowerShell\Modules\Vester\1.2.0\ Tests\Host\NTP-Servers.Vester.ps1: line 26 at <ScriptBlock>, C:\Program Files\WindowsPowerShell\Modules\Vester\1.2.0\ Tests\Host\NTP-Servers.Vester.ps1: line 25 at <ScriptBlock>, C:\Program Files\WindowsPowerShell\Modules\Vester\1.2.0\ Private\Template\VesterTemplate.Tests.ps1: line 112

Context

Your Environment

doogleit commented 6 years ago

I thought I could take a stab at this. IMHO, this is really an inconsistency/bug in the vSphere Web Client. On a default ESXi installation without any NTP servers configured the Get-VMHostNTPServer cmdlet should return 'null'. If you use any of the other management tools (the old vSphere .Net/thick client, the new HTML 5 client, or Remove-VMHostNTPServer) to remove the configured NTP servers it works as expected and returns 'null'. However, if you use the web client to remove the NTP servers it leaves a single server entry that is blank in the NTP configuration. If you check the ntp.conf file on the ESXi host you can see a server entry without a hostname/IP address following it:

[root@esx001:~] cat /etc/ntp.conf restrict default kod nomodify notrap nopeer noquery restrict 127.0.0.1 server <-- This line should not be here wihtout specifying a hostname/IP after 'server' driftfile /etc/ntp.drift

This causes Get-VMHostNTPServer to return an empty string instead of null and Remove-VMHostNTPServer throws an error on an empty string. My initial thought was to just check for this condition and not attempt to remove the blank value. However, Add-VMHostNTPServer appends the desired NTP server(s) to the already existing empty value.

For example, if the desired value is:

ntpserver1, ntpserver2

The actual configuration after remediation has a preceding empty value in the list like this:

, ntpserver1, ntpserver2

Subsequent tests will fail since the values don't match. As far as I can tell there's no way to remove the blank server value using Remove-VMHostNTPServer.

With all that said, I had started testing a fix for this before I narrowed the problem down to the web client. The alternative method I have for remediation uses Get-View to get a reference to the Host's 'DateTimeSystem' and update the NTP config. I can submit a PR, but wanted to post a more detailed explanation here for discussion. Since this appears to be fixed in the HTML 5 client, I wasn't completely decided on whether or not this is an issue that Vester needs to address.

brianbunke commented 6 years ago

Awesome sleuthing 👏 TBH, I have no opinion one way or the other.

@h0bbel, does it sound like that matches your scenario?

Any other opinions on "Should the NTP Servers test compensate for a web client bug, acknowledging that the web client is (slooowly) being phased out?"

h0bbel commented 6 years ago

@doogleit Impressive, I didn't check if this was client specific, but it seems you might have hit the nail on the head there. While I agree that this limits the potential problems, I still think it should be fixed somehow. The Web Client is dying, but that doesn't mean everything will be on the new version any time soon — there is a really long tail there...

So, IMO, Vester should account for this even if it's a problem with the Web Client.

doogleit commented 6 years ago

I was thinking the same, but could have been swayed either way. I'll go ahead and create a pull request.