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: machine autoscaler documentation #2754

Merged
merged 4 commits into from
Jul 10, 2024

Conversation

dkwon17
Copy link
Contributor

@dkwon17 dkwon17 commented Jul 8, 2024

What does this pull request change?

Adds machine autoscaler documentation.

A new section called Configuring autoscaling has been added:
Screenshot from 2024-07-08 09-45-12

I've also moved the Configuring number of replicas for a Eclipse Che container page to the new Configuring autoscaling section

What issues does this pull request fix or reference?

https://issues.redhat.com/browse/CRW-6590

Specify the version of the product this pull request applies to

Che 7.88 / Dev Spaces 3.15

Pull Request checklist

The author and the reviewers validate the content of this pull request with the following checklist, in addition to the automated tests.

  • Any procedure:
    • Successfully tested.
  • Any page or link rename:
    • The page contains a redirection for the previous URL.
    • Propagate the URL change in:
  • Builds on Eclipse Che hosted by Red Hat.
  • the Validate language on files added or modified step reports no vale warnings.

Signed-off-by: dkwon17 <dakwon@redhat.com>
@dkwon17 dkwon17 requested review from deerskindoll and a team as code owners July 8, 2024 13:51
@dkwon17 dkwon17 requested a review from azatsarynnyy July 8, 2024 13:51
Copy link

github-actions bot commented Jul 8, 2024

🎊 Navigate the preview: https://668d6baa3693aba70a089f74--eclipse-che-docs-pr.netlify.app 🎊

Copy link

github-actions bot commented Jul 8, 2024

Click here to review and test in web IDE: Contribute

@ibuziuk
Copy link
Member

ibuziuk commented Jul 8, 2024

@deerskindoll we need it for 3.15 as well

Copy link
Contributor

@deerskindoll deerskindoll left a comment

Choose a reason for hiding this comment

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

a couple of questions

[id="configuring-autoscaling"]
= Configuring autoscaling

This section describes different aspects of autoscaling for {prod}.
Copy link
Contributor

Choose a reason for hiding this comment

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

which docs do you want to link here? configuring machine autoscaling and configuring number of replicas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I've added them in my latest commit

[id="configuring-machine-autoscaling"]
= Configuring machine autoscaling

If a cluster has been configured to adjust the number of cluster nodes depending on resource needs, additional configuration is required to maintain the seamless operation of workspaces.
Copy link
Contributor

Choose a reason for hiding this comment

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

I've got some bad news. One of us will have to edit this procedure to get rid of the passive voice and replace it with a more action-oriented phrasing (e.g. "additional configuration is required" x "you must add configuration") . It will make the text flow better.

Do you want to do it or should I?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@deerskindoll , I see, I can make a new commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to:

If you configured the cluster to adjust the number of nodes depending on resource needs,
you need additional configuration to maintain the seamless operation of {prod-short} workspaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

That looks great! But now you have to tackle the other instances of passive voice, too. That's why I wrote that one of us has to update the whole procedure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I've made another commit to remove the passive voice, but please feel free to make more changes where appropriate

dkwon17 and others added 3 commits July 8, 2024 11:53
Co-authored-by: Jana Vrbkova <jvrbkova@redhat.com>
Signed-off-by: dkwon17 <dakwon@redhat.com>
Signed-off-by: dkwon17 <dakwon@redhat.com>
Copy link
Contributor

@deerskindoll deerskindoll left a comment

Choose a reason for hiding this comment

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

lgtm


Conversely when a node is being removed, ideally nodes that are running workspace pods should not be evicted by the autoscaler to avoid any interruptions while using the workspace and potentially losing any unsaved data.

== When the autoscaler adds a new node
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
== When the autoscaler adds a new node
.When the autoscaler adds a new node

- FailedScheduling
----

== When the autoscaler removes a node
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
== When the autoscaler removes a node
.When the autoscaler removes a node

@deerskindoll deerskindoll merged commit 3208e6e into eclipse-che:main Jul 10, 2024
5 of 6 checks passed
deerskindoll added a commit that referenced this pull request Jul 10, 2024
* feat: machine autoscaler documentation

Signed-off-by: dkwon17 <dakwon@redhat.com>

* Update modules/administration-guide/pages/configuring-autoscaling.adoc

Co-authored-by: Jana Vrbkova <jvrbkova@redhat.com>

* Fix PR feedback

Signed-off-by: dkwon17 <dakwon@redhat.com>

* Remove passive voice

Signed-off-by: dkwon17 <dakwon@redhat.com>

---------

Signed-off-by: dkwon17 <dakwon@redhat.com>
Co-authored-by: Jana Vrbkova <jvrbkova@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants