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

[loki-distributed] Add minAvailable poddisruptionbudget #3510

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

[loki-distributed] Add minAvailable poddisruptionbudget #3510

wants to merge 7 commits into from

Conversation

stafot
Copy link
Contributor

@stafot stafot commented Jan 3, 2025

Reasoning:
In our use case we need to be able to customize minAvailable value in podDisruptionBudgets.

@stafot stafot force-pushed the loki-add-pod-disruption-budgets-customized branch from e16e389 to 1b83c11 Compare January 3, 2025 10:23
@stafot stafot changed the title Add minAvailable poddisruptionbudget in loki-distributed [loki-distributed] Add minAvailable poddisruptionbudget Jan 3, 2025
@stafot stafot requested review from torstenwalter, zanhsieh and a team as code owners January 3, 2025 10:25
@stafot stafot force-pushed the loki-add-pod-disruption-budgets-customized branch 2 times, most recently from 980cfb4 to 0486999 Compare January 3, 2025 10:43
@stafot
Copy link
Contributor Author

stafot commented Jan 3, 2025

Lint fails because helm-docs wants to change other charts, but validate wants 1 PR per chart which is reasonable. So some code was merged recently without proper documentation update that seems to leading to this problem. @unguiculus let me know what is your process to handle this and I can work on a fix.

@stafot stafot force-pushed the loki-add-pod-disruption-budgets-customized branch from 61932aa to 784debd Compare January 7, 2025 10:24
zanhsieh
zanhsieh requested changes Jan 17, 2025
View reviewed changes
.github/workflows/linter.yml Outdated Show resolved Hide resolved
stafot and others added 4 commits January 20, 2025 17:51
@stafot
Add minAvailable poddisruptionbudget in loki-distributed
141b627 
Signed-off-by: Stavros Foteinopoulos <stafot@gmail.com>
@stafot
update docs
041855e 
Signed-off-by: Stavros Foteinopoulos <stafot@gmail.com>

Update helm-docs

Signed-off-by: Stavros Foteinopoulos <stafot@gmail.com>
@zanhsieh @stafot
Update README.md
be40ac4 
Signed-off-by: MH <zanhsieh@gmail.com>
@stafot
revert helm-docs change to do a separate PR
c80d51b 
Signed-off-by: Stavros Foteinopoulos <stafot@gmail.com>
@stafot stafot force-pushed the loki-add-pod-disruption-budgets-customized branch from 418399c to c80d51b Compare January 20, 2025 15:51
@stafot
update docs
ee57e81 
Signed-off-by: Stavros Foteinopoulos <stafot@gmail.com>
@stafot stafot mentioned this pull request Jan 20, 2025
Open
@stafot stafot requested a review from zanhsieh January 21, 2025 09:18
@zanhsieh
Copy link
Collaborator

@stafot
Can you fix helm doc generated README.md plz?

@stafot
Copy link
Contributor Author

stafot commented Jan 21, 2025 •
edited
Loading

@zanhsieh The result is what the helm-docs tool produces. So you want me to change it manually to be what the existing one expects? I believe that if we merge the other PR with the helm-docs update might fix this, but not 100% sure. Let me know if you want to manually fix with the desired state or to wait until the other PR is merged rebase and see if this fixes the issue.

@zanhsieh
Copy link
Collaborator

@stafot
In order to change the result of helmdoc generated README.md, you have to change the comments in values.yaml then run helmdoc again. Not directly change README.md.

@stafot
Copy link
Contributor Author

stafot commented Jan 21, 2025

@zanhsieh done

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

@zanhsieh zanhsieh zanhsieh approved these changes

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

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

@torstenwalter torstenwalter Awaiting requested review from torstenwalter

At least 2 approving reviews are required to merge this pull request.

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

Successfully merging this pull request may close these issues.
2 participants
@stafot @zanhsieh