Closed Bug 1040584 Opened 10 years ago Closed 10 years ago

Bare Metal Provisioning 2008 Puppet: Custom Windows MSI Provider

Categories

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

x86_64
Windows 7
task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: markco, Assigned: markco)

Details

Attachments

(1 file, 1 obsolete file)

      No description provided.
Assignee: relops → mcornmesser
Summary: Bare Metal Provisioning 2008 Puppet: Windows MSI Provider → Bare Metal Provisioning 2008 Puppet: Custom Windows MSI Provider
Attached patch 1040584.patch (obsolete) — Splinter Review
Attachment #8461064 - Flags: review?(dustin)
Comment on attachment 8461064 [details] [diff] [review]
1040584.patch

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

This looks functional (aside from some minor stuff below).  However, the File resource is going to do an md5sum of every MSI on both the agent and the server, on every puppet run, and that can take a long time.

Can you try adding checksum => 'sha256lite' to the File resource?
  http://docs.puppetlabs.com/references/latest/type.html#file-attribute-checksum

That should be faster, as it just does a sha256 of the first 512 bytes:
  https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/checksums.rb#L47
It's not perfect, as it might miss a truncated file, but it's better than pounding the puppetmasters.

r+ with those changes, assuming they all test out correctly.

::: modules/dirs/manifests/installersource/puppetagain_pub_build_mozilla_org.pp
@@ +3,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +class dirs::installersource::puppetagain_pub_build_mozilla_org {
> +    include dirs::installersource
> +    file {
> +        "$env_systemdrive\installersource\puppetagain.pub.build.mozilla.org " :

The trailing space here will probably cause a problem.  Also, backslashes.

::: modules/dirs/manifests/installersource/puppetagain_pub_build_mozilla_org/msis.pp
@@ +3,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +class dirs::installersource::puppetagain_pub_build_mozilla_org::msis {
> +    include dirs::installersource
> +    file {
> +        "$env_systemdrive/installersource/puppetagain.pub.build.mozilla.org/MSIs " :

Whitespace here, too

::: modules/packages/manifests/pkgmsi.pp
@@ +11,5 @@
> +        false => ""
> +    }
> +    file {
> +            "$installersource_posix/MSIs/$msi":
> +                source  => "puppet:///repos/MSIs/$msi",

I think this needs $p in the path: "puppet:///repos$p/MSIs/$msi".  The $p is either empty (no change) or points to /repos/private/MSIs, where the private MSIs are stored.
Attachment #8461064 - Flags: review?(dustin) → review+
sha256lite appears to be not valid. From the log: 


Wrapped exception:
Invalid value "sha256lite". Valid values are md5, md5lite, mtime, ctime, none. [0m

Which one do you think we should use?
Ah yes, that landed in 3.6.2:
  https://github.com/puppetlabs/puppet/commit/fcfb1f299f24ce9ac898648cd8e1ee03bdfbd8fe

sha1lite should be OK, or md5lite.  I just know opsec dies a little every time someone uses md5!
Most recent error for check sum: 

c:/InstallerSource/puppetagain.pub.build.mozilla.org/MSIs/UltraVnc_10962_x64.msi]: Copying group from the source file on Windows is deprecated; use source_permissions => ignore.[0m
[1;31mError: File written to disk did not match checksum; discarding changes ({md5lite}e7bf157e1bba889eee8711a7fd1dfbf7 vs {md5lite}313772784558c552dbda1a58021ab846)[0m
[1;31mError: /Stage[main]/Packages::Ultravnc/Packages::Pkgmsi[UltraVNC]/File
Ugh, so one is the correct sum for the first 512 bytes (which is what md5lite does):

[root@releng-puppet1.srv.releng.scl3.mozilla.com MSIs]# head -c 512 UltraVnc_10962_x64.msi | md5sum
313772784558c552dbda1a58021ab846  -

but the other hash is the md5sum of the whole file:

[root@releng-puppet1.srv.releng.scl3.mozilla.com MSIs]# md5sum UltraVnc_10962_x64.msi
e7bf157e1bba889eee8711a7fd1dfbf7  UltraVnc_10962_x64.msi

So it looks like puppet's checksums are STILL broken.
(so, drop the checksum option)
Attached patch 1040584.patchSplinter Review
Attachment #8461064 - Attachment is obsolete: true
Attachment #8462594 - Flags: review?(dustin)
Comment on attachment 8462594 [details] [diff] [review]
1040584.patch

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

Looks perfect!

I'll see if I can get a bug on file for the puppet checksum problem.
Attachment #8462594 - Flags: review?(dustin) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: