kthcloud / console

kthcloud web console
https://cloud.cbh.kth.se
MIT License
3 stars 3 forks source link

Specs being set to min if greater than min #278

Closed Phillezi closed 1 month ago

Phillezi commented 1 month ago

If a user has a resource with specs greater than the minimum it will be set to the minimum

To Reproduce Steps to reproduce the behavior:

  1. Go to edit a vm with specs greater than the minimum (I think the same is true for deployments too)
  2. Scroll down to the specs
  3. See the minimum specs being displayed.

Screenshots Here is how It looks like for me when checking my VM

image The actual specs from the api image

Desktop:

Caused By This is caused by the initial states of the maxCpu and maxRam being set to the min values:

const [maxCpu, setMaxCpu] = useState<number>(
    resource.type === "vm" ? MIN_CPU_VM : MIN_CPU_DEPLOYMENT
);
const [maxRam, setMaxRam] = useState<number>(
    resource.type === "vm" ? MIN_RAM_VM : MIN_RAM_DEPLOYMENT
);
const [maxReplicas, setMaxReplicas] = useState<number>(MIN_REPLICAS);

Since this useEffect is called on load and updates the cpu and ram:

 useEffect(
    () => {
      if (resource.type === "deployment") {
        const d = resource as Deployment;
        setCpu(d.cpuCores);
        setRam(d.ram);
        setReplicas(d.replicas);
        return;
      }
      if (resource.type === "vm" && resource.specs) {
        const v = resource as Vm;
        resource.specs.cpuCores && setCpu(v.specs!.cpuCores!);
        resource.specs.ram && setRam(v.specs!.ram!);
        return;
      }
    },
    // eslint-disable-next-line react-hooks/exhaustive-deps
    []
  );

Causing the cap values to max/min useEffect to update, (cpu and ram in dependency array):

 // Cap values to max/min
  useEffect(
    () => {
      if (resource.type === "vm") {
        if (ram < MIN_RAM_VM) setRam(MIN_RAM_VM);
        if (cpu < MIN_CPU_VM) setCpu(MIN_CPU_VM);
      }
      if (resource.type === "deployment") {
        if (ram < MIN_RAM_DEPLOYMENT) setRam(MIN_RAM_DEPLOYMENT);
        if (cpu < MIN_CPU_DEPLOYMENT) setCpu(MIN_CPU_DEPLOYMENT); /
        if (replicas < MIN_REPLICAS) setReplicas(MIN_REPLICAS);
      }

      // Admins can bypass max values
      if (user && user.admin) return;
      if (ram > maxRam) setRam(maxRam); // since maxRam is set to min initially this gets triggerd
      if (cpu > maxCpu) setCpu(maxCpu); // since maxCpu is set to min initially this gets triggerd
      if (replicas > maxReplicas) setReplicas(maxReplicas); // since minReplicas is set to min initially this gets triggerd
    },
    // eslint-disable-next-line react-hooks/exhaustive-deps
    [cpu, ram, replicas, maxCpu, maxRam, maxReplicas]
  );

The max values are only set to the maximum values of 20 later in the code in this useEffect call:

// Set max values
  useEffect(() => {
    setMaxCpu(20);
    setMaxRam(20);
    setMaxReplicas(20);

    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [resource]);

I have solved this here: my solution

pierrelefevre commented 1 month ago

Great work, thanks for noticing that bug and submitting a PR :D

Phillezi commented 1 month ago

Thx :)