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

Add back an instance_types rake task #707

Merged
merged 1 commit into from
May 28, 2021

Conversation

agrare
Copy link
Member

@agrare agrare commented May 28, 2021

There used to be a rake task to auto-generate the instance_types.rb (at least as early as #170)

It was removed in #449 with the plan of pulling this info from the AWS API/SDK however that never materialized.

This manifested in provisioning failing because the vpc_only attribute was flat out incorrect for a number of flavors which caused the auto-select logic to fail.

@agrare agrare requested a review from Fryguy as a code owner May 28, 2021 14:29
@agrare agrare added the wip label May 28, 2021
:memory: 17179869184
:memory_gb: 16.0
:name: a1.2xlarge
:network_performance: :up_to_10_gigabit
:network_performance: :very_high
Copy link
Member Author

Choose a reason for hiding this comment

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

This was a legit bug in the current yaml, the network performance string is "Up To 10 Gigabit" which didn't match our exact "10 Gigabit" or "20 Gigabit" check

@agrare agrare force-pushed the add_instance_types_rake_task branch 3 times, most recently from de5b1a1 to 34e3729 Compare May 28, 2021 16:11
@Fryguy Fryguy self-assigned this May 28, 2021
@Fryguy
Copy link
Member

Fryguy commented May 28, 2021

Overall LGTM - glad the script is relatively tiny.

@agrare agrare force-pushed the add_instance_types_rake_task branch 2 times, most recently from 83a2d44 to 7ff2680 Compare May 28, 2021 18:16
@agrare
Copy link
Member Author

agrare commented May 28, 2021

For now I've limited this to just the existing flavors so that the diff cleaner and it is more obvious what is changed by specifically this generator. I will follow-up with the new flavors that have been added since this was last run in a separate PR.

@agrare agrare changed the title [WIP] Add back an instance_types rake task Add back an instance_types rake task May 28, 2021
@agrare agrare added the bug label May 28, 2021
@miq-bot miq-bot removed the wip label May 28, 2021
@agrare agrare force-pushed the add_instance_types_rake_task branch from 7ff2680 to 5605751 Compare May 28, 2021 18:34
@miq-bot
Copy link
Member

miq-bot commented May 28, 2021

Checked commit agrare@5605751 with ruby 2.6.3, rubocop 1.13.0, haml-lint 0.35.0, and yamllint
1 file checked, 1 offense detected

lib/tasks_private/instance_types.rake

  • ❗ - Line 3, Col 3 - Rails/RakeEnvironment - Include :environment task as a dependency for all Rake tasks.

@Fryguy Fryguy merged commit 216036c into ManageIQ:master May 28, 2021
@agrare agrare deleted the add_instance_types_rake_task branch May 28, 2021 18:57
@Fryguy
Copy link
Member

Fryguy commented Jun 2, 2021

Backported to lasker in commit eae927b.

commit eae927bee2e0954f5f82f22283708ac6ac77f22d
Author: Jason Frey <fryguy9@gmail.com>
Date:   Fri May 28 14:57:21 2021 -0400

    Merge pull request #707 from agrare/add_instance_types_rake_task
    
    Add back an instance_types rake task
    
    (cherry picked from commit 216036c6845f5553a681cf8929a4abae68130528)

Fryguy added a commit that referenced this pull request Jun 2, 2021
Add back an instance_types rake task

(cherry picked from commit 216036c)
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.

3 participants