refact: namespace all labels in the helm charts

[wilmardo]

Describe what this PR does

• Removes heritage and release label
• Renames:
  • app > app.kubernetes.io/name
  • chart > helm.sh/chart:
  • component > app.kubernetes.io/component
• Adds:
  • app.kubernetes.io/managed-by label
  • app.kubernetes.io/managed-by

This results in the following labels:

    labels:
      app.kubernetes.io/component: provisioner
      app.kubernetes.io/instance: ceph-csi-rbd
      app.kubernetes.io/managed-by: Helm
      app.kubernetes.io/name: ceph-csi-rbd
      app.kubernetes.io/version: canary
      helm.sh/chart: ceph-csi-rbd-1.3.0-canary

Is there anything that requires special attention

The change isn't backwards compatible, helm upgrade will fail since labels are immutable after an deployment:

# helm upgrade ceph-csi-rbd charts/ceph-csi-rbd --namespace storing
Error: UPGRADE FAILED: cannot patch "ceph-csi-rbd-nodeplugin" with kind DaemonSet: DaemonSet.apps "ceph-csi-rbd-nodeplugin" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/component":"nodeplugin", "app.kubernetes.io/instance":"ceph-csi-rbd", "app.kubernetes.io/managed-by":"Helm", "app.kubernetes.io/name":"ceph-csi-rbd", "app.kubernetes.io/version":"canary"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && cannot patch "ceph-csi-rbd-provisioner" with kind Deployment: Deployment.apps "ceph-csi-rbd-provisioner" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/component":"provisioner", "app.kubernetes.io/instance":"ceph-csi-rbd", "app.kubernetes.io/managed-by":"Helm", "app.kubernetes.io/name":"ceph-csi-rbd", "app.kubernetes.io/version":"canary"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable

Can be resolved by uninstalling the current release and redeploying with the same values but this should be mentioned and this must be a major Helm chart release.

Related issues

Fixes: #612

[Madhu-1]

if you are using cephfs fuse client or rbd-nbd driver, This could be an issue as we need to delete the daemonset of both cephfs and rbd (all mounts inside the daemonset container will be gone you will see transport endpoint not connected issues) #703

we need to document the step on how to do upgrade for the same

[wilmardo]

@Madhu-1 Are the upgrade scenarios described in #770? Is there more documentation needed?

[Madhu-1]

@wilmardo we need to add a document for the upgrade with helm charts

[humblec]

@wilmardo can you rebase this PR ?

[wilmardo]

@humblec Will do in the next couple days :)

[humblec]

@wilmardo I hope we are progressing on this :) .

[wilmardo]

@Madhu-1 Thanks for the reminder, working on it as we speak. Rebased to current master but found some missed values when reviewing myself, on it as we speak!

[humblec]

@wilmardo Thanks ! :) , iic the last comment was for me .

[humblec]

@wilmardo may be little more efforts to make CI happy ? :)

[wilmardo]

@humblec Done, didn't notice that the CI failed 👍

[humblec]

@humblec Done, didn't notice that the CI failed +1

Cool. thanks . The PR looks good to me ! LGTM.

[humblec]

@Madhu-1 ptal .

[Madhu-1]

This is a breaking change, we need upgrade doc for this one and we cannot track this for 2.1.0

[Madhu-1]

this is a breaking change we need to see what we can do on this one. @nixpanic any suggestion from your side on this one?

[nixpanic]

@wilmardo could you write down your recommendation for upgrading a current deployment to the new label/format?

[humblec]

@wilmardo ping .. revisit?

[wilmardo]

Yeah busy with work and not that much time for open source at the moment.

Don't know if there is an casual upgrade path for this, Kubernetes sees these labels as immutable so it will error on trying to upgrade.
I don't know if it is feasible to remove and reinstall for user?

[Madhu-1]

Yeah busy with work and not that much time for open source at the moment.

Don't know if there is an casual upgrade path for this, Kubernetes sees these labels as immutable so it will error on trying to upgrade.
I don't know if it is feasible to remove and reinstall for user?

Thanks for the reply @wilmardo even I have the same concern as it's not feasible to reinstall the whole cluster

[wilmardo]

Might be something for the 3.x.x release then?
I deleted and redeployed my ceph pods without problems in the past, since the PV and PVC keep existing for the current pods I had no downtime.
Needs testing though, it has been sometime since I have done that so my memory might be incorrect.

[Madhu-1]

Might be something for the 3.x.x release then?
I deleted and redeployed my ceph pods without problems in the past, since the PV and PVC keep existing for the current pods I had no downtime.
Needs testing though, it has been sometime since I have done that so my memory might be incorrect.

looks like we can do it. but it will be a downtime for the user. application pods cannot use PVC during the upgrade as we are deleting and recreating all the cephcsi pods. if we can add documentation for the upgrade we can do it.
as you said we need good testing for it for both cephfs and rbd.

[mergify]

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

[Madhu-1]

moved this out of release-v3.0.0 milestone

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in a month if no further activity occurs. Thank you for your contributions.

[Madhu-1]

Might be something for the 3.x.x release then?
I deleted and redeployed my ceph pods without problems in the past, since the PV and PVC keep existing for the current pods I had no downtime.
Needs testing though, it has been sometime since I have done that so my memory might be incorrect.

looks like we can do it. but it will be a downtime for the user. application pods cannot use PVC during the upgrade as we are deleting and recreating all the cephcsi pods. if we can add documentation for the upgrade we can do it.
as you said we need good testing for it for both cephfs and rbd.

@wilmardo any comment on this one?

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in a month if no further activity occurs. Thank you for your contributions.

[stale]

This pull request has been automatically closed due to inactivity. Please re-open if these changes are still required.