Add backoff rules to provider config

[eiabea]

Checklist

• added release notes to Unreleased section in CHANGELOG.md

References

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request

[codeclimate]

Code Climate has analyzed commit 1f4adf0 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 57.1% (50% is the threshold).

This pull request will bring the total coverage in the repository to 55.8% (0.0% change).

View more on Code Climate.

[eiabea]

@marioreggiori could you please have a look at this PoC? I am not quite sure if ProviderConfig is the correct place for this settings and if we should put even more parameters in there to justify the new struct

[marioreggiori]

A constant might be sufficient too. But since we don't yet have data on how many steps are normal, a field in the provider config could be justifiable. I'd prefer a simple int value instead of a struct tho. And we should probably prefix it at least with LoadBalancer.

[marioreggiori]

Some minor comments. Please remember to document changes in CHANGELOG.md.

[eiabea]

I kind of went down a rabbit-hole by adding tests to the configuration parsing, my current conclusion is that it currently isn't working as intended. With the current implementation the default: foo of envconf is "stronger" than actual values in the yaml config file

e.g.:

defaults:

AutoDiscoveryTagPrefix string `yaml:"autoDiscoveryTagPrefix,omitempty" split_words:"true" default:"anxkube-ccm-lb"`
LoadBalancerBackoffSteps int `yaml:"loadBalancerBackoffSteps" default:"10"`

config:

loadBalancerBackoffSteps: 123
autoDiscoveryTagPrefix: "awesome"

resulting provider config:

{   anxkube-ccm-lb false  [] [] 10}

I am thinking about switching to a more up-to-date env parsing library like env

[marioreggiori]

I kind of went down a rabbit-hole by adding tests to the configuration parsing, my current conclusion is that it currently isn't working as intended. With the current implementation the default: foo of envconf is "stronger" than actual values in the yaml config file

...

I am thinking about switching to a more up-to-date env parsing library like env

I can confirm that our NewProviderConfig function probably doesn't work as intended. I'm not sure if the lib you proposed solves the problem, as it also only reads env variables. spf13/viper might be something to consider.

Ping @LittleFox94

[eiabea]

I kind of went down a rabbit-hole by adding tests to the configuration parsing, my current conclusion is that it currently isn't working as intended. With the current implementation the default: foo of envconf is "stronger" than actual values in the yaml config file
...
I am thinking about switching to a more up-to-date env parsing library like env

I can confirm that our NewProviderConfig function probably doesn't work as intended. I'm not sure if the lib you proposed solves the problem, as it also only reads env variables. spf13/viper might be something to consider.

Ping @LittleFox94

As discussed, this will be part of a bigger refactoring task

[marioreggiori]

LGTM 👍