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

feat: add MaxGrowthRate to limit the number of instances added in parallel #962

Merged
merged 3 commits into from
Sep 28, 2023
Merged

feat: add MaxGrowthRate to limit the number of instances added in parallel #962

merged 3 commits into from
Sep 28, 2023

Conversation

rohaquinlop
Copy link
Contributor

@rohaquinlop rohaquinlop commented Sep 18, 2023 •
edited by kayman-mk
Loading

Description

This feature allows to support MaxGrowthRate which limits the number of instances added in parallel by the Runner. The default of 0 (unlimited) is used if no value is given.

The parameter is added to runner_worker_docker_machine_instance as this is the only Runner type for which it makes sense. All other types are not able to scale and thus it is not a runner_worker parameter.

See advanced-configuration

Closes #851

Migrations required

NO

Verification

  • do not set the parameter --> MaxGrowthRate in config file is 0.
  • set the parameter to any value --> MaxGrowthRate in config file is "any value"

@github-actions
Copy link
Contributor

Hey @rohaquinlop! 👋

Thank you for your contribution to the project. Please refer to the contribution rules for a quick overview of the process.

Make sure that this PR clearly explains:

  • the problem being solved
  • the best way a reviewer and you can test your changes

With submitting this PR you confirm that you hold the rights of the code added and agree that it will published under this LICENSE.

The following ChatOps commands are supported:

  • /help: notifies a maintainer to help you out

Simply add a comment with the command in the first line. If you need to pass more information, separate it with a blank line from the command.

This message was generated automatically. You are welcome to improve it.

@dsalaza4
Copy link

Hi @npalm @kayman-mk,

A peer review here would be greatly appreaciated! 🙏🏽

@kayman-mk
Copy link
Collaborator

Seems that the formatting is broken. Could you please run a terraform fmt -recursive to fix that?

@kayman-mk
Copy link
Collaborator

Looks good at first hand. But I would prefer not to mention this parameter in the examples as it is quite specific and an advanced configuration.

@rohaquinlop rohaquinlop force-pushed the support-max-growth-rate branch from 712241e to 7a45a06 Compare September 20, 2023 14:36
@rohaquinlop
Copy link
Contributor Author

@kayman-mk already added your suggestions, thanks in advance for your help!

@kayman-mk
Copy link
Collaborator

Did you test the change without specifying the MaxGrowthRate?

@kayman-mk
Update versions.tf
2faeefc 
align `versions.tf` with lock file
@rohaquinlop
Copy link
Contributor Author

rohaquinlop commented Sep 21, 2023 •
edited
Loading

@kayman-mk No, I was trying to test it but I don't understand completely the documentation. I'm still trying to figure out how to run the tests.

@kayman-mk
Copy link
Collaborator

If you are talking about the tests/ directory: Don't waste your time. This is still work in progress. I test my PRs manually by deploying them.

@kayman-mk
Copy link
Collaborator

Ok, just checked, that the parameter is set as expected. Works fine.

@kayman-mk kayman-mk changed the title feat: add support MaxGrowthRate feat: add MaxGrowthRate to limit the number of instances added in parallel Sep 28, 2023
kayman-mk
kayman-mk approved these changes Sep 28, 2023
View reviewed changes
Copy link
Collaborator

@kayman-mk kayman-mk left a comment

Choose a reason for hiding this comment

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

Good job! Thanks for adding this feature.

@kayman-mk kayman-mk merged commit ae6d38a into cattle-ops:main Sep 28, 2023
22 checks passed
@cattle-ops-releaser-2 cattle-ops-releaser-2 bot mentioned this pull request Sep 28, 2023
Merged
kayman-mk pushed a commit that referenced this pull request Sep 28, 2023
@cattle-ops-releaser-2 @github-actions
chore(main): release 7.1.0 (#972)
5d3f681 
🤖 I have created a release *beep* *boop*
---


##
[7.1.0](7.0.0...7.1.0)
(2023-09-28)


### Features

* add `MaxGrowthRate` to limit the number of instances added in parallel
([#962](#962))
([ae6d38a](ae6d38a))


### Bug Fixes

* convert the fleet instance type in migration script
([#975](#975))
([51b2842](51b2842))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Signed-off-by: Niek Palm <dev.npalm@gmail.com>
Co-authored-by: cattle-ops-releaser-2[bot] <134548870+cattle-ops-releaser-2[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@dsalaza4
Copy link

@kayman-mk Thank you so much for all the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kayman-mk kayman-mk kayman-mk approved these changes

@npalm npalm Awaiting requested review from npalm npalm is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support MaxGrowthRate
3 participants
@rohaquinlop @dsalaza4 @kayman-mk