800409 - Fix always changing puppet resources on AWS machines

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task)

Description

[Rail Aliiev [:rail]]

These are:

* cpuspped shouldn't be touched
 /etc/init.d/cpuspeed status
 Frequency scaling not supported under xen kernels

* /root/.ssh/authorized_keys: rc.local updates that file, should be disabled

Comment 1

[Rail Aliiev [:rail]]

Attachment: fix v1

I don't like hostname matching here... v2 incoming

Comment 2

[Rail Aliiev [:rail]]

Attachment: fix v2

2 concerns:

1) cpuspeed may be running when we puppetize a new (or reimaged) slave for the first time.
2) not sure how service resource works with not existing services (rc.local is AWS specific).

Comment 3

[Rail Aliiev [:rail]]

And facter thinks that the AWS machines are "physical".

Comment 4

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

Comment on attachment 670433 [details] [diff] [review]
fix v2

rc.local seems fine in my testing.

Arguably cpuspeed is broken if it exits with status '0' in that case.  If it's not supported, it's status is stopped, or something nonzero anyway.  And cpuspeed doesn't "run" at all - it's just a startup script to set governors, and it doesn't do anything on AWS.  So nothing to worry about there.

So, I don't think there's any cleaner solution, and this really is perfectly fine.

Comment 5

[Rail Aliiev [:rail]]

Comment on attachment 670433 [details] [diff] [review]
fix v2

http://hg.mozilla.org/build/puppet/rev/641daa70ae77

Comment 6

[Rail Aliiev [:rail]]

Seen a couple of unchanged slaves. \o/

Comment 7

[Rail Aliiev [:rail]]

It turned out that disabling rc.local is not enough because the script has no idea about the "stop" action (the script is still called by /etc/rc.d/rc).

Comment 8

[Rail Aliiev [:rail]]

Attachment: fix #2

Comment 9

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

Comment on attachment 671250 [details] [diff] [review]
fix #2

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

r+ with that adjustment

::: modules/disableservices/manifests/common.pp
@@ +17,5 @@
> +            }
> +            exec {
> +                "disable-rc.local":
> +                    command => "/sbin/chkconfig --del rc.local",
> +                    onlyif  => "/bin/ls /etc/rc.d/rc*.d/*rc.local > /dev/null 2>&1";

Probably better to use "test -f $pathname" here - it has no output and gets you the exit status you want.

Comment 10

[Rail Aliiev [:rail]]

> Probably better to use "test -f $pathname" here - it has no output and gets
> you the exit status you want.

That was my first thought. In this case I would need very ugly command line to test all entries:

$ ls -1 /etc/rc.d/rc*.d/*rc.local
/etc/rc.d/rc0.d/K50rc.local
/etc/rc.d/rc1.d/K50rc.local
/etc/rc.d/rc2.d/K50rc.local
/etc/rc.d/rc3.d/K50rc.local
/etc/rc.d/rc4.d/K50rc.local
/etc/rc.d/rc5.d/K50rc.local
/etc/rc.d/rc6.d/K50rc.local

K50 here may change in the future for other platforms, this is why I didn't want to harcode it.

Comment 11

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

Ah, I didn't realize the glob was matching multiple files.  Ship it as-is, then!

Comment 12

[Rail Aliiev [:rail]]

Comment on attachment 671250 [details] [diff] [review]
fix #2

http://hg.mozilla.org/build/puppet/rev/f20e75b6d148

Comment 13

[Rail Aliiev [:rail]]

This should be fixed now.

Comment 14

[Rail Aliiev [:rail]]

For some reason root's authorized keys file is still changing...

Comment 15

[Rail Aliiev [:rail]]

Looks like chkconfig --del doesn't remove everything... On some machines I can find {K,S}*rc.local file in /etc/rc{0..6}.d/ directories. Sounds like we need some brute force here. :/

Comment 16

[Chris AtLee [:catlee]]

Not sure that killing rc.local is the best idea here...How about we modify rc.local to not update authorized_keys if the key being fetched by the EC2 API is already present?

Comment 17

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

Could we just add whatever key it's adding, but using puppet?  So the two agree on the correct file content?

Comment 18

[Rail Aliiev [:rail]]

Attachment: fix #3

Let's manage it! It does nothing for now.

Comment 19

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

Comment on attachment 677475 [details] [diff] [review]
fix #3

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

Looks good with that small tweak.

::: modules/tweaks/manifests/rc_local.pp
@@ +1,4 @@
> +class tweaks::rc_local {
> +
> +    case $kernel {
> +        Linux: {

This should probably be our usual $operatingsystem / CentOS combination (in case we use ubuntu or something).

Comment 20

[Rail Aliiev [:rail]]

(In reply to Dustin J. Mitchell [:dustin] from comment #19)
> This should probably be our usual $operatingsystem / CentOS combination (in
> case we use ubuntu or something).

Makes sense. I thought about CentOS+Fedora, but probably it's safe to start with CentOS only.

Comment 21

[Rail Aliiev [:rail]]

Comment on attachment 677475 [details] [diff] [review]
fix #3

pushed with a wrong commit message :/
http://hg.mozilla.org/build/puppet/rev/b1fa9c243677

Comment 22

[Rail Aliiev [:rail]]

A lot of ec2 slaves are in unchanged state now.

This is fixed for reals now! Phew...