1247708 - WIn 7 add support to toplevel::slave::releng

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: Puppet, task)

Comment 1

[Mark Cornmesser [:markco] OOO 2024/04/15]

Attachment: BUG1247708.patch

A couple of notes.

Created a node definition for d-w732 , so that we can continue to add to puppet but not interfere with current production needs to manage secrets and keys.

In packages::mozilla::py27_mercurial the build up of variables was needed because Puppet doesn't like doubles quotes within double quotes (" "" ").

The includes classes that were moved from toplevel/manifests/slave/releng.pp to toplevel/manifests/slave/releng/build.pp were not applicable to Windows testers.

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8720039 [details] [diff] [review]
BUG1247708.patch

Review of attachment 8720039 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine other than the VNC change.

::: manifests/moco-nodes.pp
@@ +35,5 @@
>      $slave_trustlevel = 'try'
>      include toplevel::slave
>  }
> +# Node declaration is for Win 7 development 
> +# To keep development and production catalogs separate 

trailing whitespace

::: modules/packages/manifests/mozilla/py27_mercurial.pp
@@ +59,5 @@
> +            $merc_exe = $hardwaremodel ? {
> +                i686    => "Mercurial-3.2.1.exe",
> +                default => "Mercurial-3.2.1-x64.exe",
> +            }
> +            $merc_exe_dir = "C:\\installersource\\puppetagain.pub.build.mozilla.org\\EXEs\\"

Might as well use single quotes here and avoid the \\

@@ +61,5 @@
> +                default => "Mercurial-3.2.1-x64.exe",
> +            }
> +            $merc_exe_dir = "C:\\installersource\\puppetagain.pub.build.mozilla.org\\EXEs\\"
> +            $merc_exe_flag = " /SILENT /DIR=C:\\mozilla-build\\hg"
> +            $quoted_merc_cmd = ""$merc_exe_dir$merc_exe$merc_exe_flag""

I think what you want here is "\"$merc_exe_dir$merc_exe$merc_exe_flag\""

::: modules/toplevel/manifests/slave/releng.pp
@@ +33,4 @@
>          include fw::windows_exceptions
>          include fw::windows_settings
>          include hardware::ec2_config
> +        include vnc

I don't think we want to add this here -- not all releng slaves use VNC (notably not the linux builders)

::: modules/tweaks/manifests/windows_network_opt_netsh.pp
@@ +19,5 @@
> +    # This will be re-addressed with bug 1247707
> +    $QuotedLAC   = $env_os_version ? {
> +        2008 =>  '"Local Area Connection "',
> +        w732 =>  '"Local Area Connection 2"',
> +    } 

trailing space

Comment 3

[Mark Cornmesser [:markco] OOO 2024/04/15]

As far as vnc it was removed from a Windows only clause in build.pp to a Windows only clause in releng.pp. It is used on all Releng Windows. So I should remove it from releng.pp and put it into a Windows only clause in build.pp and test.pp?

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8720039 [details] [diff] [review]
BUG1247708.patch

Oh, I didn't realize that was a windows-only clause from the diff.

Comment 5

[Mark Cornmesser [:markco] OOO 2024/04/15]

Attachment: BUG1247708.patch

Removed white spaces. 

Added escapes in "\"$merc_exe_dir$merc_exe$merc_exe_flag\""

Left the double quotes in "C:\\installersource\\puppetagain.pub.build.mozilla.org\\EXEs\\" because of the need trailing \ in the variable value.

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8720470 [details] [diff] [review]
BUG1247708.patch

Ugh, trailing \..

Comment 7

[Mark Cornmesser [:markco] OOO 2024/04/15]

Comment on attachment 8720470 [details] [diff] [review]
BUG1247708.patch

Review of attachment 8720470 [details] [diff] [review]:
-----------------------------------------------------------------

https://hg.mozilla.org/build/puppet/rev/3cc1488ab4ff