[Bug]: (some) strimzi PVC fails to resize

[leiyiz]

Bug Description

the CanSupport() function in csi_resizer.go has a line looking for

resizerName := pvc.Annotations[util.VolumeResizerKey]

and will fail the resize if that annotation pair does not exist and if the volume is a in-tree volume post migration. the VolumeResizerKey is defined here as volume.kubernetes.io/storage-resizer and I do not see this annotation in any of the PVCs created with strimzi. In a new cluster I created a in-tree gce-pd PVC and expanded it, after expansion I see the annotations:

Annotations:   pv.kubernetes.io/bind-completed: yes
               pv.kubernetes.io/bound-by-controller: yes
               volume.beta.kubernetes.io/storage-provisioner: pd.csi.storage.gke.io
               volume.kubernetes.io/storage-provisioner: pd.csi.storage.gke.io
               volume.kubernetes.io/storage-resizer: pd.csi.storage.gke.io

in the strimzi PVCs (no matter if the expansion succeeded or not) I see multiple annotations missing, including the storage-resizer annotation:

annotations:
    pv.kubernetes.io/bind-completed: "yes"
    strimzi.io/delete-claim: "false"

digging deeper I saw that after the annotation is added, strimzi-cluster-operator patched the PVC without these annotations and overwrote the PVC annotation which CSI migration depend on to correctly resize the PVC.

Strimzi version

0.35.0

Kubernetes version

1.23+

[scholzj]

I'm not sure I exactly follow the issue. The resizing works a little differently on every platform and Strimzi is platform agnostic. And I definitely don't think there is any general issue in resizing the volumes. Strimzi does not really know your environment and cannot set any custom annotations based on it. But if you want, you can add your own annotations through the template section of the Kafka resource: https://strimzi.io/docs/operators/latest/full/configuring.html#type-KafkaClusterTemplate-reference

[leiyiz]

Thanks for the response, by

to have them ignored

do you mean that it's actually an intentional decision to overwrite the annotations on k8s objects? if that's the case, then yes let's get the annotation volume.kubernetes.io/storage-resizer ignored in that process.

volume.kubernetes.io/storage-resizer is the one causing issue with k8s csi migration (I think AWS also have the same problem as they are part of csi migration too so I didn't say just gke). However in general I think the other ones in the list are good to add too.

[scholzj]

I think I tested resizing on AWS with EBS CSI driver in 0.35 and it worked fine - but I do not remember the version of the driver and other details out of my head. Anyway, I will add all of them to the list. Thanks for raising this ... and sorry, I think I did not fully understand the issue after the first post. This indeed looks like a bug.

[leiyiz]

yeah, EBS CSI driver would be working fine, the issue here is that if you use in-tree driver in a stoageclass such as aws-ebs or gce-pd on a cluster version post-migration (say 1.26) then the migration code path kicks in (where I mentioned above the CanSupport() function - then the resize don't work because without annotation the CSI driver which is supposed to take over the in-tree pvc requests don't know it supports those PVCs

[leiyiz]

Thanks for all the discussion - I could've described the issue clearer

[scholzj]

Ok, I merged the fix. Thanks a lot for reporting this.

[msau42]

Thanks for fixing these specific annotations. In general though, I think it would be better to ignore all annotation keys with k8s.io or kubernetes.io suffixes. Kubernetes features may add new annotations so this would be better future proofing.

[scholzj]

There is a proposal in progress about moving to use server-side-apply, which should address this in general. That should also provide a better solution than just ignoring some annotations.

Many of the kubernetes.io are driven from the Strimzi resources by the users and not set by some Kubernetes tooling. That does not seem to be the case here ... but for example the annotations for internal load balancers etc. So simply ignoring everything with the kubernetes.io domain in it would cause problems elsewhere.