Centralize configuration using a Configs struct #53
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
helgi
force-pushed
the
feature/env-handling
branch
from
aae90bf to
aaf49ca
Compare
deitch
requested changes
May 12, 2021
README.md
Outdated
| * `ROLLER_DRAIN_FORCE` [`bool` default: `true`]: If drain will force delete kubernetes resources if they violate PDB or grace periods. | ||
| * `ROLLER_IGNORE_DAEMONSETS` [`bool`, default: `true`]: If set to `false`, will not reclaim a node until there are no DaemonSets running on the node; if set to `true` (default), will reclaim node when all regular pods are drained off, but will ignore the presence of DaemonSets, which should be present on every node anyways. Normally, you want this set to `true`. | ||
| * `ROLLER_DELETE_LOCAL_DATA` [`bool`, default: `false`]: If set to `false` (default), will not reclaim a node until there are no pods with [emptyDir](https://kubernetes.io/docs/concepts/storage/volumes/#emptydir) running on the node; if set to `true`, will continue to terminate the pod and delete the local data before reclaiming the node. The default is `false` to maintain backward compatibility. | ||
| * `ROLLER_INTERVAL` [`time.Duration`, default: `30s`]: Time between roller runs. Takes time duration such as 10s, 10m, 10d |
There was a problem hiding this comment.
Can you link here to what formats are acceptable, and add a few samples? You and I may be very comfortable with time.Duration; plenty of other users may not.
There was a problem hiding this comment.
That having been said, I like this INTERVAL better than the old DELAY
There was a problem hiding this comment.
Agreed, pushed a change
|
Thanks! Will update based on the feedback and/or reply to individual things. |
|
I wanted to check in and see if there was anything else needed? |
deitch
previously approved these changes
Jun 7, 2021
|
Looks like CI failed. Not directly due to this, just because it was using go 1.12.1, when some of these packages use go 1.13+ features. I will bump to go 1.15, then you can rebase. |
|
OK, bumped. Rebase this and push, and CI should go green. |
Uses github.com/caarlos0/env/ to read ENV configuration, set type rules and few other helpers. Moves from vars and const's to passing in values consistently via functions, making testing more predictable and the flow a tad bit easier to understand. ROLLER_CHECK_DELAY is now deprecated in favor of ROLLER_INTERVAL to allow for stronger type management. This moves from an int to time.Duration, 30 vs 30s (or 30m, 30d, whatever time.ParseDuration can handle). ROLLER_CHECK_DELAY still works and is auto applied as ROLLER_INTERVAL in seconds if set. Brings in github.com/stretchr/testify as a direct depdency (previously indirect), helping with the new config testing. Further enhancements can be done in future commits to bring log handling more centralized and moving more things to structs to make assigning value easier
Introduces ROLLER_DRAIN and ROLLER_DRAIN_FORCE, both defaulting to true to keep existing behaviour for compatibility Fixes deitch#52
helgi
force-pushed
the
feature/env-handling
branch
from
e005c2d to
e6e6dd6
Compare
|
@deitch rebased :) |
deitch
approved these changes
Jun 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uses github.com/caarlos0/env/ to read ENV configuration, set type rules, and few other helpers. Moves from vars and const's to passing in values consistently via functions, making testing more predictable and the flow a tad bit easier to understand.
ROLLER_CHECK_DELAY is now deprecated in favor of ROLLER_INTERVAL to allow for stronger type management. This moves from an int to
time.Duration, 30 vs 30s (or 30m, 30d, whatever time.ParseDuration can handle).ROLLER_CHECK_DELAY still works and is auto applied as ROLLER_INTERVAL in seconds if set.
Brings in github.com/stretchr/testify as a direct dependency (previously indirect), helping with the new config testing.
Also, adds the ability to control if the roller drains and if it uses force …
ccd2f9f
Introduces ROLLER_DRAIN and ROLLER_DRAIN_FORCE, both defaulting to true to keep existing behavior for compatibility
Fixes #52