CRS controller should wait for the 'kubernetes' service ready before reconciling CRS objects

[jessehu]

What steps did you take and what happened:
This is an issue reported in the slack: https://kubernetes.slack.com/archives/C8TSNPY4T/p1667740494784379
Did anyone hit the error that ClusterResourceSet controller applies objects in ClusterResourceSet too early before the Service kubernetes is created? I hit this error once this week, while using ClusterResourceSet to deploy kapp-controller which contains a Service kapp-controller/packaging-api . This service is assigned with the IP “10.96.0.1”, and then creating the Service kubernetes failed due to service IP conflict.

# k logs -n kube-system       kube-apiserver-mycluster-controlplane-pl4vn
E1106 12:09:27.196308       1 controller.go:240] unable to sync kubernetes service: Service "kubernetes" is invalid: spec.clusterIPs: Invalid value: []string{"10.96.0.1"}: failed to allocate IP 10.96.0.1: provided IP is already allocated
E1106 12:09:37.197558       1 controller.go:240] unable to sync kubernetes service: Service "kubernetes" is invalid: spec.clusterIPs: Invalid value: []string{"10.96.0.1"}: failed to allocate IP 10.96.0.1: provided IP is already allocated

# k get svc -A
NAMESPACE         NAME            TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)                  AGE
kapp-controller   packaging-api   ClusterIP   10.96.0.1    <none>        443/TCP                  2d2h
kube-system       kube-dns        ClusterIP   10.96.0.10   <none>        53/UDP,53/TCP,9153/TCP   2d2h

# k get node
NAME                           STATUS     ROLES           AGE    VERSION
mycluster-controlplane-pl4vn   NotReady   control-plane   2d3h   v1.24.4
mycluster-workergroup1-ccfcz   NotReady   <none>          2d3h   v1.24.4
mycluster-workergroup1-lmx7b   NotReady   <none>          2d3h   v1.24.4

The service object creation timestamp:
# k get svc -n kapp-controller   packaging-api -oyaml |grep creationTimestamp
  creationTimestamp: "2022-11-04T09:37:14Z"

Seems the CRS controller just gets the remote client for the workload cluster, but does not check if the Service kubernetes in the workload cluster has been created:
https://github.com/kubernetes-sigs/cluster-api/blob/v1.2.7/exp/addons/internal/controllers/clusterresourceset_controller.go#L239-L247

What did you expect to happen:
kapp-controller CRS should be applied successfully

Anything else you would like to add:
We tried to workaround this issue by adding the wait logic before applying CRS objects like this:

err = wlcClient.Get(ctx, apitypes.NamespacedName{
	Namespace: metav1.NamespaceDefault,
	Name:      "kubernetes",
}, &corev1.Service{})
if err != nil && !apierrors.IsNotFound(err) {
	return reconcile.Result{}, err
}
if apierrors.IsNotFound(err) {
	ctx.Logger.Info("Wait for the Service kubernetes to be created")
	return reconcile.Result{RequeueAfter: NormalRequeueTimeout}, nil
}

Environment:

• Cluster-api version: 1.2.7
• minikube/kind version:
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

/kind bug
[One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels]

[fabriziopandini]

/triage accepted
I don't have strong opinions here but It seems a nice improvement that can make CRS more robust.

[killianmuldoon]

To correctly understand this use case - AFAIU this is a race condition on the creation of the service and sometimes CRS is too fast which causes cluster creation to fail. If that's correct this could happen for any secret created at Cluster initialization - it seems like this could / should be fixed at the Kubernetes level. Is there an issue open anywhere for this?

The workaround in @8048 looks okay to me, but I think we have to be careful to ensure this doesn't block in any case.

[killianmuldoon]

Looking at https://kubernetes.io/docs/concepts/services-networking/cluster-ip-allocation/#avoid-ClusterIP-conflict it seems that the kapp service shouldn't be able to get that service IP dynamically. Is something setting the ClusterIP statically for the kapp service?

[jessehu]

That's interesting. In my K8s, the K8s service CIDR is 10.96.0.0/22, so the IP range is:

Range Size: 2**10 - 2 = 1022
Band Offset: min(max(16, 1024/16), 256) = min(64, 256) = 64
Static band start: 10.96.0.1
Static band end: 10.96.0.64
Range end: 10.96.3.255

Here are the services in a normal K8s cluster:

NAMESPACE               NAME                               TYPE        CLUSTER-IP    EXTERNAL-IP   PORT(S)                        AGE
default                 kubernetes                         ClusterIP   10.96.0.1     <none>        443/TCP                        2d13h
kapp-controller         packaging-api                      ClusterIP   10.96.1.102   <none>        443/TCP                        2d13h
kube-system             kube-controller-manager            ClusterIP   10.96.2.32    <none>        10257/TCP                      2d12h
kube-system             kube-dns                           ClusterIP   10.96.0.10    <none>        53/UDP,53/TCP,9153/TCP         2d13h
kube-system             kube-scheduler                     ClusterIP   10.96.3.15    <none>        10259/TCP                      2d12h
kube-system             kubelet                            ClusterIP   None          <none>        10250/TCP,10255/TCP,4194/TCP   2d12h

The kapp-controller/packaging-api service got a dynamically allocated CLUSTER-IP 10.96.1.102. There should not be any code inside kapp-controller or our system to specific hard-coded IP 10.96.0.1 for it. But why K8s allocates the first reserved IP 10.96.0.1 in the service IP range to the 1st created Service ? The related K8s code is https://github.com/kubernetes/kubernetes/blob/release-1.24/pkg/registry/core/service/ipallocator/allocator_test.go#L33

[killianmuldoon]

Interesting - can you get a minimal test example? Maybe somehow blocking the creation of the default secret to see what IP the kapp secret gets in that case?

[Levi080513]

Looking at https://kubernetes.io/docs/concepts/services-networking/cluster-ip-allocation/#avoid-ClusterIP-conflict it seems that the kapp service shouldn't be able to get that service IP dynamically. Is something setting the ClusterIP statically for the kapp service?

this solution is seems like this kubernetes proposal https://github.com/kubernetes/enhancements/blob/77b58405f235f8c2969758241216326efb83892c/keps/sig-network/3070-reserved-service-ip-range/README.md

this proposal was alpha in kubernetes version v1.24, beta in v1.25, and GA in v1.26.
before kubernetes version v1.25, default cluster ip allocate strategy is random.

[killianmuldoon]

Thanks for getting to the bottom of that - looks like CRS does have this issue for as long as v1.24 is supported. We should definitely work on getting the fix included.

Thanks again @Levi080513! I'll attempt to do a full review of your PR soon.

[jessehu]

This issue can be closed per #8048