yawol icon indicating copy to clipboard operation
yawol copied to clipboard

target-controller: service-controller: possible bug in deletion logic

Open gfelbing opened this issue 1 year ago • 1 comments

According to

https://github.com/stackitcloud/yawol/blob/80f74e69ac155de29e6e0eeb5c835056ba1813d9/controllers/yawol-cloud-controller/targetcontroller/service_controller.go#L71

the deletion routine for services of different classes should only be triggered if the loadbalancer object still exists. As there is a comment, I think there was a conscious decision towards it.

If the deletion shouldn't be triggered, then why is the deletion routine still triggered for other services, e.g. with different types? https://github.com/stackitcloud/yawol/blob/80f74e69ac155de29e6e0eeb5c835056ba1813d9/controllers/yawol-cloud-controller/targetcontroller/service_controller.go#L78

For the example above, it would even break a test if I add the check for lb existence, as it doesn't remove the finalizer of the service (which is valid).

IMHO the check for lb existence when reconciling deletions of services with different classes is obselete / is a bug in the case when there is no lb object (anymore) but still a finalizer left.

gfelbing avatar Feb 01 '24 19:02 gfelbing

This is because if the class is different, than it would be possible that a second yawol-controllers is running and resposible for this service (this is the case in dev setups). If that is the case running the deletionRoutine() would result in a remove of the finalizer of the other yawol-controllers. In case of a different type it is not possible that a other yawol-controller is taking care of that service so running the deletion is fine in all cases.

dergeberl avatar Feb 14 '24 08:02 dergeberl