Skip to content
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

Fixes #54550 #56279

Merged
merged 10 commits into from Jun 11, 2019
@@ -307,7 +307,7 @@ if($gather_subset.Contains('memory')) {
if($gather_subset.Contains('platform')) {
$win32_cs = Get-LazyCimInstance Win32_ComputerSystem
$win32_os = Get-LazyCimInstance Win32_OperatingSystem
$ip_props = [System.Net.NetworkInformation.IPGlobalProperties]::GetIPGlobalProperties()
#$ip_props = [System.Net.NetworkInformation.IPGlobalProperties]::GetIPGlobalProperties()
This conversation was marked as resolved by ShachafGoldstein

This comment has been minimized.

Copy link
@jborean93

jborean93 May 28, 2019

Contributor

No use keeping this commented, we have git history if we need to see what was originally there.


try {
$ansible_reboot_pending = Get-PendingRebootStatus
@@ -319,10 +319,11 @@ if($gather_subset.Contains('platform')) {
$ansible_facts += @{
ansible_architecture = $win32_os.OSArchitecture
ansible_domain = $ip_props.DomainName
This conversation was marked as resolved by ShachafGoldstein

This comment has been minimized.

Copy link
@dagwieers

dagwieers May 15, 2019

Member

$ip_props is still used here.

This comment has been minimized.

Copy link
@ShachafGoldstein

ShachafGoldstein May 15, 2019

Author Contributor

Fixed

ansible_fqdn = ($ip_props.Hostname + "." + $ip_props.DomainName)
ansible_hostname = [System.Net.Dns]::GetHostName()
ansible_fqdn = ($win32_cs.DNSHostname + "." + $win32_cs.Domain.Substring($win32_cs.Workgroup.length))
This conversation was marked as resolved by ShachafGoldstein

This comment has been minimized.

Copy link
@jborean93

jborean93 May 28, 2019

Contributor

This still has the same issue as before where a non-domain host would be HOSTNAME. (with the ending .). We should conditionally add the domain part with . to the FQDN if it's not an empty string. Also the same thing here about the Substring for the domain name.

This comment has been minimized.

Copy link
@ShachafGoldstein

ShachafGoldstein May 28, 2019

Author Contributor

Kept the dot or backward compatibility, so you wish to remove it if not needed?

This comment has been minimized.

Copy link
@ShachafGoldstein

ShachafGoldstein May 28, 2019

Author Contributor

Last commit removes it

ansible_hostname = $win32_cs.DNSHostname
ansible_netbios_name = $win32_cs.Name
ansible_kernel = $osversion.Version.ToString()
ansible_nodename = ($ip_props.HostName + "." + $ip_props.DomainName)
ansible_nodename = ($win32_cs.DNSHostname + "." + $win32_cs.Domain.Substring($win32_cs.Workgroup.length))
This conversation was marked as resolved by ShachafGoldstein

This comment has been minimized.

Copy link
@jborean93

jborean93 May 28, 2019

Contributor

If we are reusing values like before it could be beneficial to store that in a var and just set the var here instead of calculating it twice.

This comment has been minimized.

Copy link
@ShachafGoldstein

ShachafGoldstein May 28, 2019

Author Contributor

Changed

ansible_machine_id = Get-MachineSid
ansible_owner_contact = ([string] $win32_cs.PrimaryOwnerContact)
ansible_owner_name = ([string] $win32_cs.PrimaryOwnerName)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.