Add support for ClusterIP in upstreams in VirtualServers/VirtualServerRoutes #1449
Conversation
github-actions
bot
added
the
enhancement
lucacome
force-pushed
the
feat/upstream-cluster-ip
branch
from
d1e3a0e
to
ecc3a84
Compare
github-actions
bot
added
the
documentation
There was a problem hiding this comment.
Hi @lucacome
Please see my feedback
There is one more issue - when the new field is enabled, since the IC no longer uses endpoints, but rather uses the service ClusterIP, there is no need to update and reload NGINX config (or update API for Plus) for the affected VirtualServers/VirtualServerRoutes, when the endpoints related to the service are updated.
I think we can easily prevent it if we update FindResourcesForEndpoints in Configuration to use a different referenceChecker - the one that could take into consideration the upstream use-cluster-ip field. What do you think?
| var podEndps []podEndpoint | ||
| var err error | ||
| var endps []string | ||
| if u.UseClusterIP { |
There was a problem hiding this comment.
I think it makes sense to refactor the getting endpoints logic into a function so that we can use it both for VS and VSR (prob after this PR) - the code looks identical
There was a problem hiding this comment.
yeah I thought about that but decided to leave it for later 👍
docs-web/configuration/virtualserver-and-virtualserverroute-resources.md
Outdated
Show resolved
Hide resolved
docs-web/configuration/virtualserver-and-virtualserverroute-resources.md
Outdated
Show resolved
Hide resolved
docs-web/configuration/virtualserver-and-virtualserverroute-resources.md
Outdated
Show resolved
Hide resolved
| @@ -1044,3 +1044,129 @@ func TestIsPolicyIsReferenced(t *testing.T) { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| func TestEndpointIsReferencedByVirtualServerAndVirtualServerRoutes(t *testing.T) { | |||
pleshakov
changed the title
Add support for ClusterIP in upstreams
TODO: