Skip to content

Add check for cpu-balance on RDS#218

Open
ghost wants to merge 1 commit intomasterfrom
unknown repository
Open

Add check for cpu-balance on RDS#218
ghost wants to merge 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jul 4, 2017

  • Based on script check-ec2-cpu_balance.rb

Pull Request Checklist

Is this in reference to an existing issue?
No

General

  • Update Changelog following the conventions laid out on Keep A Changelog

  • Update README with any necessary configuration snippets

  • Binstubs are created if needed

  • RuboCop passes

  • Existing tests pass

New Plugins

  • Tests

  • Add the plugin to the README

  • Does it have a complete header as outlined here

Purpose

Check cpu-balance for RDS instance

Known Compatibility Issues

* Based on script check-ec2-cpu_balance.rb
Copy link
Copy Markdown
Member

@majormoses majormoses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit more future proofing and need a test artifact (ideally a unit test). If not a unit test I need a gist or something similar to show what you are passing via cli and its output to ensure that it does work.

messages = "\n"
level = 0
instances.db_instances.each do |db_instance|
next unless db_instance.db_instance_class.start_with? 'db.t2.'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like this broken out as a param that we can check against a regex. My motivation is to not have to rewrite the plugin when t3's come out. It should ideally take a comma separated list and generate an array of regexes to do. Minimally a single regex exposed seems like the min requirement to not make this throw away when amazon bumps families. This unblocks user when it does break without waiting for a hotfix and release.

@majormoses
Copy link
Copy Markdown
Member

Just checking if you had plans to come back to this

@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 12, 2017

@majormoses
Sorry for my late response.
I need to take time for this, maybe during this month.

@majormoses
Copy link
Copy Markdown
Member

majormoses commented Dec 12, 2017 via email

@majormoses
Copy link
Copy Markdown
Member

@julio-ogury another friendly reminder

@majormoses
Copy link
Copy Markdown
Member

Given that the user no longer exists I will merge this and we can refactor it later.

@majormoses
Copy link
Copy Markdown
Member

I will have to come back to this one later since I am going to have to pull locally to fix conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant