Enhance helper Pod interface and configuration. #166

mgoltzsche · 2020-12-30T00:04:45Z

Make the helper Pod receive only environment variables instead of args (this changes the interface in a non-backward compatible way but is simpler to use and potentially provides more backward compatibility in the future; Allow the helper pod to read params from env vars #175).
Adds the manager options --pvc-annotation[-required] to pass through annotations from the PVC to the PV and to the helper Pod (Pass through PVC annotations to the helper Pod as env vars #164).
Merge the helper Pod's data VolumeMount with the one provided with the template to be able to specify mountPropagation within the template (Allow to configure helper Pod's data volume mount propagation #165).
Rename helperPod.yaml to helper-pod.yaml (more convenient and if we break sth we can break this as well).
Expose --helper-pod-timeout option (Make the helper Pod timeout configurable #178).
Provide a basic usage example of the new features (examples/cache).
Support forceful termination of the manager binary (2xctrl+c - since this is annoying during development otherwise; Fix manager termination #179).

Closes #164.
Closes #165.
Closes #175.
Closes #178.
Relates to #179.
Relates to #127.

mgoltzsche · 2020-12-30T00:15:48Z

provisioner.go

-	helperPod.Spec.Containers[0].Args = []string{"-p", filepath.Join(parentDir, volumeDir),
-		"-s", strconv.FormatInt(sizeInBytes, 10),
-		"-m", volumeMode}


This is a breaking change! The helper script options have been replaced by environment variables.

Reading these options within a setup/teardown script is pretty cumbersome as the previous examples show.
Also I'd like to avoid redundant code within these scripts and therefore be able to simply call the setup script from the teardown script as within my cache example - in the absence of a common script that both scripts could use.

Therefore I propose to use environment variables for these parameters instead. Also env vars potentially provide better backward compatibility than options (imagine sb builds a binary using a CLI framework: it would fail by default when new/unknown options are added while, when using env vars, this is not a problem).
I adjusted the examples correspondingly.

Alternatively we could leave the options here for now in addition to the env vars, deprecate the options somehow and remove them later. Though eventually we can remove them now since the feature is pretty young anyway and not many people may rely on it yet.

@nicktming this affects the changes you introduced in #127.

WDYT?

mgoltzsche · 2020-12-30T00:17:41Z

main.go

+	FlagConfigMapName         = "configmap-name"
+	EnvConfigMapName          = "CONFIGMAP_NAME"
+	FlagHelperPodFile         = "helper-pod-file"
+	DefaultHelperPodFile      = "helper-pod.yaml"


This is another breaking change: renamed helperPod.yaml to helper-pod.yaml. I could revert that one but the lower case notation is more convenient for a file name than the camel case notation and if we break sth here already we might as well break this one now.

mgoltzsche · 2021-01-20T00:29:07Z

@yasker please have a look

mgoltzsche · 2021-02-21T23:00:35Z

I just rebased this PR and resolved the conflicts.

innobead · 2021-03-02T17:03:39Z

Thanks for the contribution.

It looks you want to solve several issues or enhancements, so I would suggest to seperate this PR first, so we can focus on each problem you are resolving and it will be better for review.

mgoltzsche · 2021-03-06T18:50:55Z

@innobead okay. Now I created the following separate PRs:

Allow mountPropagation configuration by merging the helper Pod template's volumeMount #176 for the mountPropagation feature (Allow to configure helper Pod's data volume mount propagation #165).
Provide helper Pod params as env vars. #177 to provide params as env vars to the helper Pod (backward compatible this time) (Allow the helper pod to read params from env vars #175).
Support forceful termination on 2nd signal #180 to forcefully terminate the manager on 2xCtrl+c (Fix manager termination #179).

Once #176 and #177 are merged I'll rebase and refine this PR or create another PR for #164 and eventually add the example from this PR to it (using more compact setup/teardown scripts).

* Make the helper Pod receive only environment variables instead of args (this changes the interface in a non-backward compatible way but is simpler to use and potentially provides more backward compatibility in the future). * Adds the manager options `--pvc-annotation[-required]` to pass through annotations from the PVC to the PV and to the helper Pod. * Merge the helper Pod's `data` VolumeMount with the one provided with the template to be able to specify `mountPropagation` within the template. * Rename `helperPod.yaml` to `helper-pod.yaml` (more convenient and if we break sth we can break this as well). * Expose `--helper-pod-timeout` option. * Provide a basic usage example of the new features (`examples/cache`). * Support forceful termination of the manager binary (2xCtrl+c - since this is annoying during development otherwise). Closes rancher#164 Closes rancher#165 Signed-off-by: Max Goltzsche <[email protected]>

* Replaces local-path-provisioner since its maintainers didn't merge any of my PRs, see rancher/local-path-provisioner#166. * Adds manifest that includes an image registry.

LarsBingBong · 2022-03-21T14:49:48Z

In regards to #177 having been merged. I'm getting the following err:

In the local-path helper Pod. I deployed from https://raw.githubusercontent.com/rancher/local-path-provisioner/master/deploy/local-path-storage.yaml as the ReadMe describes - and I did so today.

Sure enough I haven't provided this VOL_DIR parameter anywhere. Where am I to do so? I can't seem to find it described anywhere.

Thank you very much.

derekbit · 2022-03-21T16:16:30Z

In regards to #177 having been merged. I'm getting the following err:

In the local-path helper Pod. I deployed from https://raw.githubusercontent.com/rancher/local-path-provisioner/master/deploy/local-path-storage.yaml as the ReadMe describes - and I did so today.

Sure enough I haven't provided this VOL_DIR parameter anywhere. Where am I to do so? I can't seem to find it described anywhere.

Thank you very much.

Thanks for your finding.

The reason is that the image in master branch's deploy/local-path-storage.yaml is rancher/local-path-provisioner:v0.0.21 rather than rancher/local-path-provisioner:master-head. The image and deploy/local-path-storage.yaml are mismatched. We will fix it later.

You can please use v0.0.21 deploy manifest.
https://github.com/rancher/local-path-provisioner/tree/v0.0.21

LarsBingBong · 2022-03-21T17:28:03Z

Yeah! That made it work! Thank you very much @derekbit

github-actions · 2024-07-27T02:10:25Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2024-08-06T02:14:04Z

This PR was closed because it has been stalled for 10 days with no activity.

mgoltzsche commented Dec 30, 2020

View reviewed changes

mgoltzsche force-pushed the helper-pod-interface-enhancement branch 5 times, most recently from 9201b02 to 0dbf8c9 Compare December 30, 2020 00:51

mgoltzsche mentioned this pull request Dec 30, 2020

ZFS example #162

Closed

mgoltzsche force-pushed the helper-pod-interface-enhancement branch 9 times, most recently from 6320315 to 9818353 Compare December 30, 2020 03:09

mgoltzsche force-pushed the helper-pod-interface-enhancement branch 2 times, most recently from a124294 to e821731 Compare February 21, 2021 22:09

mgoltzsche mentioned this pull request Mar 6, 2021

Allow mountPropagation configuration by merging the helper Pod template's volumeMount #176

Merged

This was referenced Mar 6, 2021

Support forceful termination on 2nd signal #180

Closed

Provide helper Pod params as env vars. #177

Merged

mgoltzsche force-pushed the helper-pod-interface-enhancement branch 2 times, most recently from 8c77288 to 654c7bd Compare March 7, 2021 21:34

mgoltzsche force-pushed the helper-pod-interface-enhancement branch from 654c7bd to 4e7b93f Compare March 7, 2021 21:46

mgoltzsche marked this pull request as draft March 8, 2021 00:44

mgoltzsche marked this pull request as ready for review April 26, 2021 22:35

Donatas-L mentioned this pull request May 28, 2024

Make the helper Pod timeout configurable #178

Open

github-actions bot added the stale label Jul 27, 2024

github-actions bot closed this Aug 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance helper Pod interface and configuration. #166

Enhance helper Pod interface and configuration. #166

mgoltzsche commented Dec 30, 2020 •

edited

Loading

mgoltzsche Dec 30, 2020 •

edited

Loading

mgoltzsche Dec 30, 2020 •

edited

Loading

mgoltzsche commented Jan 20, 2021

mgoltzsche commented Feb 21, 2021

innobead commented Mar 2, 2021

mgoltzsche commented Mar 6, 2021 •

edited

Loading

LarsBingBong commented Mar 21, 2022

derekbit commented Mar 21, 2022 •

edited

Loading

LarsBingBong commented Mar 21, 2022

github-actions bot commented Jul 27, 2024

github-actions bot commented Aug 6, 2024

Enhance helper Pod interface and configuration. #166

Enhance helper Pod interface and configuration. #166

Conversation

mgoltzsche commented Dec 30, 2020 • edited Loading

mgoltzsche Dec 30, 2020 • edited Loading

Choose a reason for hiding this comment

mgoltzsche Dec 30, 2020 • edited Loading

Choose a reason for hiding this comment

mgoltzsche commented Jan 20, 2021

mgoltzsche commented Feb 21, 2021

innobead commented Mar 2, 2021

mgoltzsche commented Mar 6, 2021 • edited Loading

LarsBingBong commented Mar 21, 2022

derekbit commented Mar 21, 2022 • edited Loading

LarsBingBong commented Mar 21, 2022

github-actions bot commented Jul 27, 2024

github-actions bot commented Aug 6, 2024

mgoltzsche commented Dec 30, 2020 •

edited

Loading

mgoltzsche Dec 30, 2020 •

edited

Loading

mgoltzsche Dec 30, 2020 •

edited

Loading

mgoltzsche commented Mar 6, 2021 •

edited

Loading

derekbit commented Mar 21, 2022 •

edited

Loading