search

project-codeflare / instascale

On-demand Kubernetes/OpenShift cluster scaling and aggregated resource provisioning
Apache License 2.0
10 stars 20 forks source link

Machines not scaling due to ID character limit #182

Closed VanillaSpoon closed 9 months ago

VanillaSpoon commented 11 months ago

Issue link

closes https://github.com/project-codeflare/instascale/issues/143

What changes have been made

This Pr is for 320e5f0e47e0a0211ecc8765930b3ef4ff2ff46a and will be rebased after Hypershift Integration has been merged.

This pr contains an update to the nodepools, and machinepool ID generation convention for instascale. Replacing the appended instance-type with a string of 4 random characters.

The prefix is the appwrapper name (Truncated if over the character limit when appended to the suffix, Nodepools 15, MachinePools 30).

This change also updates the scaledown functions to scaledown machines with a label pointing to the related appwrapper (composed of the appwrapper name, and namespace), as apposed to the machine ID.

These changes also rely on the codeflare-operator e2e tests being updated.

Verification steps

Dispatch an appwrapper (with a long name :) ) on hypershift and OSD with instascale enabled and a specified instance type.

Ensure the machine scales up as expected, and the machine name is truncated to a suitable character length. Ensure the machine scales down once complete.

Checks

VanillaSpoon commented 11 months ago

Thanks for your review and feedback @Fiona-Waters :)

I have pushed updates to the logging within the hypershift pr

Fiona-Waters commented 11 months ago

Thanks for your review and feedback @Fiona-Waters :)

I have pushed updates to the logging within the hypershift pr

You're welcome. Has it been retested on OSD?

VanillaSpoon commented 11 months ago

Hi Fiona, This has be retested on OSD and Hypershift, the errors are handled in a more graceful manner now. Thanks for the feedback. The only commit for this pr is add: naming convention adjustments to nodepools, and machinepools

However, I have rebased again with the others for testing purposes :)

Fiona-Waters commented 9 months ago

Just successfully ran this while testing the nodepool e2e. /lgtm

astefanutti commented 9 months ago

/lgtm

astefanutti commented 9 months ago

/approve

openshift-ci[bot] commented 9 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astefanutti

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/project-codeflare/instascale/blob/main/OWNERS)~~ [astefanutti] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment