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

deployment: Adapt values to be quoted when installed in templates #358

Merged
merged 1 commit into from
Oct 27, 2023
Merged

deployment: Adapt values to be quoted when installed in templates #358

merged 1 commit into from
Oct 27, 2023

Conversation

ardumont
Copy link
Contributor

@ardumont ardumont commented Sep 29, 2023 •
edited
Loading

(initial title: deployment: Adapt 'workerThreads' to be a string)

There is a check in the main.go module [1] which expects it to be a string so it can convert it into an integer. Without this, we cannot actually provide the configuration without hitting a conversion error. This should fix it.

Currently, we are working around it by provided a double quoted string "'8'".

[1]

local-path-provisioner/main.go

Lines 254 to 257 in 4d42c70

workerThreads := c.Int(FlagWorkerThreads)
if workerThreads <= 0 {
return fmt.Errorf("invalid zero or negative integer flag %v", FlagWorkerThreads)
}

@mircea-pavel-anton
Copy link

I think the same problem is present for the deletionRetryCount and provisioningRetryCount fields as well. Maybe we should fix those in this PR as well?

@derekbit
Copy link
Member

I think the same problem is present for the deletionRetryCount and provisioningRetryCount fields as well. Maybe we should fix those in this PR as well?

@mirceanton
Agree.

@ardumont
LGTM. Thanks for your contribution. Can you help fix deletionRetryCount and provisioningRetryCount as well? Many thanks.

@ardumont
deployment: Adapt values to be quoted when installed in templates
5501eb4 
Namely the values 'workerThreads', 'deletionRetryCount' and 'provisionRetryCount'.

There is some check in the main.go module which expects those to be a string so it can
convert them into an integer. Without this, we cannot actually provide the configuration
without hitting a conversion error. This should fix it.
@ardumont
Copy link
Contributor Author

LGTM. Thanks for your contribution. Can you help fix deletionRetryCount and provisioningRetryCount as well? Many thanks.

Sure, done.
I've rebased the PR on the latest master too.

Cheers,

@ardumont ardumont changed the title deployment: Adapt 'workerThreads' to be a string deployment: Adapt values to be quoted when installed in templates Oct 27, 2023
derekbit
derekbit approved these changes Oct 27, 2023
View reviewed changes
Copy link
Member

@derekbit derekbit left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@derekbit derekbit merged commit 7aa7e28 into rancher:master Oct 27, 2023
coolguy1771 referenced this pull request in coolguy1771/home-ops Nov 22, 2023
@lumiere-bot
fix(github-release): update local-path-provisioner to v0.0.25 (#3675)
9042e38 
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
|
[local-path-provisioner](https://togithub.com/rancher/local-path-provisioner)
| patch | `v0.0.24` -> `v0.0.25` |

---

### Release Notes

<details>
<summary>rancher/local-path-provisioner
(local-path-provisioner)</summary>

###
[`v0.0.25`](https://togithub.com/rancher/local-path-provisioner/releases/tag/v0.0.25):
Local Path Provisioner v0.0.25

[Compare
Source](https://togithub.com/rancher/local-path-provisioner/compare/v0.0.24...v0.0.25)

##### What's Changed

- feat - Node name annotation by
[@&#8203;osherElm](https://togithub.com/osherElm) in
[https://github.com/rancher/local-path-provisioner/pull/340](https://togithub.com/rancher/local-path-provisioner/pull/340)
- Helm: add namespace by
[@&#8203;sergelogvinov](https://togithub.com/sergelogvinov) in
[https://github.com/rancher/local-path-provisioner/pull/325](https://togithub.com/rancher/local-path-provisioner/pull/325)
- Add storageClass.volumeBindingMode value to helm chart by
[@&#8203;anisimovdk](https://togithub.com/anisimovdk) in
[https://github.com/rancher/local-path-provisioner/pull/347](https://togithub.com/rancher/local-path-provisioner/pull/347)
- Helm: add pod annotation, securityContext blocks by
[@&#8203;sergelogvinov](https://togithub.com/sergelogvinov) in
[https://github.com/rancher/local-path-provisioner/pull/321](https://togithub.com/rancher/local-path-provisioner/pull/321)
- Set default volume type of storage class from Helm chart by
[@&#8203;nltimv](https://togithub.com/nltimv) in
[https://github.com/rancher/local-path-provisioner/pull/357](https://togithub.com/rancher/local-path-provisioner/pull/357)
- Constraint default RBAC permissions by
[@&#8203;sbocinec](https://togithub.com/sbocinec) in
[https://github.com/rancher/local-path-provisioner/pull/336](https://togithub.com/rancher/local-path-provisioner/pull/336)
- Helm: add pod tolerations to the helper-pod by
[@&#8203;sergelogvinov](https://togithub.com/sergelogvinov) in
[https://github.com/rancher/local-path-provisioner/pull/322](https://togithub.com/rancher/local-path-provisioner/pull/322)
- deployment: Adapt values to be quoted when installed in templates by
[@&#8203;ardumont](https://togithub.com/ardumont) in
[https://github.com/rancher/local-path-provisioner/pull/358](https://togithub.com/rancher/local-path-provisioner/pull/358)
- Save the helper pod logs to the provisioner logs by
[@&#8203;js185692](https://togithub.com/js185692) in
[https://github.com/rancher/local-path-provisioner/pull/324](https://togithub.com/rancher/local-path-provisioner/pull/324)
- Add ReadWriteOncePod support for NodePath volumes by
[@&#8203;anothertobi](https://togithub.com/anothertobi) in
[https://github.com/rancher/local-path-provisioner/pull/331](https://togithub.com/rancher/local-path-provisioner/pull/331)
- feature: Add configuretion to run setup/teardown command in helper
container instead of run with script by
[@&#8203;name212](https://togithub.com/name212) in
[https://github.com/rancher/local-path-provisioner/pull/360](https://togithub.com/rancher/local-path-provisioner/pull/360)
- Helm: Add common labels value and update uninstall for helm 3 by
[@&#8203;kevinzwang](https://togithub.com/kevinzwang) in
[https://github.com/rancher/local-path-provisioner/pull/319](https://togithub.com/rancher/local-path-provisioner/pull/319)

</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:eyJjcmVhdGVkSW5WZXIiOiIzNy42MS40IiwidXBkYXRlZEluVmVyIjoiMzcuNjMuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: lumiere-bot <98047013+lumiere-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekbit derekbit derekbit approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@ardumont @mircea-pavel-anton @derekbit