Fix: change sg finalizer suffix to sgID

[Rdpaula]

We need these changes because our controller throws reconciler errors when our sgName is bigger than 63 characters. So we decided to change the finalizer suffix from sgName to sgID because it is smaller and still references which security group put that finalizer.

failed to add finalizer in xxxxx-xxxxxx: KopsMachinePool.infrastructure.cluster.x-k8s.io "xxxxxx-xxxxxx" is invalid: metadata.finalizers: Invalid value: "securitygroup.wildlife.infrastructure.io/xxxxxxxxxxxxx-xxxxxxxxxxxxx-xxxxxxxxxxx-xxxxxxxxxx-xxxxxxxxxxxx': name part must be no more than 63 characters

[rsafonseca]

Why are we overcomplicating this? The finalizer should be a static string like kcio or something like that, indicating which platform has pending actions ahead of the object being permanently removed. The sg name and Id should already be present in the object being reconciled.

[Rdpaula]

Why are we overcomplicating this? The finalizer should be a static string like kcio or something like that, indicating which platform has pending actions ahead of the object being permanently removed. The sg name and Id should already be present in the object being reconciled.

I'm not sure If I understand well. But this decision was made because if you have two SG's associated with a KMP for example, you can't have a unique finalizer. How do you know that all security groups of one kmp are detached/disassociated with only one finalizer?

[rsafonseca]

The finaliser is a property on the SG CR. If you have 2 SGs, you have 2 CRs. The finaliser is generally just a string that generally has the name of the controller which needs to perform additional actions on the object before it is deleted in kubernetes. KCIO will never add two finalisers to the same SG CR, as it would make no sense.

[Rdpaula]

The finaliser is a property on the SG CR. If you have 2 SGs, you have 2 CRs. The finaliser is generally just a string that generally has the name of the controller which needs to perform additional actions on the object before it is deleted in kubernetes. KCIO will never add two finalisers to the same SG CR, as it would make no sense.

These two finalizers are added to KMP CR, not SG CR.

[rsafonseca]

oh, this was confusing because it's in the security group controller, and thus the failure events are being attached to the securitygroup CR.
Still, the correct way to do this would be to create an owner/dependent relationship and just have a static name for the finaliser IMO, see https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/

[Rdpaula]

oh, this was confusing because it's in the security group controller, and thus the failure events are being attached to the securitygroup CR. Still, the correct way to do this would be to create an owner/dependent relationship and just have a static name for the finaliser IMO, see https://kubernetes.io/docs/concepts/overview/working-with-objects/owners-dependents/

I agree with you, but we can't do this solution because A valid owner reference consists of the object name and a UID within the same namespace as the dependent object. SGs aren't namespaced and KMPs are. 😢

[Rdpaula]

We still thought of another solution that was an intermediary resource, like PV and PVC. But we would have to do another controller and code its lifecycle, we didn't because it requires much more time to do it.

[rsafonseca]

Well, i guess this solves the bigger issue for now, we can think about further improvement later when we have the cycles for it :)
Thanks for taking the time to discuss it
LGTM