Hi @gongguan. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

/kind bug

/sig network
/area ipvs

another question:
graceful termination can be deleted too?
As service deleted, can ipvs virtual server be deleted directly, then unbind svc addr in dev ipvs0

there is a logical error in current code:
graceful termination will never executed

/assign @andrewsykim
/cc @lbernail @m1093782566

/ok-to-test

/priority important-soon

I also expected a service deletion to trigger a deletion of the matching endpoint resource which should trigger graceful termination. I

It does, but for TCP it takes 15m for those endpoints to be gracefully terminated and so the virtual server doesn't delete for 15m as well. So I think either we delete the virtual server right away (this PR) or we decrease the graceful termination timeout to a lower value.

@andrewsykim in the test I did, gracefultermination was not triggered at all (and weight remains at 1) so I suspect something is wrong in the handler

Anyway, if we agree we can remove services without gracefultermination this is not important

The PR looks good. @gongguan can you add a test for this specific scenario? (by fixing the test in TestCleanLegacyService)

@andrewsykim OK I think I know what happens: syncProxyRules is called for service deletion and endpoint deletion, but the reconcile loop only considers active services: https://github.com/kubernetes/kubernetes/blob/master/pkg/proxy/ipvs/proxier.go#L902

So the only way to trigger gracefulTermination for real servers on service deletion would be in cleanLegacyService

Thanks for clarifying @lbernail!

Anyway, if we agree we can remove services without gracefultermination this is not important

IMO if a Service is deleted there's no reason to try graceful termination. I will ask some folks in SIG Network though to confirm if this shouldn't be the case.

The PR looks good. @gongguan can you add a test for this specific scenario?

Added a test case for this here #81309, PTAL :)

/lgtm

Thanks @gongguan!

Thanks for adding the test!
We'll need to backport this to all supported branches

I just remember the context behind the graceful termination behavior: #71894
TL;DR we did not have expirenodestconn before which means when we removed a VS (and therefore RS) on service deletion, established connections were not terminated and their traffic was being blackholed.

Because we now set expirenodestconn, established connections will be terminated as soon as a packet is sent

/retest

Added a test case for this here #81309, PTAL :)

Thanks for @andrewsykim 's test. By the way, what else need to be done for me?

Not much from here, PR should be merged by the bot shortly :). Thanks @gongguan!

So how do I make sure this problem is solved now? In which version did the solution arrive?

Automatically clears unwanted ipvs rules

part of issues still exists, address bound in kube-ipvs0 will not be deleted in kubernetes1.18.0

SAME IN kubernetes1.23.5

Please open a new issue with the details included and assign it to me and @uablrek

I can't reproduce this in v1.23.5. I tried;

• Connect via tcp to a ClusterIP and let the connection stay open
• watch ip addr show dev kube-ipvs0 in another terminal
• kubectl delete svc-name

The ClusterIP is removed from kube-ipvs0 almost immediately. The tcp connection is black-holed.