Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(helm): configure version for snapshot api #361

Merged
merged 1 commit into from
Jul 16, 2021
Merged

fix(helm): configure version for snapshot api #361

merged 1 commit into from
Jul 16, 2021

Conversation

kmova
Copy link
Contributor

@kmova kmova commented Jul 16, 2021

On some managed K8s clusters like OpenShift or GKE that ship with
their own default CSI Drivers, the controllers will end up creating
v1beta1 objects. An issue like the following was noticed when running
cstor charts on GKE 1.19.

 unable to recognize "": no matches for kind "VolumeSnapshotClass" in
 version "snapshot.storage.k8s.io/v1"

This commit uses the helm .Capabilities to check the version available
in the cluster and install the class accordingly. As the .Capabilities
might return false when using with template command, v1 is kept for false
(or default) case.

Manually verified on GKE with and without setting the flag as follows:

kiran_mova_mayadata_io@kmova-dev:charts$ helm install openebs-cstor . -n openebs --create-namespace --set openebsNDM.enabled =false 

NAME: openebs-cstor
LAST DEPLOYED: Fri Jul 16 16:55:45 2021
NAMESPACE: openebs
STATUS: deployed
REVISION: 1
TEST SUITE: None
NOTES:
The OpenEBS cstor has been installed check its status by running:
....

Also, verified that v1 version is generated when running helm template

Signed-off-by: kmova kiran.mova@mayadata.io

@shovanmaity
Copy link
Contributor

We can make use of built in object Capabilities.APIVersions.Has to know if that version is available in the cluster or not. One reference is here - https://github.com/helm/charts/blob/f4f301ae450101b981805bd045451f08c0d74afa/stable/rethinkdb/templates/rethinkdb-cluster-storage-class.yaml#L2-L6

On some managed K8s clusters like OpenShift or GKE that ship with
their own default CSI Drivers, the controllers will end up creating
v1beta1 objects. An issue like the following was noticed when running
cstor charts on GKE 1.19.

```
 unable to recognize "": no matches for kind "VolumeSnapshotClass" in
 version "snapshot.storage.k8s.io/v1"
```

This commit uses the helm .Capabilities to check the version available
in the cluster and install the class accordingly. As the .Capabilities
might return false when using with template command, v1 is kept for false
(or default) case.

Signed-off-by: kmova <kiran.mova@mayadata.io>

fix(helm): replacing flag with auto-detect

Signed-off-by: kmova <kiran.mova@mayadata.io>
@kmova
Copy link
Contributor Author

kmova commented Jul 16, 2021

Thanks @shovanmaity - using .Capabilities is very clean. Updated the code.

@prateekpandey14 prateekpandey14 merged commit 3cef351 into openebs-archive:master Jul 16, 2021
@shovanmaity
Copy link
Contributor

shovanmaity commented Jul 16, 2021

One more changes requested. Sorry I have not explained it explained properly..
There could be 2 versions present but kind can belong to any one of them so we need to include kind also

apiVersion: {{ if .Capabilities.APIVersions.Has "snapshot.storage.k8s.io/v1beta1/VolumeSnapshotClass" -}}
  snapshot.storage.k8s.io/v1beta1
{{- else -}}
  snapshot.storage.k8s.io/v1
{{- end }}

ref - https://helm.sh/docs/chart_template_guide/builtin_objects

@shovanmaity
Copy link
Contributor

shovanmaity commented Jul 16, 2021

$ kubectl api-versions | grep "snapshot.storage.k8s.io"
snapshot.storage.k8s.io/v1
snapshot.storage.k8s.io/v1beta1
$ kubectl api-resources | grep VolumeSnapshotClass
volumesnapshotclasses                                              snapshot.storage.k8s.io/v1beta1             false        VolumeSnapshotClass

As we are providing only v1 crd according to this code if v1 crd installed it will fail as snapshot.storage.k8s.io/v1 and snapshot.storage.k8s.io/v1beta1 both are available and snapshot.storage.k8s.io/v1/VolumeSnapshotClass is available but snapshot.storage.k8s.io/v1beta1/VolumeSnapshotClass is not available

@kmova
Copy link
Contributor Author

kmova commented Jul 17, 2021

Above comment is fixed in #362

@kmova kmova mentioned this pull request Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants