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

Patch the init container with compute resources #85

Merged
merged 3 commits into from
Jun 1, 2024

Conversation

feizhe1996
Copy link
Contributor

Fix for #83.
Previously, the compute resources specified in values.yaml did not get passed down to the initContainer specs. When the namespace is guarded by a resource quota, each container needs to specify the resource requirement, otherwise, k8s throws quota errors.

Since the resources definition is a multi-line string block, the fix tries to pass a json formatted string as an argument to the cli.

@feizhe1996
Copy link
Contributor Author

cc: @yonatankahana for review

@codecov-commenter
Copy link

codecov-commenter commented Sep 19, 2023

Codecov Report

Patch coverage: 63.41% and project coverage change: +0.40% 🎉

Comparison is base (5e1f816) 67.66% compared to head (90da7fa) 68.07%.

❗ Current head 90da7fa differs from pull request most recent head 8d7d8c3. Consider uploading reports for the commit 8d7d8c3 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
+ Coverage   67.66%   68.07%   +0.40%     
==========================================
  Files           6        6              
  Lines         838      855      +17     
==========================================
+ Hits          567      582      +15     
- Misses        234      236       +2     
  Partials       37       37              
Files Changed Coverage Δ
pkg/admission/admission.go 63.59% <61.90%> (+0.03%) ⬆️
pkg/inject/inject.go 93.33% <65.00%> (-0.03%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@feizhe1996 feizhe1996 force-pushed the patch_compute_resources branch 2 times, most recently from 620f1fc to 8d7d8c3 Compare September 21, 2023 15:13
pkg/inject/inject.go Outdated Show resolved Hide resolved
pkg/inject/inject.go Outdated Show resolved Hide resolved
charts/k8tz/templates/controller.yaml Outdated Show resolved Hide resolved
@yonatankahana
Copy link
Member

ping @feizhe1996

@feizhe1996
Copy link
Contributor Author

ping @feizhe1996

Ack

@feizhe1996
Copy link
Contributor Author

Ping @yonatankahana for review

@feizhe1996 feizhe1996 force-pushed the patch_compute_resources branch 2 times, most recently from bdf5805 to 31b9d48 Compare April 23, 2024 17:00
Fix for k8tz#83.
Previously, the compute resources specified in values.yaml did not get passed down to the initContainer specs. When the namespace is guarded by a resource quota, each container needs to specify the resource requirement, otherwise, k8s throws quota errors.

Since the resources definition is a multi-line string block, the fix tries to pass a json formatted string as an argument to the cli.
@feizhe1996
Copy link
Contributor Author

ping @yonatankahana for review

@jiriksykora
Copy link

we are still waiting for mr

@feizhe1996
Copy link
Contributor Author

we are still waiting for mr

ping @yonatankahana again

Copy link
Member

@yonatankahana yonatankahana left a comment

Choose a reason for hiding this comment

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

hi, sorry for my late responses. please fix this small things and let's release it this weekend.

charts/k8tz/values.yaml Outdated Show resolved Hide resolved
charts/k8tz/values.yaml Outdated Show resolved Hide resolved
@feizhe1996 feizhe1996 force-pushed the patch_compute_resources branch 2 times, most recently from 472d0a5 to 9b7148a Compare June 1, 2024 00:54
…ontainerResources param in helm chart's readme
@feizhe1996
Copy link
Contributor Author

Commit updated cc: @yonatankahana

@yonatankahana yonatankahana merged commit b9dfb1d into k8tz:master Jun 1, 2024
2 of 3 checks passed
kireque pushed a commit to kireque/home-ops that referenced this pull request Jun 1, 2024
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [k8tz](http://k8tz.io) ([source](https://togithub.com/k8tz/k8tz)) |
patch | `0.16.0` -> `0.16.1` |

---

### Release Notes

<details>
<summary>k8tz/k8tz (k8tz)</summary>

###
[`v0.16.1`](https://togithub.com/k8tz/k8tz/blob/HEAD/CHANGELOG.md#0161)

- Add possibility to set init container resources
([k8tz/k8tz#85)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zODUuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM4NS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9oZWxtIiwidHlwZS9wYXRjaCJdfQ==-->

Co-authored-by: kireque-bot[bot] <143391978+kireque-bot[bot]@users.noreply.github.com>
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.

None yet

4 participants