[Bug fix] Revive serviceReconciler.reconcileTargetsResource() to handle k8sService deletion de-registering targets

[zijun726911]

…8sService deletion

What type of PR is this?

Which issue does this PR fix: bug fix for #331
It's quite possible that this previous refactoring service_controller.go, cause a regression that "deleting k8sService do NOT de-registering targets". Bring back the (r *serviceReconciler) reconcileTargetsResource() logic to handle the k8sService deletion could fix this issue

What does this PR do / Why do we need it: bug fix for #331

If an issue # is not available please add repro steps and logs from aws-gateway-controller showing the issue:

Testing done on this change:

Automation added to e2e:

newly un-commented e2etest case pass, all test cases pass

Ran 29 of 29 Specs in 1876.938 seconds
SUCCESS! -- 29 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestIntegration (1877.46s)
PASS

E2E tests for new revision also pass

Will this PR introduce any new dependencies?:

Will this break upgrades or downgrades. Has updating a running cluster been tested?:

Does this PR introduce any user-facing change?:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[solmonk]

can you also check manually that target de-registration is happening?

[zijun726911]

can you also check manually that target de-registration is happening?

Already did the manual test, single step debug to trace the build_model and synthesizer de-registering targets
code path, it works as expected

[solmonk]

LGTM

[liwenwu-amazon]

Please make sure the e2e test cover this scenario! So that we will NOT have another regression in future

[liwenwu-amazon]

You need finalizer. Otherwise, the service delete event can be lost.

[zijun726911]

You need finalizer. Otherwise, the service delete event can be lost.

finalizer added at line 126, please check the controllers/service_controller.go
code, for k8sService creation, it only add the finalizer. for k8sService deletion, it does de-registering targets and then remove finalizer.

deregister_targets_test.go e2etest covered these use cases

[liwenwu-amazon]

Why do u delete this particular case?

[zijun726911]

This test case is still there at line71 https://github.com/aws/aws-application-networking-k8s/pull/387/files#diff-5d3817ed5a0b35b8fd4bd339f69b3b19b3b007739c3b25e938e976d2e5fefb9dR71