Improve validation

evergreen-ci · Oct 3, 2023 · 8856f4e · 8856f4e
1 parent c1fa7a1
commit 8856f4e
Show file tree

Hide file tree

Showing 12 changed files with 132 additions and 70 deletions.
diff --git a/cypress/integration/distroSettings/general_section.ts b/cypress/integration/distroSettings/general_section.ts
@@ -39,4 +39,16 @@ describe("general section", () => {
     save();
     cy.validateToast("success");
   });
+
+  describe("container pool distro", () => {
+    beforeEach(() => {
+      cy.visit("/distro/ubuntu1604-parent/settings/general");
+    });
+
+    it("warns users that the distro will not be spawned for tasks", () => {
+      cy.contains(
+        "Distro is a container pool, so it cannot be spawned for tasks."
+      ).should("be.visible");
+    });
+  });
 });
diff --git a/cypress/integration/distroSettings/host_section.ts b/cypress/integration/distroSettings/host_section.ts
@@ -12,6 +12,8 @@ describe("host section", () => {
       cy.dataCy("maximum-hosts-input").should("not.exist");
       cy.dataCy("idle-time-input").should("not.exist");
       cy.dataCy("future-fraction-input").should("not.exist");
+      cy.dataCy("rounding-rule-select").should("not.exist");
+      cy.dataCy("feedback-rule-select").should("not.exist");
     });
 
     it("shows an error when selecting an incompatible host communication method", () => {
@@ -29,8 +31,6 @@ describe("host section", () => {
       cy.getInputByLabel("SSH User").type("sudo");
       cy.contains("button", "Add SSH option").click();
       cy.getInputByLabel("SSH Option").type("BatchMode=yes");
-      cy.selectLGOption("Host Allocator Rounding Rule", "Round down");
-      cy.selectLGOption("Host Allocator Feedback Rule", "No feedback");
       cy.selectLGOption(
         "Host Overallocation Rule",
         "Terminate hosts when overallocated"
@@ -46,8 +46,6 @@ describe("host section", () => {
       cy.getInputByLabel("SSH User").clear();
       cy.getInputByLabel("SSH User").type("ubuntu");
       cy.dataCy("delete-item-button").click();
-      cy.selectLGOption("Host Allocator Rounding Rule", "Default");
-      cy.selectLGOption("Host Allocator Feedback Rule", "Default");
       cy.selectLGOption("Host Overallocation Rule", "Default");
 
       save();

diff --git a/src/pages/distroSettings/Tabs.tsx b/src/pages/distroSettings/Tabs.tsx
@@ -43,7 +43,10 @@ export const DistroSettingsTabs: React.FC<Props> = ({ distro }) => {
         <Route
           path={DistroSettingsTabRoutes.General}
           element={
-            <GeneralTab distroData={tabData[DistroSettingsTabRoutes.General]} />
+            <GeneralTab
+              distroData={tabData[DistroSettingsTabRoutes.General]}
+              minimumHosts={distro.hostAllocatorSettings.minimumHosts}
+            />
           }
         />
         <Route

diff --git a/src/pages/distroSettings/tabs/GeneralTab/GeneralTab.tsx b/src/pages/distroSettings/tabs/GeneralTab/GeneralTab.tsx
@@ -1,14 +1,29 @@
 import { useMemo } from "react";
+import { useParams } from "react-router-dom";
 import { SettingsCard, SettingsCardTitle } from "components/SettingsCard";
+import { useSpruceConfig } from "hooks";
 import { DeleteDistro } from "pages/distroSettings/DeleteDistro";
 import { BaseTab } from "../BaseTab";
 import { getFormSchema } from "./getFormSchema";
 import { TabProps } from "./types";
 
-export const GeneralTab: React.FC<TabProps> = ({ distroData }) => {
+export const GeneralTab: React.FC<TabProps> = ({
+  distroData,
+  minimumHosts,
+}) => {
+  const { distroId } = useParams();
+  const { containerPools } = useSpruceConfig();
+  const containerPoolDistros =
+    containerPools?.pools?.map(({ distro }) => distro) ?? [];
+
+  const isContainerDistro = containerPoolDistros.includes(distroId);
+
   const initialFormState = distroData;
 
-  const formSchema = useMemo(() => getFormSchema(), []);
+  const formSchema = useMemo(
+    () => getFormSchema(isContainerDistro, minimumHosts),
+    [isContainerDistro, minimumHosts]
+  );
 
   return (
     <>

diff --git a/src/pages/distroSettings/tabs/GeneralTab/getFormSchema.ts b/src/pages/distroSettings/tabs/GeneralTab/getFormSchema.ts
@@ -1,8 +1,10 @@
-import { css } from "@emotion/react";
 import { GetFormSchema } from "components/SpruceForm";
 import { CardFieldTemplate } from "components/SpruceForm/FieldTemplates";
 
-export const getFormSchema = (): ReturnType<GetFormSchema> => ({
+export const getFormSchema = (
+  isContainerDistro: boolean,
+  minimumHosts: number
+): ReturnType<GetFormSchema> => ({
   fields: {},
   schema: {
     type: "object" as "object",
@@ -33,44 +35,44 @@ export const getFormSchema = (): ReturnType<GetFormSchema> => ({
           },
         },
       },
-      distroNote: {
-        type: "object" as "object",
-        title: "Notes",
-        properties: {
-          note: {
-            type: "string" as "string",
-            title: "Notes",
-            default: "",
-          },
-        },
-      },
       distroOptions: {
         type: "object" as "object",
         title: "Distro Options",
         properties: {
           isCluster: {
             type: "boolean" as "boolean",
-            title:
-              "Mark distro as a cluster (jobs are not run on this host, used for special purposes).",
+            title: "Mark distro as cluster",
             default: false,
           },
           disableShallowClone: {
             type: "boolean" as "boolean",
-            title: "Disable shallow clone for this distro.",
+            title: "Disable shallow clone for this distro",
             default: false,
           },
           disabled: {
             type: "boolean" as "boolean",
-            title: "Disable queueing for this distro.",
+            title: "Disable queueing for this distro",
             default: false,
           },
+          note: {
+            type: "string" as "string",
+            title: "Notes",
+            default: "",
+          },
         },
       },
     },
   },
   uiSchema: {
     distroName: {
       "ui:ObjectFieldTemplate": CardFieldTemplate,
+      identifier: {
+        ...(isContainerDistro && {
+          "ui:warnings": [
+            "Distro is a container pool, so it cannot be spawned for tasks.",
+          ],
+        }),
+      },
     },
     distroAliases: {
       "ui:rootFieldId": "aliases",
@@ -84,25 +86,22 @@ export const getFormSchema = (): ReturnType<GetFormSchema> => ({
         },
       },
     },
-    distroNote: {
-      "ui:ObjectFieldTemplate": CardFieldTemplate,
-      note: {
-        "ui:widget": "textarea",
-        "ui:elementWrapperCSS": textAreaCSS,
-      },
-    },
     distroOptions: {
       "ui:ObjectFieldTemplate": CardFieldTemplate,
+      isCluster: {
+        "ui:description":
+          "Jobs will not be run on this host. Used for special purposes.",
+      },
       disabled: {
         "ui:description": "Tasks already in the task queue will be removed.",
+        ...(minimumHosts > 0 && {
+          "ui:tooltipDescription": `This will still allow the minimum number of hosts (${minimumHosts}) to start`,
+        }),
+      },
+      note: {
+        "ui:rows": 7,
+        "ui:widget": "textarea",
       },
     },
   },
 });
-
-const textAreaCSS = css`
-  box-sizing: border-box;
-  textarea {
-    min-height: 150px;
-  }
-`;
diff --git a/src/pages/distroSettings/tabs/GeneralTab/transformers.test.tsx b/src/pages/distroSettings/tabs/GeneralTab/transformers.test.tsx
@@ -20,22 +20,20 @@ const generalForm: GeneralFormState = {
   distroAliases: {
     aliases: ["rhel71-power8", "rhel71-power8-build"],
   },
-  distroNote: {
-    note: "distro note",
-  },
   distroOptions: {
     isCluster: false,
     disableShallowClone: true,
     disabled: false,
+    note: "distro note",
   },
 };
 
 const generalGql: DistroInput = {
   ...distroData,
   name: "rhel71-power8-large",
   aliases: ["rhel71-power8", "rhel71-power8-build"],
-  note: "distro note",
   isCluster: false,
   disableShallowClone: true,
   disabled: false,
+  note: "distro note",
 };
diff --git a/src/pages/distroSettings/tabs/GeneralTab/transformers.ts b/src/pages/distroSettings/tabs/GeneralTab/transformers.ts
@@ -16,25 +16,23 @@ export const gqlToForm = ((data) => {
     distroAliases: {
       aliases,
     },
-    distroNote: {
-      note,
-    },
     distroOptions: {
       isCluster,
       disableShallowClone,
       disabled,
+      note,
     },
   };
 }) satisfies GqlToFormFunction<Tab>;
 
 export const formToGql = ((
-  { distroAliases, distroName, distroNote, distroOptions },
+  { distroAliases, distroName, distroOptions },
   distro
 ) => ({
   ...distro,
   name: distroName.identifier,
   aliases: distroAliases.aliases,
-  note: distroNote.note,
+  note: distroOptions.note,
   isCluster: distroOptions.isCluster,
   disableShallowClone: distroOptions.disableShallowClone,
   disabled: distroOptions.disabled,

diff --git a/src/pages/distroSettings/tabs/GeneralTab/types.ts b/src/pages/distroSettings/tabs/GeneralTab/types.ts
@@ -5,16 +5,15 @@ export interface GeneralFormState {
   distroAliases: {
     aliases: string[];
   };
-  distroNote: {
-    note: string;
-  };
   distroOptions: {
     isCluster: boolean;
     disableShallowClone: boolean;
     disabled: boolean;
+    note: string;
   };
 }
 
 export type TabProps = {
   distroData: GeneralFormState;
+  minimumHosts: number;
 };
diff --git a/src/pages/distroSettings/tabs/HostTab/getFormSchema.tsx b/src/pages/distroSettings/tabs/HostTab/getFormSchema.tsx
@@ -128,7 +128,10 @@ export const getFormSchema = ({
       setup: setup.uiSchema(architecture, hasStaticProvider),
       bootstrapSettings: bootstrapProperties.uiSchema(architecture),
       sshConfig: sshConfigProperties.uiSchema(hasStaticProvider),
-      allocation: allocationProperties.uiSchema(hasEC2Provider),
+      allocation: allocationProperties.uiSchema(
+        hasEC2Provider,
+        hasStaticProvider
+      ),
     },
   };
 };

diff --git a/src/pages/distroSettings/tabs/HostTab/schemaFields.tsx b/src/pages/distroSettings/tabs/HostTab/schemaFields.tsx
@@ -145,6 +145,7 @@ export const icecreamSchedulerHost = {
     title: "Icecream Scheduler Host",
   },
   uiSchema: {
+    "ui:description": "Host name to connect to the icecream scheduler",
     "ui:elementWrapperCSS": indentCSS,
   },
 };
@@ -155,6 +156,7 @@ export const icecreamConfigPath = {
     title: "Icecream Config File Path",
   },
   uiSchema: {
+    "ui:description": "Path to the icecream config file",
     "ui:elementWrapperCSS": indentCSS,
   },
 };
@@ -285,7 +287,7 @@ const lockedMemoryKb = {
 const virtualMemoryKb = {
   schema: {
     type: "number" as "number",
-    title: "Virtual Memory (kB)",
+    title: "Virtual Memory",
     minimum: -1,
   },
   uiSchema: {
@@ -362,7 +364,12 @@ const preconditionScripts = {
       script: {
         "ui:description":
           "The precondition script that must run and succeed before Jasper can start.",
-        "ui:widget": "textarea",
+        "ui:elementWrapperCSS": css`
+          textarea {
+            font-family: ${fontFamilies.code};
+          }
+        `,
+        "ui:rows": 8,
       },
     },
   },
@@ -401,6 +408,8 @@ const authorizedKeysFile = {
   },
   uiSchema: (hasStaticProvider: boolean) => ({
     "ui:data-cy": "authorized-keys-input",
+    "ui:description": "Path to file containing authorized SSH keys",
+    "ui:placeholder": "~/.ssh/authorized_keys",
     ...(!hasStaticProvider && { "ui:widget": "hidden" }),
   }),
 };
@@ -448,9 +457,11 @@ const roundingRule = {
     title: "Host Allocator Rounding Rule",
     oneOf: enumSelect(roundingRuleToCopy),
   },
-  uiSchema: {
+  uiSchema: (hasStaticProvider: boolean) => ({
     "ui:allowDeselect": false,
-  },
+    "ui:data-cy": "rounding-rule-select",
+    ...(hasStaticProvider && { "ui:widget": "hidden" }),
+  }),
 };
 
 const feedbackRule = {
@@ -459,9 +470,11 @@ const feedbackRule = {
     title: "Host Allocator Feedback Rule",
     oneOf: enumSelect(feedbackRuleToCopy),
   },
-  uiSchema: {
+  uiSchema: (hasStaticProvider: boolean) => ({
     "ui:allowDeselect": false,
-  },
+    "ui:data-cy": "feedback-rule-select",
+    ...(hasStaticProvider && { "ui:widget": "hidden" }),
+  }),
 };
 
 const hostsOverallocatedRule = {
@@ -490,7 +503,7 @@ const minimumHosts = {
 const maximumHosts = {
   schema: {
     type: "number" as "number",
-    title: "Maxiumum Number of Hosts Allowed",
+    title: "Maximum Number of Hosts Allowed",
     minimum: 0,
   },
   uiSchema: (hasEC2Provider: boolean) => ({
@@ -502,7 +515,7 @@ const maximumHosts = {
 const acceptableHostIdleTime = {
   schema: {
     type: "number" as "number",
-    title: "Acceptable Host Idle Time (s)",
+    title: "Acceptable Host Idle Time (ms)",
     minimum: 0,
   },
   uiSchema: (hasEC2Provider: boolean) => ({
@@ -614,11 +627,11 @@ export const allocation = {
     acceptableHostIdleTime: acceptableHostIdleTime.schema,
     futureHostFraction: futureHostFraction.schema,
   },
-  uiSchema: (hasEC2Provider: boolean) => ({
+  uiSchema: (hasEC2Provider: boolean, hasStaticProvider: boolean) => ({
     "ui:ObjectFieldTemplate": CardFieldTemplate,
     version: version.uiSchema,
-    roundingRule: roundingRule.uiSchema,
-    feedbackRule: feedbackRule.uiSchema,
+    roundingRule: roundingRule.uiSchema(hasStaticProvider),
+    feedbackRule: feedbackRule.uiSchema(hasStaticProvider),
     hostsOverallocatedRule: hostsOverallocatedRule.uiSchema,
     minimumHosts: minimumHosts.uiSchema(hasEC2Provider),
     maximumHosts: maximumHosts.uiSchema(hasEC2Provider),