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

Adding demo for namespace recommendations #86

Merged
merged 13 commits into from
Oct 1, 2024

Conversation

shekhar316
Copy link
Contributor

@shekhar316 shekhar316 commented Jul 23, 2024

Namespace Recommendations Demo

Image: http://quay.io/rh-ee-shesaxen/autotune/kruize-namespacetest-v5

Note:

  • Run local_monitoring_namespace_demo script to create experiment and generate recommendations for namespace.

@shekhar316 shekhar316 self-assigned this Jul 23, 2024
@shekhar316
Copy link
Contributor Author

@chandrams created a different script for namespace demo and to use test-multiple-import namespace.

@chandrams
Copy link
Contributor

In the recommendations json output, can you remove the namespace name as it repeated and use underscore in namespaceRecommendations to keep it consistent

  "kubernetes_objects": [
      {
        "namespace": "test-multiple-import",
        "namespaceAPIObject": {
          "namespace_name": "test-multiple-import",
          "namespaceRecommendations": {

@shekhar316
Copy link
Contributor Author

shekhar316 commented Jul 25, 2024

In the recommendations json output, can you remove the namespace name as it repeated and use underscore in namespaceRecommendations to keep it consistent

  "kubernetes_objects": [
      {
        "namespace": "test-multiple-import",
        "namespaceAPIObject": {
          "namespace_name": "test-multiple-import",
          "namespaceRecommendations": {

Updated the format according to the document.

@chandrams
Copy link
Contributor

@shekhar316 - Can you resolve the conflicts & update documentation

@dinogun
Copy link
Contributor

dinogun commented Sep 26, 2024

@shekhar316 Please fix the conflicts

@shekhar316
Copy link
Contributor Author

@shekhar316 Please fix the conflicts

Resolved the conflicts.

@dinogun
Copy link
Contributor

dinogun commented Sep 27, 2024

@shekhar316 I would suggest to not have a separate script for namespace, it is better if this is part of the local_monitoring one itself

Signed-off-by: Shekhar Saxena <[email protected]>
@shekhar316
Copy link
Contributor Author

@shekhar316 I would suggest to not have a separate script for namespace, it is better if this is part of the local_monitoring one itself

Sure, I have updated the demo to use the single script.

Copy link
Contributor

@dinogun dinogun left a comment

Choose a reason for hiding this comment

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

LGTM

@dinogun dinogun merged commit d0cc702 into kruize:main Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants