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(KONFLUX-1503): Set some requests and limits so pods can spread across cluster nodes #1247

Merged
merged 3 commits into from
Aug 12, 2024

Conversation

jhutar
Copy link
Contributor

@jhutar jhutar commented Aug 5, 2024

@jhutar jhutar changed the title WIP: feat(KONFLUX-1503): Set some requests and limits so pods can spread across cluster nodes feat(KONFLUX-1503): Set some requests and limits so pods can spread across cluster nodes Aug 5, 2024
Copy link
Member

@arewm arewm left a comment

Choose a reason for hiding this comment

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

While I do not know about the changes to the computeResources specifically, I don't have an issue with this change if it will improve the behavior and performance of the clusters (i.e. improved auto-scaling).

Copy link

@hugares hugares left a comment

Choose a reason for hiding this comment

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

/lgtm

@hugares
Copy link

hugares commented Aug 6, 2024

@jhutar Can you rebase the PR

@jhutar
Copy link
Contributor Author

jhutar commented Aug 7, 2024

Fixed a typo found by PR checks

Copy link
Contributor

@rcerven rcerven left a comment

Choose a reason for hiding this comment

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

please add resource request only to the task and not steps for sake of all us :)

@jhutar
Copy link
Contributor Author

jhutar commented Aug 12, 2024

please add resource request only to the task and not steps for sake of all us :)

Hello @rcerven . For sake of me, please could you give more info on why? I assume you meant this functionality: https://tekton.dev/docs/pipelines/compute-resources/#task-level-compute-resources-configuration but it is marked as Beta, so not sure if it is OK to use it. I think it is better to use more granular settings. You are concerned about maintainability of these settings?

If I read the docs for Task-level resources feature correctly:

If users specify a Task-level resource limit, no Step may use more than that amount of resources.

all the containers will run with whatever I configure on Task-level, so if I would use 1/2Gi requests (as that is biggest one used for build step), it will run all the containers with that, these 9 build pod containers would account for 9/18Gi requests that might be a significant hit for cluster resources (at least compared to what we have now). With limits, we would run out of node size.

I'm happy to do so as it will just mean more compute plane nodes created by node autoscaler and better distribution of tasks, but I thought what I propose is already big change. Also I would like to be sure it is a general consensus before doing these changes.

@jhutar
Copy link
Contributor Author

jhutar commented Aug 12, 2024

FYI rebased.

@hugares hugares added this pull request to the merge queue Aug 12, 2024
Merged via the queue into konflux-ci:main with commit 43cf5b8 Aug 12, 2024
9 checks passed
msugakov added a commit to stackrox/stackrox that referenced this pull request Oct 10, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
msugakov added a commit to stackrox/scanner that referenced this pull request Oct 10, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
msugakov added a commit to stackrox/collector that referenced this pull request Oct 10, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
msugakov added a commit to stackrox/scanner that referenced this pull request Oct 10, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
msugakov added a commit to stackrox/stackrox that referenced this pull request Oct 10, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
msugakov added a commit to stackrox/stackrox that referenced this pull request Oct 14, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
msugakov added a commit to stackrox/stackrox that referenced this pull request Oct 16, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
msugakov added a commit to stackrox/stackrox that referenced this pull request Oct 17, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
msugakov added a commit to stackrox/stackrox that referenced this pull request Oct 17, 2024
Let the Konflux team manage these resources as they seem to be on
track of doing that
konflux-ci/build-definitions#1247
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.

6 participants