1346207 - vmware-tools failed to install in Ubuntu 16.04

Status: RESOLVED FIXED

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

Description

[Dragos Crisan [:dragrom]]

During test the puppet on Ubuntu 16.04, I received the following error when i tried to install vmware:

open-vm-tools are available from the OS vendor and VMware recommends using 
open-vm-tools. See http://kb.vmware.com/kb/2073803 for more information.
Do you still want to proceed with this legacy installer? [no]

for the moment unlocked me by adding a -f switch to the command: 
/vmware-tools-distrib/vmware-install.pl -d into file: modules/vmwaretools/manifests/install/exec.pp

Comment 1

[Dragos Crisan [:dragrom]]

Attachment: puppetagain_vmware.patch

Comment 2

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

Comment on attachment 8847644 [details] [diff] [review]
puppetagain_vmware.patch

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

Was there a rationale for not following the suggestion in the error message (using open-vm-tools)?

Comment 3

[Dragos Crisan [:dragrom]]

I don't know if the tools that use vmware-tool note require that specific version of vmware-tool. This was the reason for what I forced to install existed version.

Comment 4

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

OK - we can leave it to the vmware team to give us more direction.

Comment 5

[Kendall Libby [:fubar] (he/him)]

CC on sanvm folks for their input

Comment 6

[Greg Cox [:gcox]]

[unofficial editorial: the VMware tools, while deprecating, are more informative than the open source ones, so I like them better when they're in support, but that's a slowly-dying opinion]

When you install open-vm-tools, you shouldn't be even installing the (losing-support-as-time-goes-on-for-newer-OSes) package from VMware.  That way lies sadness.

I'd f- that patch, and suggest a rewrite that's basically 'if CentOS >= 7 and Ubuntu >= 16 use open-vm-tools, else do what you're doing now'

Comment 7

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

Comment on attachment 8847644 [details] [diff] [review]
puppetagain_vmware.patch

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

(based on feedback)

Comment 8

[Dragos Crisan [:dragrom]]

The open-vm-tools package is in xenial-update distribution. I found, for the precise, the precise-update was added into releng-updates repository. So, for the open-vm-tools I'll need to add xenial-updates into releng-updates repository and copy the opent-vm-tools into pool. Is this the correct approach?

Comment 9

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

I think so, assuming that it doesn't require mirroring many GB of additional packages onto the puppet masters.

Comment 10

[Jake Watkins [:dividehex]]

(In reply to Dragos Crisan [:dragrom] from comment #8)
> The open-vm-tools package is in xenial-update distribution. I found, for the
> precise, the precise-update was added into releng-updates repository. So,
> for the open-vm-tools I'll need to add xenial-updates into releng-updates
> repository and copy the opent-vm-tools into pool. Is this the correct
> approach?


I like :gcox's feedback here...
If open-vm-tools is already available in the xenial-update repo, then by all means we should use that for xenial and leave precise and trusty alone.  We should not be touching the releng-updates repo, as those repos is "frozen" and should never be updated.  In fact, we are completely abandoning the releng and releng-updates repos in xenial (unless a really good reason comes up)

https://dxr.mozilla.org/build-central/source/puppet/modules/packages/manifests/setup.pp#230

:dragrom, I would suggest adding a open-vm-tools.pp to modules/packages/manifests that installs open-vm-tools (for 16.04 only) and add a case statement for 16.04 vs 12.04,14.04 under the hardware module.

https://dxr.mozilla.org/build-central/source/puppet/modules/hardware/manifests/init.pp#38

Following the same logic :gcox suggested,  if 12.04,14.04, use the class vmwaretools else if 16.04, use class packages::open-vm-tools.

Comment 11

[Dragos Crisan [:dragrom]]

Attachment: puppetagain_openvmtools.patch

Comment 12

[Greg Cox [:gcox]]

Comment on attachment 8851997 [details] [diff] [review]
puppetagain_openvmtools.patch

Unsolicited outsider feedback: the softest of f-, but easily overruled by someone closer to the project.  I don't see a good reason to version-pin open-vm-tools to a specific ubuntu package version.

* Our (VM team's) concern is mostly that you have tools installed; the specific version is not important to us once you're on open-vm-tools; the VM guest manages the version.  'present' or 'latest' is maybe fine / less work than pinning?

* If nothing else, your open_vm_tools.pp is now ubuntu-specific without declaring that it's for ubuntu within that file.  (You only use it for ubuntu now, but as more flavors of linux come out, open_vm_tools will be more prevalent; may as well put in the skeleton framework for CentOS-future or multiple-ubuntu-future)

Comment 13

[Jake Watkins [:dividehex]]

Comment on attachment 8851997 [details] [diff] [review]
puppetagain_openvmtools.patch

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

r+. Just remove trailing whitespace

::: modules/packages/manifests/open_vm_tools.pp
@@ +2,5 @@
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +class packages::open_vm_tools {
> +
> +    package { 

Line 6 has a trailing whitespace

Comment 14

[Jake Watkins [:dividehex]]

Comment on attachment 8851997 [details] [diff] [review]
puppetagain_openvmtools.patch

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

Some how I jumped the gun on the r+. All package classes need an OS case switch and this is missing that completely.  You should case for Ubuntu and 16.04, then explicitly fail () for default.

::: modules/packages/manifests/open_vm_tools.pp
@@ +5,5 @@
> +
> +    package { 
> +        'open-vm-tools':
> +            ensure => '2:10.0.7-3227872-5ubuntu1~16.04.1'
> +    }

I missed the fact that there is no OS case switch and default fail here.  This should always be the case for install packages.

Comment 15

[Jake Watkins [:dividehex]]

(In reply to Greg Cox [:gcox] from comment #12)
> Comment on attachment 8851997 [details] [diff] [review]
> puppetagain_openvmtools.patch
> 
> Unsolicited outsider feedback: the softest of f-, but easily overruled by
> someone closer to the project.  I don't see a good reason to version-pin
> open-vm-tools to a specific ubuntu package version.
> 
> * Our (VM team's) concern is mostly that you have tools installed; the
> specific version is not important to us once you're on open-vm-tools; the VM
> guest manages the version.  'present' or 'latest' is maybe fine / less work
> than pinning?

That does make sense, but in releng puppet, we prefer the practice of explicitly pinning package versions since it has bitten us hard in the past.  Repo changes should not be the trigger for system changes and ensures extra attention and a record of change in the vcs log when it comes to upgrading packages.  This is also especially important when it come to hosts running talos tests as slight changes in system packages can change test baselines. (Although I will concede, we don't intend to run talos on vmware.)

> * If nothing else, your open_vm_tools.pp is now ubuntu-specific without
> declaring that it's for ubuntu within that file.  (You only use it for
> ubuntu now, but as more flavors of linux come out, open_vm_tools will be
> more prevalent; may as well put in the skeleton framework for CentOS-future
> or multiple-ubuntu-future)

Good catch!  I've added notes to my previous review.  Package declarations must be explicit in which OS/Version it is intended for and must explicitly fail otherwise.

Comment 16

[Dragos Crisan [:dragrom]]

Attachment: puppetagain_openvmtools.patch

Comment 17

[Jake Watkins [:dividehex]]

Comment on attachment 8852364 [details] [diff] [review]
puppetagain_openvmtools.patch

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

Looks good

Comment 18

[Jake Watkins [:dividehex]]

Comment on attachment 8852364 [details] [diff] [review]
puppetagain_openvmtools.patch

https://hg.mozilla.org/build/puppet/rev/c652860a19c3f1e4554d2326a7a5e75d9039805f
https://hg.mozilla.org/build/puppet/rev/00446b4d1edca60c829718647d2693886ee2bc5b