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

Save ram and storage of a request in gb internally #342

Merged
merged 18 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -473,4 +473,4 @@ RUBY VERSION
ruby 2.5.0p0

BUNDLED WITH
1.16.2
1.17.1
4 changes: 1 addition & 3 deletions app/controllers/requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,12 @@ def prepare_params
return unless request_parameters

request_parameters[:name] &&= replace_whitespaces(request_parameters[:name])
request_parameters[:ram_mb] &&= gb_to_mb(request_parameters[:ram_mb].to_i)
request_parameters[:storage_mb] &&= gb_to_mb(request_parameters[:storage_mb].to_i)
request_parameters[:sudo_user_ids] &&= request_parameters[:sudo_user_ids][1..-1]
end

# Never trust parameters from the scary internet, only allow the white list through.
def request_params
params.require(:request).permit(:name, :cpu_cores, :ram_mb, :storage_mb, :operating_system,
params.require(:request).permit(:name, :cpu_cores, :ram_gb, :storage_gb, :operating_system,
:port, :application_name, :description, :comment,
:rejection_information, user_ids: [], sudo_user_ids: [])
end
Expand Down
8 changes: 0 additions & 8 deletions app/helpers/requests_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,4 @@ module RequestsHelper
def replace_whitespaces(name)
name.parameterize
end

def mb_to_gb(megabytes)
megabytes / 1000
end

def gb_to_mb(gigabytes)
gigabytes * 1000
end
end
20 changes: 14 additions & 6 deletions app/models/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@ class Request < ApplicationRecord

MAX_NAME_LENGTH = 20
MAX_CPU_CORES = 64
MAX_RAM_MB = 256_000
MAX_STORAGE_MB = 1_000_000
MAX_RAM_GB = 256
MAX_STORAGE_GB = 1_000

enum status: %i[pending accepted rejected]
validates :name,
length: { maximum: MAX_NAME_LENGTH, message: 'only allows a maximum of %{count} characters' },
format: { with: /\A[a-zA-Z1-9\-\s]+\z/, message: 'only letters and numbers allowed' },
uniqueness: true
validates :cpu_cores, :ram_mb, :storage_mb, :operating_system, :description, presence: true
validates :cpu_cores, :ram_gb, :storage_gb, :operating_system, :description, presence: true
validates :cpu_cores, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_CPU_CORES }
validates :ram_mb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_RAM_MB }
validates :storage_mb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_STORAGE_MB }
validates :ram_gb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_RAM_GB }
validates :storage_gb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_STORAGE_GB }

def description_text(host_name)
description = "- VM Name: #{name}\n"
Expand Down Expand Up @@ -69,7 +69,7 @@ def non_sudo_user_assignments
def create_vm
folder = VSphere::Connection.instance.root_folder
clusters = VSphere::Cluster.all
folder.create_vm(cpu_cores, ram_mb, storage_mb, name, clusters.first) if clusters.first
folder.create_vm(cpu_cores, gibi_to_mibi(ram_gb), gibi_to_kibi(storage_gb), name, clusters.first) if clusters.first
end

def push_to_git
Expand Down Expand Up @@ -114,4 +114,12 @@ def generate_puppet_name_script
def url(host_name)
Rails.application.routes.url_helpers.request_url self, host: host_name
end

def gibi_to_mibi(gibi)
gibi * 1024
Copy link
Contributor

Choose a reason for hiding this comment

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

magic number, please use 2**10

end

def gibi_to_kibi(gibi)
gibi * 1024**2
Copy link
Contributor

@Kenneth-Schroeder Kenneth-Schroeder Feb 1, 2019

Choose a reason for hiding this comment

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

please use 2** 10** 2

Copy link
Contributor

Choose a reason for hiding this comment

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

or (2**10) **2

Copy link
Contributor

Choose a reason for hiding this comment

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

or 2**20

Copy link
Contributor

Choose a reason for hiding this comment

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

or 2**(10*2)

Copy link
Contributor

Choose a reason for hiding this comment

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

or 2**(2**3+2) **2

Copy link
Contributor

Choose a reason for hiding this comment

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

or 2**(2**(2+2/2)+2) **2

Copy link
Contributor

Choose a reason for hiding this comment

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

or make a method "binary_base", returning 2 and change formula to
binary_base**(binary_base**(binary_base + binary_base/binary_base)+ binary_base) ** binary_base

Copy link
Contributor

Choose a reason for hiding this comment

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

or make a method "binary_base", returning 2 and change formula to
binary_base**(binary_base**(binary_base + binary_base/binary_base)+ binary_base) ** binary_base

that way we have no magic numbers

Copy link
Contributor

Choose a reason for hiding this comment

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

@BjoernDaase we should remove all magic numbers that way

Copy link
Contributor

Choose a reason for hiding this comment

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

@Kenneth-Schroeder Are you sober?

end
end
10 changes: 3 additions & 7 deletions app/models/request_template.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
# frozen_string_literal: true

class RequestTemplate < ApplicationRecord
MAX_CPU_CORES = 64
MAX_RAM_GB = 256
MAX_STORAGE_GB = 1_000

validates :name, :cpu_cores, :ram_gb, :storage_gb, :operating_system, presence: true
validates :cpu_cores, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_CPU_CORES }
validates :ram_gb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_RAM_GB }
validates :storage_gb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: MAX_STORAGE_GB }
validates :cpu_cores, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: Request::MAX_CPU_CORES }
validates :ram_gb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: Request::MAX_RAM_GB }
validates :storage_gb, numericality: { only_integer: true, greater_than: 0, less_than_or_equal_to: Request::MAX_STORAGE_GB }
end
4 changes: 2 additions & 2 deletions app/views/request_templates/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
</div>

<div class="field form-group">
<%= form.label :ram_mb, "RAM in GB" %>
<%= form.label :ram_gb, "RAM in GB" %>
<%= form.number_field :ram_gb, min: 0, class: 'form-control' %>
</div>

<div class="field form-group">
<%= form.label :storage_mb, "Storage in GB" %>
<%= form.label :storage_gb, "Storage in GB" %>
<%= form.number_field :storage_gb, min: 0, class: 'form-control' %>
</div>

Expand Down
8 changes: 4 additions & 4 deletions app/views/requests/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
</div>

<div class="field form-group">
<%= form.label :ram_mb, "RAM in GB" %>
<%= form.number_field :ram_mb, value: (@request && !@request&.ram_mb.nil?) ? mb_to_gb(@request.ram_mb) : nil, min: 0, class: 'form-control', id: 'ram' %>
<%= form.label :ram_gb, "RAM in GB" %>
<%= form.number_field :ram_gb, min: 0, class: 'form-control', id: 'ram' %>
</div>

<div class="field form-group">
<%= form.label :storage_mb, "Storage in GB" %>
<%= form.number_field :storage_mb, value: (@request && !@request&.storage_mb.nil?) ? mb_to_gb(@request.storage_mb) : nil, min: 0, class: 'form-control', id: 'storage' %>
<%= form.label :storage_gb, "Storage in GB" %>
<%= form.number_field :storage_gb, min: 0, class: 'form-control', id: 'storage' %>
</div>

<div class="field form-group">
Expand Down
2 changes: 1 addition & 1 deletion app/views/requests/_request.json.jbuilder
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# frozen_string_literal: true

json.extract! request, :id, :name, :cpu_cores, :ram_mb, :storage_mb, :operating_system, :comment, :rejection_information, :status, :created_at, :updated_at
json.extract! request, :id, :name, :cpu_cores, :ram_gb, :storage_gb, :operating_system, :comment, :rejection_information, :status, :created_at, :updated_at
json.url request_url(request, format: :json)
8 changes: 4 additions & 4 deletions app/views/requests/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
<%= link_to request.name, request_path(request), class: "btn btn-link" %>
</td>
<td class="table-active"><%= request.cpu_cores %></td>
<td class="table-active"><%= mb_to_gb(request.ram_mb) %> GB</td>
<td class="table-active"><%= mb_to_gb(request.storage_mb) %> GB</td>
<td class="table-active"><%= request.ram_gb %> GB</td>
<td class="table-active"><%= request.storage_gb %> GB</td>
<td class="table-active"><%= request.operating_system %></td>
<td class="table-active"><%= request.port %></td>
<td class="table-active"><%= request.application_name %></td>
Expand Down Expand Up @@ -71,8 +71,8 @@
<%= link_to request.name, request_path(request), class: "btn btn-link" %>
</td>
<td class="table-active"><%= request.cpu_cores %></td>
<td class="table-active"><%= mb_to_gb(request.ram_mb) %> GB</td>
<td class="table-active"><%= mb_to_gb(request.storage_mb) %> GB</td>
<td class="table-active"><%= request.ram_gb %> GB</td>
<td class="table-active"><%= request.storage_gb %> GB</td>
<td class="table-active"><%= request.operating_system %></td>
<td class="table-active"><%= request.port %></td>
<td class="table-active"><%= request.application_name %></td>
Expand Down
4 changes: 2 additions & 2 deletions app/views/requests/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@

<tr>
<td><strong>RAM in GB:</strong></td>
<td><%= mb_to_gb(@request.ram_mb) %></td>
<td><%= @request.ram_gb %></td>
</tr>

<tr>
<td><strong>Storage in GB:</strong></td>
<td><%= mb_to_gb(@request.storage_mb) %></td>
<td><%= @request.storage_gb %></td>
</tr>

<tr>
Expand Down
23 changes: 23 additions & 0 deletions db/migrate/20190131115425_rename_request_mb_to_gb.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

class RenameRequestMbToGb < ActiveRecord::Migration[5.2]
def up
update_requests
rename_column :requests, :ram_mb, :ram_gb
rename_column :requests, :storage_mb, :storage_gb
end

def update_requests
Request.all.each do |request|
unless request.nil?
request.ram_mb = request.ram_mb / 1000
request.storage_mb = request.storage_mb / 1000
end
request.save
end
end

def down
raise ActiveRecord::IrreversibleMigration
end
end
6 changes: 3 additions & 3 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2019_01_29_184200) do
ActiveRecord::Schema.define(version: 2019_01_31_115425) do

create_table "archivation_requests", force: :cascade do |t|
t.string "name", null: false
Expand Down Expand Up @@ -52,8 +52,8 @@
create_table "requests", force: :cascade do |t|
t.string "name"
t.integer "cpu_cores"
t.integer "ram_mb"
t.integer "storage_mb"
t.integer "ram_gb"
t.integer "storage_gb"
t.string "operating_system"
t.text "comment"
t.text "rejection_information"
Expand Down
12 changes: 6 additions & 6 deletions spec/controllers/requests_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
{
name: 'myvm',
cpu_cores: 2,
ram_mb: 1,
storage_mb: 2,
ram_gb: 1,
storage_gb: 2,
operating_system: 'MyOS',
description: 'Description',
comment: 'Comment',
Expand All @@ -52,8 +52,8 @@
{
name: '',
cpu_cores: 2,
ram_mb: 1000,
storage_mb: -2000,
ram_gb: 1000,
storage_gb: -2000,
operating_system: '',
description: '',
comment: 'Comment',
Expand Down Expand Up @@ -175,8 +175,8 @@
{
name: 'mynewvm',
cpu_cores: 3,
ram_mb: 2,
storage_mb: 3,
ram_gb: 2,
storage_gb: 3,
operating_system: 'MyNewOS',
comment: 'newComment',
status: 'pending',
Expand Down
4 changes: 2 additions & 2 deletions spec/factories/requests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
factory :request do
name { 'myvm' }
cpu_cores { 2 }
ram_mb { 1000 }
storage_mb { 2000 }
ram_gb { 1 }
storage_gb { 3 }
operating_system { 'MyOS' }
description { 'Description' }
comment { 'Comment' }
Expand Down
10 changes: 0 additions & 10 deletions spec/helpers/requests_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,4 @@
teststring = 'teststring'
expect(replace_whitespaces(teststring)).to eq(teststring)
end

it 'converts mb to gb correctly' do
megabyte = 9_000
expect(mb_to_gb(megabyte)).to eq(9)
end

it 'converts gb to mb correctly' do
gigabyte = 9
expect(gb_to_mb(gigabyte)).to eq(9000)
end
end
12 changes: 6 additions & 6 deletions spec/models/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
end

it 'is invalid with no ram specifiation' do
request.ram_mb = nil
request.ram_gb = nil
expect(request).to be_invalid
end

it 'is invalid with no storage specifiation' do
request.storage_mb = nil
request.storage_gb = nil
expect(request).to be_invalid
end

Expand All @@ -53,22 +53,22 @@
end

it 'is invalid with negative ram specifiation' do
request.ram_mb = -1
request.ram_gb = -1
expect(request).to be_invalid
end

it 'is invalid with to much ram' do
request.ram_mb = Request::MAX_RAM_MB + 1
request.ram_gb = Request::MAX_RAM_GB + 1
expect(request).to be_invalid
end

it 'is invalid with negative storage specifiation' do
request.storage_mb = -1
request.storage_gb = -1
expect(request).to be_invalid
end

it 'is invalid with to much storage' do
request.storage_mb = Request::MAX_STORAGE_MB + 1
request.storage_gb = Request::MAX_STORAGE_GB + 1
expect(request).to be_invalid
end

Expand Down
66 changes: 66 additions & 0 deletions spec/models/request_templates_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe RequestTemplate, type: :model do
describe 'validation tests' do
let(:request_template) { FactoryBot.build :request_template }

context 'when request template is invalid' do
it 'is invalid with no name' do
request_template.name = ''
expect(request_template).to be_invalid
end

it 'is invalid with no cpu_cores specifiation' do
request_template.cpu_cores = nil
expect(request_template).to be_invalid
end

it 'is invalid with no ram specifiation' do
request_template.ram_gb = nil
expect(request_template).to be_invalid
end

it 'is invalid with no storage specifiation' do
request_template.storage_gb = nil
expect(request_template).to be_invalid
end

it 'is invalid with no operating_system specification' do
request_template.operating_system = ''
expect(request_template).to be_invalid
end

it 'is invalid with negative cpu_cores specifiation' do
request_template.cpu_cores = -1
expect(request_template).to be_invalid
end

it 'is invalid with too many cpu_cores ' do
request_template.cpu_cores = Request::MAX_CPU_CORES + 1
expect(request_template).to be_invalid
end

it 'is invalid with negative ram specifiation' do
request_template.ram_gb = -1
expect(request_template).to be_invalid
end

it 'is invalid with to much ram' do
request_template.ram_gb = Request::MAX_RAM_GB + 1
expect(request_template).to be_invalid
end

it 'is invalid with negative storage specifiation' do
request_template.storage_gb = -1
expect(request_template).to be_invalid
end

it 'is invalid with to much storage' do
request_template.storage_gb = Request::MAX_STORAGE_GB + 1
expect(request_template).to be_invalid
end
end
end
end
Loading