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

Impl/app image pull #145

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/apps/console/page-components/app-states.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const defaultApp: AppIn = {
{
image: '',
name: 'container-1',
env: [],
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import useCustomSwr from '~/lib/client/hooks/use-custom-swr';
import { useConsoleApi } from '~/console/server/gql/api-provider';
import { parseNodes } from '~/console/server/r-utils/common';
import { registryHost } from '~/lib/configs/base-url.cjs';
import { Checkbox } from '~/components/atoms/checkbox';
import { plans } from '../../../../new-app/datas';

const valueRender = ({
Expand Down Expand Up @@ -63,6 +64,8 @@ const SettingCompute = () => {
const { values, errors, handleChange, submit, resetValues } = useForm({
initialValues: {
imageUrl: getContainer(0)?.image || '',
imagePullPolicy:
app.spec.containers[activeContIndex]?.imagePullPolicy || 'IfNotPresent',
pullSecret: 'TODO',
cpuMode: app.metadata?.annotations?.[keyconstants.cpuMode] || 'shared',
memPerCpu: app.metadata?.annotations?.[keyconstants.memPerCpu] || 1,
Expand Down Expand Up @@ -131,7 +134,7 @@ const SettingCompute = () => {
? `${values.repoName}:${values.repoImageTag}`
: `${registryHost}/${values.repoAccountName}/${values.repoName}:${values.repoImageTag}`,
name: 'container-0',
imagePullPolicy: 'Always',
imagePullPolicy: val.imagePullPolicy,
resourceCpu:
val.selectionMode === 'quick'
? {
Expand Down Expand Up @@ -261,6 +264,15 @@ const SettingCompute = () => {
}
loading={digestLoading}
/>

<Checkbox
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Consistency in the UI is key for a good user experience. The addition of the 'Image Pull Policy' checkbox here, similar to its addition in the new-app compute settings, helps maintain that consistency. Just ensure that the default state and any state changes are correctly managed across different components and routes.

label="Image Pull Policy"
checked={values.imagePullPolicy === 'Always'}
onChange={(val) => {
const imagePullPolicy = val ? 'Always' : 'IfNotPresent';
handleChange('imagePullPolicy')(dummyEvent(imagePullPolicy));
}}
/>
</div>
<div className="flex flex-col">
<div className="flex flex-row gap-lg items-center pb-3xl">
Expand Down Expand Up @@ -309,7 +321,7 @@ const SettingCompute = () => {
);
}}
/>
<div className="flex flex-col gap-md p-2xl rounded border border-border-default">
<div className="flex flex-col gap-md p-2xl rounded border border-border-default bg-surface-basic-default">
<div className="flex flex-row gap-lg items-center">
<div className="bodyMd-medium text-text-default">
Select CPU
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const SettingCompute = () => {
/>
{values.autoscaling ? (
<div className="flex flex-col gap-3xl">
<div className="flex flex-col gap-md p-2xl rounded border border-border-default">
<div className="flex flex-col gap-md p-2xl rounded border border-border-default bg-surface-basic-default">
<div className="flex flex-row gap-lg items-center">
<div className="bodyMd-medium text-text-default">
Select min and max replicas
Expand All @@ -125,55 +125,56 @@ const SettingCompute = () => {
max={10}
value={[values.minReplicas, values.maxReplicas]}
onChange={(value) => {
console.log(value);
Copy link

Choose a reason for hiding this comment

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

suggestion (code_refinement): Removal of the console.log statement is a good practice for production code to avoid exposing potentially sensitive information and to keep the console clean. Ensure that all necessary debugging has been completed.

if (Array.isArray(value)) {
handleChange('minReplicas')(dummyEvent(value[0]));
handleChange('maxReplicas')(dummyEvent(value[1]));
}
}}
/>
</div>
<div className="flex flex-col gap-md p-2xl rounded border border-border-default">
<div className="flex flex-row gap-lg items-center">
<div className="bodyMd-medium text-text-default">
Select CPU
<div className="flex flex-row justify-between gap-3xl">
<div className="flex flex-col gap-md p-2xl rounded border border-border-default bg-surface-basic-default w-full">
<div className="flex flex-row gap-lg items-center">
<div className="bodyMd-medium text-text-default">
Select CPU
</div>
<code className="bodyMd text-text-soft flex-1 text-end">
{values.cpuThreshold || 75}% CPU
</code>
</div>
<code className="bodyMd text-text-soft flex-1 text-end">
{values.cpuThreshold || 75}% CPU
</code>
<Slider
step={1}
min={50}
max={95}
value={values.cpuThreshold}
onChange={(value) => {
handleChange('cpuThreshold')(dummyEvent(value));
}}
/>
</div>
<Slider
step={1}
min={50}
max={95}
value={values.cpuThreshold}
onChange={(value) => {
handleChange('cpuThreshold')(dummyEvent(value));
}}
/>
</div>
<div className="flex flex-col gap-md p-2xl rounded border border-border-default">
<div className="flex flex-row gap-lg items-center">
<div className="bodyMd-medium text-text-default">
Select Memory
<div className="flex flex-col gap-md p-2xl rounded border border-border-default bg-surface-basic-default w-full">
<div className="flex flex-row gap-lg items-center">
<div className="bodyMd-medium text-text-default">
Select Memory
</div>
<code className="bodyMd text-text-soft flex-1 text-end">
{values.memoryThreshold || 75}% Memory
</code>
</div>
<code className="bodyMd text-text-soft flex-1 text-end">
{values.memoryThreshold || 75}% Memory
</code>
<Slider
step={1}
min={50}
max={95}
value={values.memoryThreshold}
onChange={(value) => {
handleChange('memoryThreshold')(dummyEvent(value));
}}
/>
</div>
<Slider
step={1}
min={50}
max={95}
value={values.memoryThreshold}
onChange={(value) => {
handleChange('memoryThreshold')(dummyEvent(value));
}}
/>
</div>
</div>
) : (
<div className="flex flex-col gap-md p-2xl rounded border border-border-default">
<div className="flex flex-col gap-md p-2xl rounded border border-border-default bg-surface-basic-default">
<div className="flex flex-row gap-lg items-center">
<div className="bodyMd-medium text-text-default">
Select replicas
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { registryHost } from '~/lib/configs/base-url.cjs';
import { BottomNavigation } from '~/console/components/commons';
import { useOutletContext } from '@remix-run/react';
import { useLog } from '~/root/lib/client/hooks/use-log';
import { Checkbox } from '~/components/atoms/checkbox';
import { plans } from './datas';
import { IProjectContext } from '../../_layout';

Expand Down Expand Up @@ -81,9 +82,17 @@ const AppCompute = () => {
const { values, errors, handleChange, isLoading, submit } = useForm({
initialValues: {
imageUrl: app.spec.containers[activeContIndex]?.image || '',
imagePullPolicy:
app.spec.containers[activeContIndex]?.imagePullPolicy || 'IfNotPresent',
pullSecret: 'TODO',
cpuMode: app.metadata?.annotations?.[keyconstants.cpuMode] || 'shared',
memPerCpu: app.metadata?.annotations?.[keyconstants.memPerCpu] || '1',
autoscaling: app.spec.hpa?.enabled || false,
minReplicas: app.spec.hpa?.minReplicas || 1,
maxReplicas: app.spec.hpa?.maxReplicas || 3,
cpuThreshold: app.spec.hpa?.thresholdCpu || 75,
memoryThreshold: app.spec.hpa?.thresholdMemory || 75,
replicas: app.spec.replicas || 1,

cpu: parseValue(
app.spec.containers[activeContIndex]?.resourceCpu?.max,
Expand Down Expand Up @@ -159,7 +168,7 @@ const AppCompute = () => {
? `${values.repoName}:${values.repoImageTag}`
: `${registryHost}/${values.repoAccountName}/${values.repoName}:${values.repoImageTag}`,
name: 'container-0',
imagePullPolicy: 'Always',
imagePullPolicy: val.imagePullPolicy,
resourceCpu:
val.selectionMode === 'quick'
? {
Expand All @@ -184,6 +193,14 @@ const AppCompute = () => {
},
},
],
hpa: {
enabled: val.autoscaling,
maxReplicas: val.maxReplicas,
minReplicas: val.minReplicas,
thresholdCpu: val.cpuThreshold,
thresholdMemory: val.memoryThreshold,
},
replicas: val.replicas,
},
}));
},
Expand Down Expand Up @@ -301,6 +318,16 @@ const AppCompute = () => {
}
loading={digestLoading}
/>

<Checkbox
label="Image Pull Policy"
checked={values.imagePullPolicy === 'Always'}
onChange={(val) => {
const imagePullPolicy = val ? 'Always' : 'IfNotPresent';
console.log(imagePullPolicy);
handleChange('imagePullPolicy')(dummyEvent(imagePullPolicy));
}}
Comment on lines +323 to +329
Copy link

Choose a reason for hiding this comment

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

suggestion (code_clarification): The implementation of the 'Image Pull Policy' checkbox is a good addition for user flexibility. However, ensure that the UI clearly communicates the implications of changing this setting to the user, as it can affect the deployment's behavior regarding image updates.

/>
</div>

<div className="flex flex-col">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export const EnvironmentVariables = () => {
submit,
resetValues: resetAppValue,
} = useForm({
initialValues: getContainer().env || [],
Copy link

Choose a reason for hiding this comment

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

suggestion (edge_case_not_handled): This change simplifies the initialization of environment variables by assuming that getContainer().env will always return a valid array. Ensure that the getContainer function or its callers are robust against undefined or null values to prevent runtime errors.

initialValues: getContainer().env,
validationSchema: Yup.array(entry),
onSubmit: (val) => {
setContainer((c) => ({
Expand All @@ -252,15 +252,16 @@ export const EnvironmentVariables = () => {
}, [hasChanges]);

const addEntry = (val: IEnvVariable) => {
const tempVal = val || [];
setValues((v = []) => {
const data = {
key: val.key,
type: val.type,
refName: val.refName || '',
refKey: val.refKey || '',
value: val.value || '',
key: tempVal.key,
type: tempVal.type,
refName: tempVal.refName || '',
refKey: tempVal.refKey || '',
value: tempVal.value || '',
};
return [...v, data];
return [...(v || []), data];
});
};

Expand Down Expand Up @@ -312,8 +313,6 @@ export const EnvironmentVariables = () => {
}),
}),
onSubmit: () => {
console.log(eValues);

if (eValues.textInputValue) {
const ev: IEnvVariable = {
key: eValues.key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ const AppReview = () => {

useEffect(() => {
const res = validateType(app, 'AppIn');
// console.log('res', res);
setErrors(res);
}, []);

Expand Down
Loading