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.

Sign up for GitHub

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

Jump to bottom

Add check for cpu-balance on RDS #218

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add check for cpu-balance on RDS #218

wants to merge 1 commit into from

Conversation

ghost
Copy link

@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

Add check for cpu-balance on RDS
06bb7e8 
* Based on script check-ec2-cpu_balance.rb
majormoses
majormoses requested changes Jul 4, 2017
View reviewed changes
Copy link
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.

bin/check-rds-cpu_balance.rb
messages = "\n"
level = 0
instances.db_instances.each do |db_instance|
next unless db_instance.db_instance_class.start_with? 'db.t2.'
Copy link
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
Member

Just checking if you had plans to come back to this

@ghost
Copy link
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
Member

majormoses commented Dec 12, 2017 via email

@majormoses
Copy link
Member

@julio-ogury another friendly reminder

@majormoses
Copy link
Member

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

@majormoses
Copy link
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
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Awaiting Response Type: Enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@majormoses