stop service update when disable svclb flag is passed #543
Conversation
8bad267 to
94b5a22
Compare
|
LGTM, thanks! |
|
While this probably fixes the problem, no tests were added to ensure this doesn't happen again. There are two ways to do this, either we start having functional testing of metallb (nice but will require a certain time investment), or we add a unit test for this. It can totally be done in another PR, though. |
There was a problem hiding this comment.
Excuse me for my noobness here... I am also curious about this patch, shouldn't we ensure that the svclb controller is not deployed in the first place when the flag is passed, rather than update locally things in the controller itself? It would be easier to write unit tests to test if something isn't present/running vs testing its behaviour? Or do you mean we always have to bundle the two, and make sure one is disabled this way?
|
Great questions @evrardjp ! Originally the fix was setup not to register the controller if the flag is present but it looked liked we might want the current change for maintenance purposes since the controller was already being passed an enabled flag. There is probably some more manual testing that can be done to ensure the behavior is correct and related code cleanup if necessary. We definitely need and are planning to write automated tests for all of the k3s features, but still coming up with a test plan on how that will work. |
That's what I guessed. Thanks for clarifying :)
I will try to track this not this week-end but the one afterwards. Don't hesitate to ping me if you need a manual tester on this. It would until there is a plan for the automated testing, of course ;) |
|
I have completely let that slip. I don't see an issue to explain the plan for testing, will it be documented there? |
#542