[CORE-12314] Use two ServiceEndpoint instances for host-network and pod-network#4474
[CORE-12314] Use two ServiceEndpoint instances for host-network and pod-network#4474coutinhop wants to merge 1 commit intotigera:masterfrom
Conversation
coutinhop
force-pushed
the
pedro-CORE-12314
branch
from
445c75d to
a24a266
Compare
| // ServiceEndpoint is the Host/Port of the K8s endpoint. | ||
| // HostNetworkHost/HostNetworkPort are used for host-networked pods, while | ||
| // PodNetworkHost/PodNetworkPort are used for pod-networked pods. | ||
| type ServiceEndpoint struct { |
There was a problem hiding this comment.
Does it make sense to extend this struct? Or just to use two instances?
ep := ServiceEndpoint{}
hostNetEP := ServiceEndpoint{}
^ This feels a bit conceptually cleaner to me? I think this will make the code a lot clearer.
| } | ||
| } | ||
|
|
||
| if provider == operator.ProviderDockerEE && !hostNetworked && k8s.Host == dockerEEProxyLocal { |
There was a problem hiding this comment.
Is this change intentional? Would this be a breaking change for MKE?
There was a problem hiding this comment.
it was intentional, my reasoning was that if we are making a change for !hostNetworked then this condition would be redundant... but if we do change the default behaviour in this PR then this might need to be restored...
pkg/controller/utils/utils.go
Outdated
| k8sapi.Endpoint.HostNetworkHost = cm.Data["KUBERNETES_SERVICE_HOST"] | ||
| k8sapi.Endpoint.HostNetworkPort = cm.Data["KUBERNETES_SERVICE_PORT"] | ||
| k8sapi.Endpoint.PodNetworkHost = cm.Data["KUBERNETES_SERVICE_HOST_POD_NETWORK"] | ||
| k8sapi.Endpoint.PodNetworkPort = cm.Data["KUBERNETES_SERVICE_PORT_POD_NETWORK"] |
There was a problem hiding this comment.
Hm, I think this is a breaking change - today, both pods share the same configuraiton.
I think we want to keep that behavior if the POD_NETWORK fields aren't present (i.e., use the host network ones).
At a minimum, we need a release note. Ideally we wouldn't break anyone on upgrade, though!
coutinhop
force-pushed
the
pedro-CORE-12314
branch
from
a24a266 to
d084fb3
Compare
coutinhop
changed the title
…od-network Use two separate ServiceEndpoint instances (Endpoint and PodNetworkEndpoint) instead of a single struct, to distinguish between host-network and pod-network K8s API server endpoints. Simplify EnvVars() by removing hostNetworked/provider params — callers pick the appropriate instance. Remove Docker EE proxy.local special-case, which is now handled implicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
coutinhop
force-pushed
the
pedro-CORE-12314
branch
from
d084fb3 to
a747635
Compare
3 participants
Description
Use two separate ServiceEndpoint instances (Endpoint and PodNetworkEndpoint)
instead of a single struct, to distinguish between host-network and pod-network
K8s API server endpoints. Simplify EnvVars() by removing hostNetworked/provider
params — callers pick the appropriate instance. Remove Docker EE proxy.local
special-case, which is now handled implicitly.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Release Note
For PR author
make gen-filesmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.