Modify data center config #243 (#245)

* Modify data center config #243

* Fix issues reported by Codeclimate

docker-compose.yml

@@ -1,7 +1,7 @@
 version: '3.2'
 services:
   web:
-    image: "hailstorm3/hailstorm-web-client:1.8.10"
+    image: "hailstorm3/hailstorm-web-client:1.9.10"
     ports:
       - "8080:80"
     networks:
@@ -22,7 +22,7 @@ services:
       - "start.sh"
 
   hailstorm-api:
-    image: "hailstorm3/hailstorm-api:1.0.18"
+    image: "hailstorm3/hailstorm-api:1.0.19"
     ports:
       - "4567:8080"
     environment:

hailstorm-api/app/api/clusters.rb

@@ -90,8 +90,9 @@
 
 patch '/projects/:project_id/clusters/:id' do |project_id, id|
   found_project = Hailstorm::Model::Project.find(project_id)
-  project_config = ProjectConfiguration.find_by_project_id!(found_project.id)
+  return 422 unless found_project.current_execution_cycle.nil?
 
+  project_config = ProjectConfiguration.find_by_project_id!(found_project.id)
   # @type [Hailstorm::Support::Configuration] hailstorm_config
   hailstorm_config = deep_decode(project_config.stringified_config)
   matched_cluster_cfg = find_cluster_cfg(hailstorm_config, id)
@@ -103,12 +104,11 @@
   is_project_live = !found_project.load_agents.empty?
   data.each_pair do |key, value|
     field_name = key.underscore.to_sym
-    return 422 if matched_cluster_cfg.active == false && field_name != :active
-    return 422 if field_name == :region
-    return 422 if field_name == :base_ami && matched_cluster_cfg.base_ami.blank?
     return 422 if is_project_live && field_name != :max_threads_per_agent
+    return 422 unless patch_request_valid?(matched_cluster_cfg, field_name)
 
-    matched_cluster_cfg.send("#{field_name}=", value)
+    field_value = query_field_value(matched_cluster_cfg, field_name: field_name, value: value)
+    matched_cluster_cfg.send("#{field_name}=", field_value)
   end
 
   project_config.update!(stringified_config: deep_encode(hailstorm_config))

hailstorm-api/app/helpers/clusters_helper.rb

@@ -152,4 +152,28 @@ def string_to_id(str)
   def to_array(any)
     any.is_a?(Array) ? any : [any]
   end
+
+  # @param [Hailstorm::Support::Configuration::ClusterBase] cluster_config
+  # @param [String] field_name
+  # @param [Object] value
+  # @return [Object]
+  def query_field_value(cluster_config, field_name:, value:)
+    field_value = value
+    if field_name == :ssh_identity && cluster_config.cluster_type == :data_center
+      field_value = "#{value['path']}/#{value['name']}"
+    end
+
+    field_value
+  end
+
+  # @param [Hailstorm::Support::Configuration::ClusterBase] cluster_config
+  # @param [String] field_name
+  def patch_request_valid?(cluster_config, field_name)
+    return false if cluster_config.active == false && field_name != :active
+    return false if field_name == :region
+    return false if field_name == :base_ami && cluster_config.base_ami.blank?
+    return false if field_name == :code
+
+    true
+  end
 end

hailstorm-api/app/version.rb

@@ -3,6 +3,6 @@
 # Version
 module Hailstorm
   module Api
-    VERSION = '1.0.18'
+    VERSION = '1.0.19'
   end
 end

hailstorm-api/spec/api/clusters_spec.rb

@@ -381,30 +381,8 @@
       @hailstorm_config = Hailstorm::Support::Configuration.new
     end
 
-    it 'should update the cluster attributes in project configuration' do
-      @hailstorm_config.clusters(:data_center) do |dc|
-        # @type [Hailstorm::Support::Configuration::DataCenter] dc
-        dc.title = 'Ice station Zebra'
-        dc.user_name = 'ubuntu'
-        dc.ssh_identity = '123/foo.pem'
-        dc.machines = %w[172.16.0.10 172.16.0.20 172.16.0.30]
-        dc.ssh_port = 8022
-        dc.cluster_code = 'ice-station-zebra-119'
-        dc.active = false
-      end
-
-      ProjectConfiguration.create!(project: @project, stringified_config: deep_encode(@hailstorm_config))
-      cluster_id = @hailstorm_config.clusters.first.title.to_java_string.hash_code
-      @browser.patch("/projects/#{@project.id}/clusters/#{cluster_id}", JSON.dump({ active: true }))
-      expect(@browser.last_response.status).to be == 200
-      project_config = ProjectConfiguration.first
-      hailstorm_config = deep_decode(project_config.stringified_config)
-      dc = hailstorm_config.clusters.first
-      expect(dc.active).to be true
-    end
-
-    context 'cluster is disabled' do
-      it 'should not update any field other than active' do
+    context 'any kind of cluster' do
+      before(:each) do
         @hailstorm_config.clusters(:data_center) do |dc|
           # @type [Hailstorm::Support::Configuration::DataCenter] dc
           dc.title = 'Ice station Zebra'
@@ -417,9 +395,63 @@
         end
 
         ProjectConfiguration.create!(project: @project, stringified_config: deep_encode(@hailstorm_config))
-        cluster_id = @hailstorm_config.clusters.first.title.to_java_string.hash_code
-        @browser.patch("/projects/#{@project.id}/clusters/#{cluster_id}", JSON.dump({ user_name: 'root' }))
+        @cluster_id = @hailstorm_config.clusters.first.title.to_java_string.hash_code
+      end
+
+      it 'should be able to activate the cluster' do
+        @browser.patch("/projects/#{@project.id}/clusters/#{@cluster_id}", JSON.dump({ active: true }))
+        expect(@browser.last_response.status).to be == 200
+        project_config = ProjectConfiguration.first
+        hailstorm_config = deep_decode(project_config.stringified_config)
+        dc = hailstorm_config.clusters.first
+        expect(dc.active).to be true
+      end
+
+      it 'should be able to de-activate the cluster' do
+        @hailstorm_config.clusters.first.active = true
+        ProjectConfiguration.first.update_attributes!(stringified_config: deep_encode(@hailstorm_config))
+        @browser.patch("/projects/#{@project.id}/clusters/#{@cluster_id}", JSON.dump({ active: false }))
+        expect(@browser.last_response.status).to be == 200
+        project_config = ProjectConfiguration.first
+        hailstorm_config = deep_decode(project_config.stringified_config)
+        dc = hailstorm_config.clusters.first
+        expect(dc.active).to be false
+      end
+
+      context 'cluster is disabled' do
+        it 'should not update any field other than active' do
+          @browser.patch("/projects/#{@project.id}/clusters/#{@cluster_id}", JSON.dump({ user_name: 'root' }))
+          expect(@browser.last_response.status).to be == 422
+        end
+      end
+
+      it 'should not update cluster_code' do
+        @browser.patch("/projects/#{@project.id}/clusters/#{@cluster_id}", JSON.dump({ code: 'bot-cluster-200' }))
         expect(@browser.last_response.status).to be == 422
+        project_config = ProjectConfiguration.first
+        hailstorm_config = deep_decode(project_config.stringified_config)
+        # @type [Hailstorm::Support::Configuration::DataCenter] dc
+        dc = hailstorm_config.clusters.first
+        expect(dc.cluster_code).to be == 'ice-station-zebra-119' # unchanged in the update
+      end
+
+      context 'project is running tests' do
+        it 'should not update any attribute' do
+          Hailstorm::Model::ExecutionCycle.create!(
+            project: @project,
+            status: Hailstorm::Model::ExecutionCycle::States::STARTED,
+            started_at: Time.now.ago(30.minutes),
+            threads_count: 100
+          )
+
+          @browser.patch("/projects/#{@project.id}/clusters/#{@cluster_id}", JSON.dump({ active: true }))
+          expect(@browser.last_response.status).to be == 422
+          project_config = ProjectConfiguration.first
+          hailstorm_config = deep_decode(project_config.stringified_config)
+          # @type [Hailstorm::Support::Configuration::DataCenter] dc
+          dc = hailstorm_config.clusters.first
+          expect(dc.active).to be_blank
+        end
       end
     end
 
@@ -462,5 +494,45 @@
         end
       end
     end
+
+    context 'when DataCenter cluster' do
+      before(:each) do
+        @hailstorm_config.clusters(:data_center) do |dc|
+          # @type [Hailstorm::Support::Configuration::DataCenter] dc
+          dc.title = 'Bot cluster 2'
+          dc.user_name = 'root'
+          dc.ssh_identity = '123/foo.pem'
+          dc.machines = %w[172.16.0.10 172.16.0.20]
+          dc.ssh_port = 22
+          dc.cluster_code = 'bot-cluster-2'
+        end
+
+        ProjectConfiguration.create!(project: @project, stringified_config: deep_encode(@hailstorm_config))
+        @cluster_id = @hailstorm_config.clusters.first.title.to_java_string.hash_code
+      end
+
+      it 'should update all allowed attributes' do
+        request_params = {
+          title: 'Bot cluster 1',
+          userName: 'ubuntu',
+          sshIdentity: { name: 'secure.pem', path: '1234' },
+          sshPort: 8022,
+          machines: %w[172.16.0.10 172.16.0.20 172.16.0.30]
+        }
+
+        @browser.patch("/projects/#{@project.id}/clusters/#{@cluster_id}", JSON.dump(request_params))
+        expect(@browser.last_response.status).to be == 200
+        project_config = ProjectConfiguration.first
+        hailstorm_config = deep_decode(project_config.stringified_config)
+        # @type [Hailstorm::Support::Configuration::DataCenter] dc
+        dc = hailstorm_config.clusters.first
+        expect(dc.title).to be == request_params[:title]
+        expect(dc.user_name).to be == request_params[:userName]
+        expect(dc.ssh_identity).to be == "#{request_params[:sshIdentity][:path]}/#{request_params[:sshIdentity][:name]}"
+        expect(dc.machines).to be == request_params[:machines]
+        expect(dc.ssh_port).to be == request_params[:sshPort]
+        expect(dc.cluster_code).to be == 'bot-cluster-2' # unchanged in the update
+      end
+    end
   end
 end

hailstorm-web-client/README.md

@@ -83,7 +83,7 @@ This section has moved here: https://facebook.github.io/create-react-app/docs/tr
 
 ## Principles
 
-- Use flat source structure as much as possible.
-- Global state and reducer if needed in a component, are passed on as props to children if they too require the global state and reducer.
+- Use flat source structure as much as possible. At the lowest level, have components that render HTML and not another component. If a component is decomposed to more components, it is elevated one level up and becomes a component folder. There are always only two levels.
+- Design most components to be pure functions.
 - Use CSS Modules for component CSS customizations, and SCSS for global styles.
 - Write new tests with [React Testing Library](https://testing-library.com/docs/react-testing-library/intro/). If existing Enyzme tests are flaky, change to React Testing Library. A test can be considered flaky if the test fails when an implementation detail changes, or they are timing dependent.

hailstorm-web-client/package.json

@@ -1,6 +1,6 @@
 {
   "name": "hailstorm-web-client",
-  "version": "1.8.10",
+  "version": "1.9.10",
   "private": true,
   "dependencies": {
     "date-fns": "^2.6.0",

hailstorm-web-client/src/ClusterConfiguration/AWSForm.tsx → hailstorm-web-client/src/AWSForm/AWSForm.tsx

@@ -1,14 +1,14 @@
 import React from 'react';
-import { ErrorMessage, Field, Form, Formik } from 'formik';
+import { Form, Formik } from 'formik';
 import { AmazonCluster, Project } from '../domain';
 import { FormikActionsHandler } from '../JMeterConfiguration/domain';
 import { AWSFormField, AWSInstanceChoiceField } from './AWSFormField';
 import { AWSRegionChoice } from './AWSRegionChoice';
-import { ClusterFormFooter } from './ClusterFormFooter';
-import { AWSInstanceChoiceOption, AWSRegionList } from './domain';
-import { ReadOnlyField } from './ReadOnlyField';
+import { ClusterFormFooter } from '../ClusterConfiguration/ClusterFormFooter';
+import { AWSInstanceChoiceOption, AWSRegionList } from '../ClusterConfiguration/domain';
+import { ReadOnlyField } from '../ClusterConfiguration/ReadOnlyField';
 import { MaxUsersByInstance } from './AWSInstanceChoice';
-import { RemoveCluster } from './RemoveCluster';
+import styles from '../NewProjectWizard/NewProjectWizard.module.scss';
 
 export function AWSForm({
   cluster,
@@ -60,7 +60,7 @@ export function AWSForm({
     >
       {({ isSubmitting, isValid, setFieldTouched, handleChange, values }) => (
       <Form data-testid="AWSForm">
-        <div className="card-content">
+        <div className={`card-content${cluster && cluster.disabled ? ` ${styles.disabledContent}` : ''}`}>
           <AWSFormField
             labelText="AWS Access Key"
             required={true}
@@ -132,26 +132,12 @@ export function AWSForm({
           />
           )}
         </div>
-        {formMode === 'new' && dispatch ? (
-        <ClusterFormFooter {...{dispatch}} disabled={isSubmitting || !isValid} />
-        ) : (
-        activeProject && cluster && dispatch && (
-        <div className="card-footer">
-          <RemoveCluster {...{activeProject, cluster, dispatch}} />
-          {!cluster.disabled && (
-            <div className="card-footer-item">
-              <button
-                type="submit"
-                className="button is-primary"
-                role="Update Cluster"
-                disabled={isSubmitting || !isValid}
-              >
-                Update
-              </button>
-            </div>
-          )}
-        </div>)
-        )}
+        {dispatch && (
+        <ClusterFormFooter
+          {...{dispatch, activeProject, cluster}}
+          disabled={isSubmitting || !isValid}
+          newCluster={formMode === 'new'}
+        />)}
       </Form>)}
     </Formik>
   );

hailstorm-web-client/src/ClusterConfiguration/AWSFormField.tsx → hailstorm-web-client/src/AWSForm/AWSFormField.tsx

@@ -1,8 +1,8 @@
 import React, { useState } from 'react';
 import { ErrorMessage, Field } from "formik";
-import { ReadOnlyField } from "./ReadOnlyField";
+import { ReadOnlyField } from "../ClusterConfiguration/ReadOnlyField";
 import { AWSInstanceChoice } from './AWSInstanceChoice';
-import { AWSInstanceChoiceOption } from './domain';
+import { AWSInstanceChoiceOption } from '../ClusterConfiguration/domain';
 import { ApiFactory } from '../api';
 
 export function AWSFormField({

hailstorm-web-client/src/ClusterConfiguration/AWSInstanceCalculator.test.ts → hailstorm-web-client/src/AWSForm/AWSInstanceCalculator.test.ts

@@ -1,5 +1,5 @@
 import { computeChoice } from './AWSInstanceCalculator';
-import { AWSInstanceChoiceOption } from './domain';
+import { AWSInstanceChoiceOption } from '../ClusterConfiguration/domain';
 
 describe('AWSInstanceCalculator', () => {
   for (const dataPoint of [

hailstorm-web-client/src/ClusterConfiguration/AWSInstanceCalculator.ts → hailstorm-web-client/src/AWSForm/AWSInstanceCalculator.ts

@@ -1,4 +1,4 @@
-import { AWSInstanceChoiceOption } from "./domain";
+import { AWSInstanceChoiceOption } from "../ClusterConfiguration/domain";
 
 export function computeChoice(
   numThreads: number,

hailstorm-web-client/src/ClusterConfiguration/AWSInstanceChoice.test.tsx → hailstorm-web-client/src/AWSForm/AWSInstanceChoice.test.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import { render, mount } from 'enzyme';
 import { AWSInstanceChoice, InstanceTypeMeter } from './AWSInstanceChoice';
-import { AWSInstanceChoiceOption } from './domain';
+import { AWSInstanceChoiceOption } from '../ClusterConfiguration/domain';
 import { render as renderComponent, fireEvent } from '@testing-library/react';
 import { Form, Formik } from 'formik';
 import { AppNotificationProviderWithProps } from '../AppNotificationProvider';

hailstorm-web-client/src/ClusterConfiguration/AWSInstanceChoice.tsx → hailstorm-web-client/src/AWSForm/AWSInstanceChoice.tsx

@@ -1,7 +1,7 @@
 import React, { useState, useEffect } from 'react';
 import { NonLinearSlider } from './NonLinearSlider';
 import { computeChoice, maxThreadsByCluster } from './AWSInstanceCalculator';
-import { AWSInstanceChoiceOption } from './domain';
+import { AWSInstanceChoiceOption } from '../ClusterConfiguration/domain';
 import { Loader, LoadingMessage } from '../Loader';
 import { Field } from 'formik';
 import { useNotifications } from '../app-notifications';

hailstorm-web-client/src/ClusterConfiguration/AWSRegionChoice.test.tsx → hailstorm-web-client/src/AWSForm/AWSRegionChoice.test.tsx

@@ -2,7 +2,7 @@ import React from 'react';
 import { render, mount } from 'enzyme';
 import { fireEvent, render as renderComponent, RenderResult, wait } from '@testing-library/react';
 import { AWSRegionChoice } from './AWSRegionChoice';
-import { AWSRegionList, AWSRegionType } from './domain';
+import { AWSRegionList, AWSRegionType } from '../ClusterConfiguration/domain';
 import { act } from 'react-dom/test-utils';
 
 describe('<AWSRegionChoice />', () => {

hailstorm-web-client/src/ClusterConfiguration/AWSRegionChoice.tsx → hailstorm-web-client/src/AWSForm/AWSRegionChoice.tsx

@@ -1,5 +1,5 @@
 import React, { useState, useEffect } from 'react';
-import { AWSRegionType, AWSRegionList } from './domain';
+import { AWSRegionType, AWSRegionList } from '../ClusterConfiguration/domain';
 import { Loader } from '../Loader/Loader';
 import { useNotifications } from '../app-notifications';
 import { ErrorMessage, Field, Form, Formik } from 'formik';

hailstorm-web-client/src/AWSForm/package.json

@@ -0,0 +1,3 @@
+{
+  "main": "AWSForm.tsx"
+}