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

feat: Add binpacking examples #615

Merged
merged 5 commits into from
Sep 19, 2024
Merged

feat: Add binpacking examples #615

merged 5 commits into from
Sep 19, 2024

Conversation

hitsub2
Copy link
Contributor

@hitsub2 hitsub2 commented Aug 19, 2024

What does this PR do?

🛑 Please open an issue first to discuss any significant work and flesh out details/direction - we would hate for your time to be wasted.
Consult the CONTRIBUTING guide for submitting pull-requests.

Issue: 614

Motivation

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Mandatory for new blueprints. Yes, I have added a example to support my blueprint PR
  • Mandatory for new blueprints. Yes, I have updated the website/docs or website/blog section for this feature
  • Yes, I ran pre-commit run -a with this PR. Link for installing pre-commit locally

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

image

image

@hitsub2 hitsub2 changed the title add binpacking examples feat: add binpacking examples Aug 20, 2024
@hitsub2 hitsub2 changed the title feat: add binpacking examples feat: Add binpacking examples Aug 20, 2024
@hitsub2 hitsub2 closed this Aug 20, 2024
@hitsub2 hitsub2 reopened this Aug 20, 2024
Copy link
Collaborator

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

Thanks for the doc @hitsub2 ! Please check the comments.
Could you please elaborate this doc like a blog and talk about the problem, solution and pros and cons of maintaining this custom scheduler. How the upgrades are managed with this scheduler etc.

@@ -0,0 +1,56 @@
---
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you move this file under resources section? https://github.com/awslabs/data-on-eks/tree/main/website/docs/resources

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

```bash
kind: Pod
spec:
schedulerName: my-scheduler
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's give the name as custom-k8s-scheduler

Copy link
Collaborator

Choose a reason for hiding this comment

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

Neuron scheduler deployment is also using the same name :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done



## Deploying the Solution
Please follow [this link](https://github.com/aws-samples/custom-scheduler-eks) to deploy the custom scheduler.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add the deployment step directly here and ask the users to refer to the aws-samples project for more details.
Also, please explain more details of any caution that users needs to be aware while deploying this scheduler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a section called considerations, but indeed we should give the adoption guidance for the users


# Binpacking for Amazon EKS

## Introduction
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's talk little bit more about the problem statement and what is "LeastAllocated" and "MostAllocated". It would be nice to show the difference in images if you have any.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Collaborator

Choose a reason for hiding this comment

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

Move these images also under resources section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

making new folder called img under resources

@hitsub2
Copy link
Contributor Author

hitsub2 commented Aug 21, 2024

Could you please elaborate this doc like a blog and talk about the problem, solution and pros and cons of maintaining this custom scheduler. How the upgrades are managed with this scheduler etc.

yes, thanks for the comment. Will add more details around these.

Copy link
Collaborator

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @hitsub2 👏🏼

Copy link
Collaborator

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

one minor comment

```


## Verfication and Monitor via [eks-node-viewer](https://github.com/awslabs/eks-node-viewer)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fix this spelling

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@vara-bonthu vara-bonthu merged commit a428dbe into awslabs:main Sep 19, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants