Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix `govuk_node_list --with-puppet-class` to show all the hosts #10049

Merged
merged 2 commits into from Jan 14, 2020

Conversation

@issyl0
Copy link
Member

issyl0 commented Jan 14, 2020

  • We were doing on-call out of hours reboots this morning and noticed
    that on the AWS side none of the AWS hostnames made any sense. "It
    would be nice if the reboot check could show what class of machine
    that is". This would avoid us having to login to the AWS Console,
    search the EC2 section for the hostname and then check against the
    reboots doc.
  • Turns out this will already work, but only if the
    govuk_node_list command it relies on works properly itself.
  • Before, the --with-puppet-class function wasn't iterating through
    the instances properly as the return statement was returning the
    first instance it found (in all my testing, either mongo-1 or
    whitehall-backend-1). This fixes it so that rather than:
$ govuk_node_list --with-puppet-class
ip-10-1-6-48.eu-west-1.compute.internal:whitehall-backend-1
$

it returns all the values:

$ govuk_node_list --with-puppet-class
[...]
ip-10-1-6-231.eu-west-1.compute.internal:content-store-1
ip-10-1-6-45.eu-west-1.compute.internal:draft-cache-1
ip-10-1-6-185.eu-west-1.compute.internal:cache-1
ip-10-1-6-4.eu-west-1.compute.internal:rabbitmq
ip-10-1-6-48.eu-west-1.compute.internal:whitehall-backend-1
$

https://trello.com/c/dw9iaOBM/1676-improve-reboot-required-icinga-check-wording

- We were doing on-call out of hours reboots this morning and noticed
  that on the AWS side none of the AWS hostnames made any sense. "It
  would be nice if the reboot check could show what class of machine
  that is". This would avoid us having to login to the AWS Console,
  search the EC2 section for the hostname and _then_ check against the
  reboots doc.
- Turns out this will already work, but only if the
  `govuk_node_list` command it relies on works properly itself.
- Before, the `--with-puppet-class` function wasn't iterating through
  the instances properly as the `return` statement was returning the
  first instance it found (in all my testing, either `mongo-1` or
  `whitehall-backend-1`). This fixes it so that rather than:

```
$ govuk_node_list --with-puppet-class
ip-10-1-6-48.eu-west-1.compute.internal:whitehall-backend-1
$
```

  it returns all the values:

```
$ govuk_node_list --with-puppet-class
[...]
ip-10-1-6-231.eu-west-1.compute.internal:content-store-1
ip-10-1-6-45.eu-west-1.compute.internal:draft-cache-1
ip-10-1-6-185.eu-west-1.compute.internal:cache-1
ip-10-1-6-4.eu-west-1.compute.internal:rabbitmq
ip-10-1-6-48.eu-west-1.compute.internal:whitehall-backend-1
$
```

https://trello.com/c/dw9iaOBM/1676-improve-reboot-required-icinga-check-wording
Copy link
Contributor

ChrisBAshton left a comment

Nice 👏

Copy link
Contributor

thomasleese left a comment

I realise this code is the same as before, but I thought I would review it anyway.

Co-Authored-By: Thomas Leese <thomas.leese@digital.cabinet-office.gov.uk>
@issyl0 issyl0 force-pushed the fix-govuk_node_list-with-puppet-class-option branch from 9447021 to 0650ca2 Jan 14, 2020
@issyl0

This comment has been minimized.

Copy link
Member Author

issyl0 commented Jan 14, 2020

Thanks for the refactor, @thomasleese! I've tested, it all works fine, and pushed the changes in a separate commit.

@issyl0 issyl0 merged commit 76fb409 into master Jan 14, 2020
1 check passed
1 check passed
continuous-integration/jenkins/branch This commit looks good
Details
@issyl0 issyl0 deleted the fix-govuk_node_list-with-puppet-class-option branch Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.