759488 - point non-puppetagain slaves toward local python mirrors

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task)

Description

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

This is basically bug 727283, but done for old puppet and opsi.

It's probably best for it to just look like

[global]
no-index = true
find-links = http://repos/python/packages

rather than trying to get the redundancy we have in puppetagain.

Comment 1

[Chris Cooper [:coop] (he/him)]

(In reply to Dustin J. Mitchell [:dustin] from comment #0)
> This is basically bug 727283, but done for old puppet and opsi.

I don't think that's the bug you meant to link.

Comment 2

[Chris Cooper [:coop] (he/him)]

(In reply to Dustin J. Mitchell [:dustin] from comment #0)
> It's probably best for it to just look like
> 
> [global]
> no-index = true
> find-links = http://repos/python/packages
> 
> rather than trying to get the redundancy we have in puppetagain.

kmoir: is this something you could take on?

Comment 3

[Kim Moir [:kmoir] ET]

I couldn't find the puppetAgain bug to look at for reference.  I assume this is just a change in the modules/python/manifests/virtualenv.pp in puppet-manifests?

Comment 4

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

Sorry, the bug is bug 757283.  It will be a change to add ~cltbld/.pip/pip.conf with the contents in comment 0.

Sorry for the bug number typo!

Comment 5

[Kim Moir [:kmoir] ET]

Attachment: patch

The old puppet doesn't inherit the user characteristics from a central module etc so this just states them explicitly in the module.  I didn't think it was worth refactoring since it's the old version.

Comment 6

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

Comment on attachment 638406 [details] [diff] [review]
patch

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

Looks good to me.  I've added r? rail since I haven't been following build/puppet-manifests for a long time, but that can likely be a simple "lgtm" review.

Comment 7

[Rail Aliiev [:rail]]

Comment on attachment 638406 [details] [diff] [review]
patch

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

lgtm with some nits:

::: modules/python/manifests/pip.pp
@@ +6,2 @@
>                      creates => "/usr/local/bin/pip",
> +                    command => "/usr/bin/easy_install pip" ;

extra space before ";"  and ":" looks inconsistent with other manifests.

::: modules/python/manifests/user_pip_conf.pp
@@ +2,5 @@
> +    $user = $title
> +    $homedir_ = $operatingsystem ? {
> +        CentOS => "/home/$user",
> +        Darwin => "/Users/$user",
> +        default => $homedir,

Do you really need $homedir as a parameter? It can't be used on CentOS and Darwin from what I see here.

@@ +5,5 @@
> +        Darwin => "/Users/$user",
> +        default => $homedir,
> +    }
> +    $group = $operatingsystem ? {
> +        CentOS => "$user",

The line above can be dropped since it matches "default".

@@ +22,5 @@
> +            owner => $user,
> +            group => $group,
> +            mode => 0644 ;
> +    }
> +}
\ No newline at end of file

Can you add a newline at the end of file?
The same regarding spaces.

Comment 8

[Kim Moir [:kmoir] ET]

Attachment: updated patch

Comment 9

[Kim Moir [:kmoir] ET]

Closing.