Skip to content

Commit

Permalink
Fix Service update processing
Browse files Browse the repository at this point in the history
Currently the installServices method is somewhat long and redundant,
making it hard to maintain and error-prone. In fact, a few bugs were
found in the recent releases which are related to it more or less. While
sorting out the code, I found there are actually more bugs in it:

1. After updating stickyMaxAgeSeconds, the flow for ClusterIP isn't
   updated because the installServiceFlows interface skip updating flows
   whose cache keys already exist.
2. After updating stickyMaxAgeSeconds, the flows for NodePort and
   LoadBalancerIPs aren't updated because the installServiceFlows
   interface is not even called.
3. After updating InternalTrafficPolicy, the flows for ClusterIP isn't
   updated.
4. After updating InternalTrafficPolicy for a ClusterIP Service, the
   stale group is not removed.
5. Endpoints are installed repeatedly even though there are already
   reference counters for them.

This patch tries to refactor the method to make it eaiser to understand
and maintain, and fixes all the above bugs. It makes the following
changes:

1. Code redundancy is reduced with some shareable sub-procedures being
   extracted to sub-functions.
2. Calculation of Variables that are required by a sub-procedure only
   are moved to the corresponding sub-function.
2. Repeated code that retrieves the group IDs are removed.
3. The ways of processing ClusterIP, NodePort, and LoadBalancerIPs are
   unified.
4. A method for installing Endpoints in the same way as uninstalling
   Endpoints is added.
5. Use needUpdateService to represent all the flows of the Service need
   update, and use needUpdateServiceExternalAddresses to represent only
   the flows related to ExternalAddresses need update.

Signed-off-by: Quan Tian <qtian@vmware.com>
  • Loading branch information
tnqn committed Apr 12, 2023
1 parent 6093379 commit 1392758
Show file tree
Hide file tree
Showing 2 changed files with 265 additions and 299 deletions.
Loading

0 comments on commit 1392758

Please sign in to comment.