fix(pkg/automationconfig): do not try to access non-declared split horizon dns records #890
Conversation
maxgio92
requested review from
chatton,
ciprian-tibulca,
irajdeep,
mircea-cosbuc,
priyolahiri and
rodrigovalin
as code owners
…rizon dns records The issue occurs during scaling down, as the automation config is tried to be built with actual numbers of replicas with desired split-horizon dns records. Then a panic with out of range of the related slice is triggered. Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
maxgio92
force-pushed
the
fix/scale-down-automation-config
branch
from
27ef4c7 to
2fd3877
Compare
priyolahiri
left a comment
priyolahiri
left a comment
There was a problem hiding this comment.
Do you want to add a log entry if the horizon entries are being ignored because of wrong entries or blank config?
|
Also, please check #851 (comment) to ensure that this case is covered as well |
Signed-off-by: maxgio92 <massimiliano.giovagnoli.1992@gmail.com>
|
Hello @priyolahiri, thank you for your time. I added a second test case for multiple DNS horizons. About the logging, I noticed that the builder structures leveraged to build the automation config doesn't accept a logger as of now. I'd avoid to extend those structures, as in the end, this state is transitionary during the reconciliation and when the reconciliation will be completed no Otherwise we can think about extending those structures to accept loggers as done in other packages. What do you think? |
We can work with this for now. Let's thick about extending later. |
2 participants
This PR tries to fix #889 by not to not try to access non-declared split horizon DNS records.
The issue occurs during scaling down, as the automation config is tried to be built with actual numbers of replicas with desired split-horizon dns records. Then a panic with out of range on the related slice is triggered.
Closes #889
Signed-off-by: maxgio92 massimiliano.giovagnoli.1992@gmail.com