opened
randomvariable
username_0
<!-- NOTE: ⚠️ For larger proposals, we follow the CAEP process as outlined in https://sigs.k8s.io/cluster-api/CONTRIBUTING.md. -->

**User Story**

As a operator, I deploy a cluster in my infrastructure provider. The infrastructure has a CIDR range of 192.168.0.0/24, and I set a pod and service CIDR range of 192.168.0.0/16.

I see intermittent communication failures, but Cluster API hasn't told me what is wrong.

**Detailed Description**

Cluster API should inform a user that there is overlap between their infrastructure networking and that for the pods and service CIDRs, such that it could cause communication issues. This ideally would be a condition on the Cluster resource, that infrastructure providers could inform.

**Anything else you would like to add:**

[Miscellaneous information that will assist in solving the issue.]

/kind feature
Cluster condition, consumable by Infrastructure Providers for machine IP / pod & service CIDR overlap
issue
created
vincepri
username_1
/milestone v0.3.x
/help
comment
created
vincepri
username_1
/milestone v0.4.0
comment
created
vincepri
username_1
/help
/priority important-longterm
comment
created
iamemilio
username_2
/assign
I would like to take a look at this
comment
created
fabriziopandini
username_3
What about having a validation webhook that prevents misconfiguration instead of reporting errors after the cluster is created?
comment
created
randomvariable
username_0
@username_3 Could do, but would have to be infrastructure specific then, which is fine as well.
comment
created
randomvariable
username_0
@username_3 would need validation webhooks performed by each infraprovider, since the cluster object has no idea of the infrastructure networks being used.
comment
created
randomvariable
username_0
Additionally in DHCP environments, even the infra provider doesn't know what the networks are.
comment
created
iamemilio
username_2
Excuse my Newbieness, but do we need to dynamically check the network CIDR range? For this validation check, why should we not just assume that the CIDR ranges the users entered [here](https://github.com/kubernetes-sigs/cluster-api/blob/7fdecfe013260f6ca4dd5afa1ae72a37766f7506/api/v1alpha3/cluster_types.go#L67) are valid, and just check if they overlap?

If we want to validate the CIDR ranges themselves, I think that should be a separate step, and should be handled by the infra providers
comment
created
randomvariable
username_0
We don't know the network range of the infrastructure from that information, which is the main problem that pops up. That struct only tells us the CNI networking.

Should probably be along the lines of comparing Machine.Status.Addresses vs. the CIDRs in the struct above, and add a condition representing a clash.
comment
created
iamemilio
username_2
Thanks, that makes more sense. My next question would be why a validation webhook? Do we do a simlar things for other validation steps? Do we have a process for validating configs?
comment
created
iamemilio
username_2
@username_0 for clarification, by infraprovider do you mean the underlying infra, or cluster-api-provider-foo?
comment
created
iamemilio
username_2
Can we define an interface version of our [crd validation webhooks](https://github.com/kubernetes-sigs/cluster-api/blob/9cbf8be8d643203d99aca6714825abdabcbca0b1/api/v1alpha3/cluster_webhook.go#L64) then have the cluster-api-providers implement it downstream?
comment
created
randomvariable
username_0
@username_2 There's no guarantee an infra provider knows what the infrastructure network will be ahead of time, so it can't happen via a webhook.

If we take vSphere or bare metal as an example, you may very well be putting a machine in an L2 network with some external DHCP. There's nothing on the machine specification that mentions an IP address at that time.
comment
created
iamemilio
username_2
Given those restrictions, the only way this can really work on all platforms the same way is to check the ip assigned to each machine against the service and pods network CIDRS and throw and error or warning in the machine-controller logs if they overlap. Is that an acceptable solution?

Another option to consider is how things are handled in OpenShift. We require admins to do a little legwork upfront and tell us the CIDR of the network our nodes will get their IPs from. This makes the validation easy and upfront. In the CAPI case, this would mean that we would add another optional param to the ClusterNetwork object: `Machines *NetworkRanges`. If this is provided, we can quickly check for overlap with a webhook. The big question is if this is worth adding a parameter to the cluster API.
comment
created
ncdc
username_4
Logs are not user-facing. The only available user-facing status indicator is the conditions array in status. The original description above suggests making this a condition in Cluster status.

I'll defer to @username_5 @CecileRobertMichon @username_0 etc. for thoughts on the suggested new API field.
comment
created
detiber
username_5
I'm not necessarily sure I'm a fan of adding a field for this unless we have a comprehensive proposal around it, how it would impact providers, and how it would impact users.

I want to make sure we aren't creating a situation where it's really easy for a user to shoot themselves in the foot, and that we aren't necessarily imposing requirements on providers that are unreasonable.

For example, with AWS, I could easily see us being able to report the network range in use by the VPC (either cluster-api managed or user-provided) in a way that the Cluster resource could verify things (likely not at admission time, but could still short circuit the feedback loop on conditions before things get too far along). I'm not sure how feasible that approach would be for vSphere, metal3, packet, or other types of bare metal'ish providers.

Also depending on the use case we are talking about (such as a managed service provider model), it may not be reasonable to expect the end user to know what network address block will be used in advance.
comment
created
randomvariable
username_0
Agree with @username_5 .

Just to clarify the use case for having a condition on the cluster object:

Having a tool like the at-the-glance status in https://github.com/kubernetes-sigs/cluster-api/issues/3802 could show the problem across infrastructure providers.

Concretely, just thinking of a defined `ConditionType` constant for the condition and leaving the rest to infra providers.
comment
created
fabriziopandini
username_3
/remove-lifecycle stale
comment