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.

Sign up for GitHub

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

Jump to bottom

OSDOCS:11739 add support for cron scaler in CMA #80708

Merged
merged 1 commit into from
Aug 26, 2024
Merged

OSDOCS:11739 add support for cron scaler in CMA #80708

merged 1 commit into from
Aug 26, 2024

Conversation

mburke5678
Copy link
Contributor

@mburke5678 mburke5678 commented Aug 20, 2024 •
edited
Loading

https://issues.redhat.com/browse/OSDOCS-11739

Preview:
Understanding the cron trigger

QE review:

  • QE has approved this change.

@mburke5678 mburke5678 added this to the Continuous Release milestone Aug 20, 2024
@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 20, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Aug 20, 2024 •
edited
Loading

🤖 Mon Aug 26 16:59:53 - Prow CI generated the docs preview:

https://80708--ocpdocs-pr.netlify.app/openshift-dedicated/latest/nodes/cma/nodes-cma-autoscaling-custom-trigger.html
https://80708--ocpdocs-pr.netlify.app/openshift-enterprise/latest/nodes/cma/nodes-cma-autoscaling-custom-trigger.html
https://80708--ocpdocs-pr.netlify.app/openshift-rosa/latest/nodes/cma/nodes-cma-autoscaling-custom-trigger.html

@mburke5678 mburke5678 force-pushed the cma-cron-trigger branch 2 times, most recently from f9b266b to f150dc4 Compare August 20, 2024 15:58
@mburke5678
Copy link
Contributor Author

@joelsmith PTAL

@jkyros
Copy link

jkyros commented Aug 22, 2024

Thanks @mburke5678 , looks great!
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 22, 2024
@mburke5678 mburke5678 mentioned this pull request Aug 23, 2024
Merged
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Aug 26, 2024

New changes are detected. LGTM label has been removed.

@mburke5678 mburke5678 added the peer-review-needed Signifies that the peer review team needs to review this PR label Aug 26, 2024
@skopacz1 skopacz1 added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Aug 26, 2024
skopacz1
skopacz1 reviewed Aug 26, 2024
View reviewed changes
Copy link
Contributor

@skopacz1 skopacz1 left a comment

Choose a reason for hiding this comment

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

Left a few comments but otherwise LGTM!

modules/nodes-cma-autoscaling-custom-trigger-cron.adoc Outdated
The custom metrics autoscaler with cron trigger scales pods based only on the specified times and cannot scale pods on a daily, weekly, monthly, or yearly schedule.
====

The following example scales the pods associated with this scaled object from `0` to `100` from 6:00 a.m to 6:30 p.m India Standard Time.
Copy link
Contributor

Choose a reason for hiding this comment

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

Per IBM word usage (AM and PM), the times should be in caps and without periods:

Suggested change
The following example scales the pods associated with this scaled object from `0` to `100` from 6:00 a.m to 6:30 p.m India Standard Time.
The following example scales the pods associated with this scaled object from `0` to `100` from 6:00 AM to 6:30 PM India Standard Time.

Copy link
Contributor Author

@mburke5678 mburke5678 Aug 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@skopacz1 Thank you! They must have changed this one on me. I have the IBM style guide in book form, which I did consult here. Ooops. The book is 22 years old. I guess I need a new one!

modules/nodes-cma-autoscaling-custom-trigger-cron.adoc Outdated
Comment on lines 44 to 48
<2> Specifies a cron trigger.
<3> Specifies the timezone for the time frame. This value must be from the link:https://data.iana.org/time-zones/tzdb-2021a/zone1970.tab[IANA Time Zone Database].
<4> Specifies the start of the time frame.
<5> Specifies the end of the time frame.
<6> Specifies the number of pods to scale to between the start and end of the time frame. Should be the same as `maxReplicaCount`.
Copy link
Contributor

Choose a reason for hiding this comment

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

These values all need to be bumped down by 1 since you added a new callout here:

Suggested change
<2> Specifies a cron trigger.
<3> Specifies the timezone for the time frame. This value must be from the link:https://data.iana.org/time-zones/tzdb-2021a/zone1970.tab[IANA Time Zone Database].
<4> Specifies the start of the time frame.
<5> Specifies the end of the time frame.
<6> Specifies the number of pods to scale to between the start and end of the time frame. Should be the same as `maxReplicaCount`.
<3> Specifies a cron trigger.
<4> Specifies the timezone for the time frame. This value must be from the link:https://data.iana.org/time-zones/tzdb-2021a/zone1970.tab[IANA Time Zone Database].
<5> Specifies the start of the time frame.
<6> Specifies the end of the time frame.
<7> Specifies the number of pods to scale to between the start and end of the time frame. Should be the same as `maxReplicaCount`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Too much going on this morning.

modules/nodes-cma-autoscaling-custom-trigger-cron.adoc Outdated
<3> Specifies the timezone for the time frame. This value must be from the link:https://data.iana.org/time-zones/tzdb-2021a/zone1970.tab[IANA Time Zone Database].
<4> Specifies the start of the time frame.
<5> Specifies the end of the time frame.
<6> Specifies the number of pods to scale to between the start and end of the time frame. Should be the same as `maxReplicaCount`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion to make the last sentence complete and resemble the similar statement in the other callout:

Suggested change
<6> Specifies the number of pods to scale to between the start and end of the time frame. Should be the same as `maxReplicaCount`.
<6> Specifies the number of pods to scale to between the start and end of the time frame. This value should be the same as `maxReplicaCount`.

@skopacz1 skopacz1 added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR peer-review-needed Signifies that the peer review team needs to review this PR labels Aug 26, 2024
@asahay19
Copy link

/lgtm , thank you !

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Aug 26, 2024

@mburke5678: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@mburke5678 mburke5678 merged commit 852be4d into openshift:main Aug 26, 2024
2 checks passed
@mburke5678 mburke5678 deleted the cma-cron-trigger branch August 26, 2024 17:05
@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.12

@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.13

@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.14

@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.15

@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.16

@mburke5678
Copy link
Contributor Author

/cherrypick enterprise-4.17

@openshift-cherrypick-robot
Copy link

@mburke5678: new pull request created: #80896

In response to this:

/cherrypick enterprise-4.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

This was referenced Aug 26, 2024
Merged
Merged
@openshift-cherrypick-robot
Copy link

@mburke5678: new pull request created: #80897

In response to this:

/cherrypick enterprise-4.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Aug 26, 2024
Merged
@openshift-cherrypick-robot
Copy link

@mburke5678: new pull request created: #80898

In response to this:

/cherrypick enterprise-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Aug 26, 2024
Merged
@openshift-cherrypick-robot
Copy link

@mburke5678: new pull request created: #80899

In response to this:

/cherrypick enterprise-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot
Copy link

@mburke5678: new pull request created: #80900

In response to this:

/cherrypick enterprise-4.16

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

This was referenced Aug 26, 2024
Merged
Merged
@openshift-cherrypick-robot
Copy link

@mburke5678: new pull request created: #80901

In response to this:

/cherrypick enterprise-4.17

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@mburke5678 mburke5678 mentioned this pull request Aug 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skopacz1 skopacz1 skopacz1 left review comments

Assignees

@jkyros jkyros

Labels
branch/enterprise-4.12 branch/enterprise-4.13 branch/enterprise-4.14 branch/enterprise-4.15 branch/enterprise-4.16 branch/enterprise-4.17 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
Continuous Release
Development

Successfully merging this pull request may close these issues.
6 participants
@mburke5678 @ocpdocs-previewbot @jkyros @asahay19 @openshift-cherrypick-robot @skopacz1