From b979c133663897a05e87b8f10a3647937f92bf09 Mon Sep 17 00:00:00 2001 From: MaximilianKoenig Date: Fri, 1 Feb 2019 14:26:32 +0100 Subject: [PATCH] Save ram and storage of a request in gb internally (#342) * Save ram and storage of a request in gb internally * Fix CodeFactor * Fix Tests * Fix same Tests, hopefully * Forgot Gemfile * Add some tests for request templates and convert gb to mibi and kibi for vsphere * Rename comments * Modify migration * CodeFactor ... * Modify migration again * Refactor os on vm edit config page --- Gemfile.lock | 2 +- app/api/v_sphere/virtual_machine.rb | 4 +- app/controllers/requests_controller.rb | 4 +- app/helpers/requests_helper.rb | 8 --- app/helpers/vms_helper.rb | 4 -- app/models/request.rb | 20 ++++-- app/models/request_template.rb | 10 +-- app/views/request_templates/_form.html.erb | 4 +- app/views/requests/_form.html.erb | 8 +-- app/views/requests/_request.json.jbuilder | 2 +- app/views/requests/index.html.erb | 8 +-- app/views/requests/show.html.erb | 4 +- app/views/vms/edit_config.html.erb | 4 +- .../20190131115425_rename_request_mb_to_gb.rb | 23 +++++++ db/schema.rb | 6 +- spec/controllers/requests_controller_spec.rb | 12 ++-- spec/factories/requests.rb | 4 +- spec/helpers/requests_helper_spec.rb | 10 --- spec/helpers/vms_helper_spec.rb | 11 ---- spec/models/request_spec.rb | 12 ++-- spec/models/request_templates_spec.rb | 66 +++++++++++++++++++ spec/views/requests/edit.html.erb_spec.rb | 8 +-- spec/views/requests/index.html.erb_spec.rb | 8 +-- spec/views/requests/new.html.erb_spec.rb | 16 ++--- spec/views/requests/show.html.erb_spec.rb | 4 +- 25 files changed, 160 insertions(+), 102 deletions(-) create mode 100644 db/migrate/20190131115425_rename_request_mb_to_gb.rb delete mode 100644 spec/helpers/vms_helper_spec.rb create mode 100644 spec/models/request_templates_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index 1488d1ed..0799c890 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -473,4 +473,4 @@ RUBY VERSION ruby 2.5.0p0 BUNDLED WITH - 1.16.2 + 1.17.1 diff --git a/app/api/v_sphere/virtual_machine.rb b/app/api/v_sphere/virtual_machine.rb index 48a6894b..03223cdd 100644 --- a/app/api/v_sphere/virtual_machine.rb +++ b/app/api/v_sphere/virtual_machine.rb @@ -265,12 +265,12 @@ def macs @vm.macs end - private - def request Request.accepted.find { |each| name == each.name } end + private + def archivation_request ArchivationRequest.find_by_name(name) end diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 85bb68f6..6b0f5df6 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -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 diff --git a/app/helpers/requests_helper.rb b/app/helpers/requests_helper.rb index 666f9949..5fee3dd3 100644 --- a/app/helpers/requests_helper.rb +++ b/app/helpers/requests_helper.rb @@ -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 diff --git a/app/helpers/vms_helper.rb b/app/helpers/vms_helper.rb index 60549a64..7073f50f 100644 --- a/app/helpers/vms_helper.rb +++ b/app/helpers/vms_helper.rb @@ -32,8 +32,4 @@ def status_for(vm) 'offline' end end - - def request_for(vm) - Request.accepted.find { |each| vm.name == each.name } - end end diff --git a/app/models/request.rb b/app/models/request.rb index 8bd86579..dc5bec43 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -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" @@ -70,7 +70,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 @@ -115,4 +115,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 + end + + def gibi_to_kibi(gibi) + gibi * 1024**2 + end end diff --git a/app/models/request_template.rb b/app/models/request_template.rb index d8296885..4dd5a500 100644 --- a/app/models/request_template.rb +++ b/app/models/request_template.rb @@ -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 diff --git a/app/views/request_templates/_form.html.erb b/app/views/request_templates/_form.html.erb index 84d4d130..c1f87388 100644 --- a/app/views/request_templates/_form.html.erb +++ b/app/views/request_templates/_form.html.erb @@ -23,12 +23,12 @@
- <%= form.label :ram_mb, "RAM in GB" %> + <%= form.label :ram_gb, "RAM in GB" %> <%= form.number_field :ram_gb, min: 0, class: 'form-control' %>
- <%= form.label :storage_mb, "Storage in GB" %> + <%= form.label :storage_gb, "Storage in GB" %> <%= form.number_field :storage_gb, min: 0, class: 'form-control' %>
diff --git a/app/views/requests/_form.html.erb b/app/views/requests/_form.html.erb index aca1a338..7a8464bb 100644 --- a/app/views/requests/_form.html.erb +++ b/app/views/requests/_form.html.erb @@ -21,13 +21,13 @@
- <%= 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' %>
- <%= 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' %>
diff --git a/app/views/requests/_request.json.jbuilder b/app/views/requests/_request.json.jbuilder index 61f19db7..2508d87e 100644 --- a/app/views/requests/_request.json.jbuilder +++ b/app/views/requests/_request.json.jbuilder @@ -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) diff --git a/app/views/requests/index.html.erb b/app/views/requests/index.html.erb index ff80b9f0..68476bb4 100644 --- a/app/views/requests/index.html.erb +++ b/app/views/requests/index.html.erb @@ -35,8 +35,8 @@ <%= link_to request.name, request_path(request), class: "btn btn-link" %> <%= request.cpu_cores %> - <%= mb_to_gb(request.ram_mb) %> GB - <%= mb_to_gb(request.storage_mb) %> GB + <%= request.ram_gb %> GB + <%= request.storage_gb %> GB <%= request.operating_system %> <%= request.port %> <%= request.application_name %> @@ -71,8 +71,8 @@ <%= link_to request.name, request_path(request), class: "btn btn-link" %> <%= request.cpu_cores %> - <%= mb_to_gb(request.ram_mb) %> GB - <%= mb_to_gb(request.storage_mb) %> GB + <%= request.ram_gb %> GB + <%= request.storage_gb %> GB <%= request.operating_system %> <%= request.port %> <%= request.application_name %> diff --git a/app/views/requests/show.html.erb b/app/views/requests/show.html.erb index 244871f1..027eb039 100644 --- a/app/views/requests/show.html.erb +++ b/app/views/requests/show.html.erb @@ -14,12 +14,12 @@ RAM in GB: - <%= mb_to_gb(@request.ram_mb) %> + <%= @request.ram_gb %> Storage in GB: - <%= mb_to_gb(@request.storage_mb) %> + <%= @request.storage_gb %> diff --git a/app/views/vms/edit_config.html.erb b/app/views/vms/edit_config.html.erb index 38b6865d..bb44a530 100644 --- a/app/views/vms/edit_config.html.erb +++ b/app/views/vms/edit_config.html.erb @@ -31,12 +31,12 @@ Operating System: - <%= request_for(@vm).operating_system %> + <%= @vm.request.operating_system %> Comment: - <%= request_for(@vm).comment %> + <%= @vm.request.comment %> diff --git a/db/migrate/20190131115425_rename_request_mb_to_gb.rb b/db/migrate/20190131115425_rename_request_mb_to_gb.rb new file mode 100644 index 00000000..0b234da5 --- /dev/null +++ b/db/migrate/20190131115425_rename_request_mb_to_gb.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class RenameRequestMbToGb < ActiveRecord::Migration[5.2] + def up + rename_column :requests, :ram_mb, :ram_gb + rename_column :requests, :storage_mb, :storage_gb + update_requests + end + + def update_requests + Request.all.each do |request| + unless request.nil? + request.ram_gb = (request.ram_gb / 1000.0).ceil + request.storage_gb = (request.storage_gb / 1000.0).ceil + end + request.save + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/schema.rb b/db/schema.rb index a5e5fc0b..f149f1e5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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 @@ -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" diff --git a/spec/controllers/requests_controller_spec.rb b/spec/controllers/requests_controller_spec.rb index d2451057..d3e6e05f 100644 --- a/spec/controllers/requests_controller_spec.rb +++ b/spec/controllers/requests_controller_spec.rb @@ -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', @@ -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', @@ -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', diff --git a/spec/factories/requests.rb b/spec/factories/requests.rb index 07b823a6..cd00f38b 100644 --- a/spec/factories/requests.rb +++ b/spec/factories/requests.rb @@ -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' } diff --git a/spec/helpers/requests_helper_spec.rb b/spec/helpers/requests_helper_spec.rb index 9f7e42f4..567bc6a4 100644 --- a/spec/helpers/requests_helper_spec.rb +++ b/spec/helpers/requests_helper_spec.rb @@ -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 diff --git a/spec/helpers/vms_helper_spec.rb b/spec/helpers/vms_helper_spec.rb deleted file mode 100644 index 2b37a1fc..00000000 --- a/spec/helpers/vms_helper_spec.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe VmsHelper, type: :helper do - it 'returns the request to the vm correctly' do - vm = v_sphere_vm_mock 'testvm' - request = FactoryBot.create :accepted_request, name: 'testvm' - expect(request_for(vm).name).to eq(request.name) - end -end diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb index 3bc97802..586adb6b 100644 --- a/spec/models/request_spec.rb +++ b/spec/models/request_spec.rb @@ -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 @@ -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 diff --git a/spec/models/request_templates_spec.rb b/spec/models/request_templates_spec.rb new file mode 100644 index 00000000..c9071bfb --- /dev/null +++ b/spec/models/request_templates_spec.rb @@ -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 diff --git a/spec/views/requests/edit.html.erb_spec.rb b/spec/views/requests/edit.html.erb_spec.rb index 4d932e0a..c244c037 100644 --- a/spec/views/requests/edit.html.erb_spec.rb +++ b/spec/views/requests/edit.html.erb_spec.rb @@ -7,8 +7,8 @@ @request = assign(:request, Request.create!( name: 'myvm', cpu_cores: 2, - ram_mb: 1000, - storage_mb: 1000, + ram_gb: 1, + storage_gb: 2, operating_system: 'MyOS', port: '4000', application_name: 'MyName', @@ -34,9 +34,9 @@ assert_select 'input[name=?]', 'request[cpu_cores]' - assert_select 'input[name=?]', 'request[ram_mb]' + assert_select 'input[name=?]', 'request[ram_gb]' - assert_select 'input[name=?]', 'request[storage_mb]' + assert_select 'input[name=?]', 'request[storage_gb]' assert_select 'select[name=?]', 'request[operating_system]' diff --git a/spec/views/requests/index.html.erb_spec.rb b/spec/views/requests/index.html.erb_spec.rb index 933e7892..28c8b8ab 100644 --- a/spec/views/requests/index.html.erb_spec.rb +++ b/spec/views/requests/index.html.erb_spec.rb @@ -8,8 +8,8 @@ Request.create!( name: 'myvm', cpu_cores: 3, - ram_mb: 1000, - storage_mb: 2000, + ram_gb: 1, + storage_gb: 2, operating_system: 'MyOS', port: '4000', application_name: 'MyName', @@ -21,8 +21,8 @@ Request.create!( name: 'myvm1', cpu_cores: 3, - ram_mb: 1000, - storage_mb: 2000, + ram_gb: 1, + storage_gb: 2, operating_system: 'MyOS', port: '4000', application_name: 'MyName', diff --git a/spec/views/requests/new.html.erb_spec.rb b/spec/views/requests/new.html.erb_spec.rb index 174116d4..908470a8 100644 --- a/spec/views/requests/new.html.erb_spec.rb +++ b/spec/views/requests/new.html.erb_spec.rb @@ -9,8 +9,8 @@ Request.new( name: 'myvm', cpu_cores: 2, - ram_mb: 1000, - storage_mb: 2000, + ram_gb: 1, + storage_gb: 2, operating_system: 'MyOS', port: '4000', application_name: 'MyName', @@ -60,9 +60,9 @@ assert_select 'input[name=?][min=?]', 'request[cpu_cores]', '0' - assert_select 'input[name=?][min=?]', 'request[ram_mb]', '0' + assert_select 'input[name=?][min=?]', 'request[ram_gb]', '0' - assert_select 'input[name=?][min=?]', 'request[storage_mb]', '0' + assert_select 'input[name=?][min=?]', 'request[storage_gb]', '0' assert_select 'select[name=?]', 'request[operating_system]' @@ -84,12 +84,12 @@ expect(request).not_to be_persisted end - it 'renders ram_mb as gb' do - assert_select 'input[id=ram][value=?]', (request.ram_mb / 1000).to_s + it 'renders ram_gb' do + assert_select 'input[id=ram][value=?]', request.ram_gb.to_s end - it 'renders storage_mb as gb' do - assert_select 'input[id=storage][value=?]', (request.storage_mb / 1000).to_s + it 'renders storage_gb' do + assert_select 'input[id=storage][value=?]', request.storage_gb.to_s end end end diff --git a/spec/views/requests/show.html.erb_spec.rb b/spec/views/requests/show.html.erb_spec.rb index ca4ded65..facb5228 100644 --- a/spec/views/requests/show.html.erb_spec.rb +++ b/spec/views/requests/show.html.erb_spec.rb @@ -11,8 +11,8 @@ @request = assign(:request, Request.create!( name: 'myvm', cpu_cores: 3, - ram_mb: 1000, - storage_mb: 2000, + ram_gb: 1, + storage_gb: 2, operating_system: 'MyOS', port: '4000', application_name: 'MyName',