Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

Commit

Permalink
EVG-20995: Improve distro settings validation (#2078)
Browse files Browse the repository at this point in the history
  • Loading branch information
sophstad authored Oct 4, 2023
1 parent b54c968 commit 7c08755
Show file tree
Hide file tree
Showing 17 changed files with 150 additions and 81 deletions.
16 changes: 14 additions & 2 deletions cypress/integration/distroSettings/general_section.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe("general section", () => {
cy.contains("button", "Add alias").click();
cy.getInputByLabel("Alias").type("localhost-alias");
cy.getInputByLabel("Notes").type("this is a note");
cy.getInputByLabel("Disable shallow clone for this distro.").check({
cy.getInputByLabel("Disable shallow clone for this distro").check({
force: true,
});
save();
Expand All @@ -26,7 +26,7 @@ describe("general section", () => {
cy.reload();
cy.getInputByLabel("Alias").should("have.value", "localhost-alias");
cy.getInputByLabel("Notes").should("have.value", "this is a note");
cy.getInputByLabel("Disable shallow clone for this distro.").should(
cy.getInputByLabel("Disable shallow clone for this distro").should(
"be.checked"
);

Expand All @@ -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");
});
});
});
6 changes: 2 additions & 4 deletions cypress/integration/distroSettings/host_section.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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"
Expand All @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions cypress/integration/distroSettings/provider_section.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe("provider section", () => {
force: true,
});
cy.contains("button", "Add security group").click();
cy.getInputByLabel("Security Group ID").type("group-1234");
cy.getInputByLabel("Security Group ID").type("sg-1234");
cy.contains("button", "Add host").click();
cy.getInputByLabel("Name").type("host-1234");
save();
Expand Down Expand Up @@ -153,7 +153,7 @@ describe("provider section", () => {
cy.getInputByLabel("EC2 AMI ID").type("ami-1234");
cy.getInputByLabel("Instance Type").type("m5.xlarge");
cy.contains("button", "Add security group").click();
cy.getInputByLabel("Security Group ID").type("security-group-1234");
cy.getInputByLabel("Security Group ID").type("sg-5678");
save();
cy.validateToast("success", "Updated distro.");

Expand Down Expand Up @@ -230,7 +230,7 @@ describe("provider section", () => {
cy.getInputByLabel("EC2 AMI ID").type("ami-1234");
cy.getInputByLabel("Instance Type").type("m5.xlarge");
cy.contains("button", "Add security group").click();
cy.getInputByLabel("Security Group ID").type("security-group-1234");
cy.getInputByLabel("Security Group ID").type("sg-0000");
save();
cy.validateToast("success", "Updated distro.");

Expand Down
2 changes: 1 addition & 1 deletion cypress/integration/distroSettings/task_section.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe("task section", () => {
});

it("should surface warnings for invalid number inputs", () => {
const inputLabel = "Patch Factor (0 to 100 inclusive)";
const inputLabel = "Patch Factor";
cy.selectLGOption("Task Planner Version", "Tunable");
cy.getInputByLabel(inputLabel).clear();
cy.getInputByLabel(inputLabel).type("500");
Expand Down
5 changes: 5 additions & 0 deletions src/components/SpruceForm/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ export const transformErrors = (errors: AjvError[]) =>
...error,
message: "Please select one of the available options.",
};
case "pattern":
return {
...error,
message: `Field should match pattern ${error.params.pattern}`,
};
case "format":
switch (error.params.format) {
case "noSpaces":
Expand Down
2 changes: 2 additions & 0 deletions src/gql/generated/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,9 @@ export type GroupedBuildVariant = {

export type GroupedFiles = {
__typename?: "GroupedFiles";
execution: Scalars["Int"]["output"];
files?: Maybe<Array<File>>;
taskId: Scalars["String"]["output"];
taskName?: Maybe<Scalars["String"]["output"]>;
};

Expand Down
5 changes: 4 additions & 1 deletion src/pages/distroSettings/Tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 17 additions & 4 deletions src/pages/distroSettings/tabs/GeneralTab/GeneralTab.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,31 @@
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 }) => {
const initialFormState = distroData;
export const GeneralTab: React.FC<TabProps> = ({
distroData,
minimumHosts,
}) => {
const { distroId } = useParams();
const { containerPools } = useSpruceConfig();
const containerPoolDistros =
containerPools?.pools?.map(({ distro }) => distro) ?? [];

const formSchema = useMemo(() => getFormSchema(), []);
const isContainerDistro = containerPoolDistros.includes(distroId);

const formSchema = useMemo(
() => getFormSchema(isContainerDistro, minimumHosts),
[isContainerDistro, minimumHosts]
);

return (
<>
<BaseTab formSchema={formSchema} initialFormState={initialFormState} />
<BaseTab formSchema={formSchema} initialFormState={distroData} />
<SettingsCardTitle>Remove Configuration</SettingsCardTitle>
<SettingsCard>
<DeleteDistro />
Expand Down
61 changes: 30 additions & 31 deletions src/pages/distroSettings/tabs/GeneralTab/getFormSchema.ts
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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;
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -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",
};
8 changes: 3 additions & 5 deletions src/pages/distroSettings/tabs/GeneralTab/transformers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions src/pages/distroSettings/tabs/GeneralTab/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
5 changes: 4 additions & 1 deletion src/pages/distroSettings/tabs/HostTab/getFormSchema.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
),
},
};
};
Expand Down
Loading

0 comments on commit 7c08755

Please sign in to comment.