Skip to content

Commit

Permalink
fix(usage): add nil checks to avoid panic (#1720)
Browse files Browse the repository at this point in the history
Once a PVC create request is processed by the CAST,
the response is inspected to fetch the PVC name
and other details to generate an analytics log.

This was done as part of the PR: #1708

This commit adds a nil check to handle the case of an error response
to avoid a panic.

Signed-off-by: kmova <kiran.mova@mayadata.io>
  • Loading branch information
kmova authored Jul 1, 2020
1 parent 4c064a3 commit d70090d
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 7 deletions.
1 change: 1 addition & 0 deletions changelogs/unreleased/1720-kmova
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fix(apiserver): panic when PVC is created with name containing greater than 63 characters
11 changes: 7 additions & 4 deletions cmd/maya-apiserver/app/server/volume_endpoint_v1alpha1.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ type volumeAPIOpsV1alpha1 struct {
}

// sendEventOrIgnore sends anonymous volume (de)-provision events
func sendEventOrIgnore(cvol *v1alpha1.CASVolume, pvc, method string) {
func sendEventOrIgnore(cvol *v1alpha1.CASVolume, pvcName, method string) {
if menv.Truthy(menv.OpenEBSEnableAnalytics) && cvol != nil {
usage.New().Build().ApplicationBuilder().
SetVolumeType(cvol.Spec.CasType, method).
SetDocumentTitle(cvol.ObjectMeta.Name).
SetCampaignName(pvc).
SetCampaignName(pvcName).
SetLabel(usage.EventLabelCapacity).
SetReplicaCount(cvol.Spec.Replicas, method).
SetCategory(method).
Expand All @@ -85,8 +85,11 @@ func (s *HTTPServer) volumeV1alpha1SpecificRequest(resp http.ResponseWriter, req
switch req.Method {
case "POST":
cvol, err := volOp.create()
pvc := cvol.ObjectMeta.Labels["openebs.io/persistent-volume-claim"]
sendEventOrIgnore(cvol, pvc, usage.VolumeProvision)
pvcName := ""
if err != nil && cvol != nil && cvol.ObjectMeta.Labels != nil {
pvcName = cvol.ObjectMeta.Labels["openebs.io/persistent-volume-claim"]
}
sendEventOrIgnore(cvol, pvcName, usage.VolumeProvision)
return cvol, err
case "GET":
return volOp.httpGet()
Expand Down
6 changes: 5 additions & 1 deletion cmd/provisioner-localpv/app/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ func (p *Provisioner) Delete(pv *v1.PersistentVolume) (err error) {
size = pv.Spec.Capacity["storage"]
}

sendEventOrIgnore(pv.Spec.ClaimRef.Name, pv.Name, size.String(), pvType, analytics.VolumeDeprovision)
pvcName := ""
if pv.Spec.ClaimRef != nil {
pvcName = pv.Spec.ClaimRef.Name
}
sendEventOrIgnore(pvcName, pv.Name, size.String(), pvType, analytics.VolumeDeprovision)
if pvType == "local-device" {
err := p.DeleteBlockDevice(pv)
if err != nil {
Expand Down
11 changes: 11 additions & 0 deletions tests/artifacts/jiva-pvc-invalid-name.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
kind: PersistentVolumeClaim
apiVersion: v1
metadata:
name: jiva-pvc-more-than-63-chars-long-due-helm-sub-chart-insallation-or-some-crazy-logic
spec:
storageClassName: openebs-jiva-default
accessModes:
- ReadWriteOnce
resources:
requests:
storage: "5G"
4 changes: 2 additions & 2 deletions tests/artifacts/jiva-pvc.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
kind: PersistentVolumeClaim
apiVersion: v1
metadata:
name: cstor-testvolume-pvc
name: jiva-testvolume-pvc
namespace: openebs
spec:
storageClassName: openebs-jiva-default
accessModes:
- ReadWriteOnce
resources:
requests:
storage: "5G"
storage: "5G"
34 changes: 34 additions & 0 deletions tests/artifacts/local-d-pvc-invalid-name.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
kind: PersistentVolumeClaim
apiVersion: v1
metadata:
name: device-pvc-more-than-63-chars-long-due-helm-sub-chart-insallation-or-some-crazy-logic
spec:
storageClassName: openebs-device
accessModes:
- ReadWriteOnce
resources:
requests:
storage: "5G"
---
apiVersion: v1
kind: Pod
metadata:
name: busybox-device
namespace: default
spec:
containers:
- command:
- sh
- -c
- 'date > /mnt/store1/date.txt; hostname >> /mnt/store1/hostname.txt; sync; sleep 5; sync; tail -f /dev/null;'
image: busybox
imagePullPolicy: Always
name: busybox
volumeMounts:
- mountPath: /mnt/store1
name: demo-vol1
volumes:
- name: demo-vol1
persistentVolumeClaim:
claimName: device-pvc-more-than-63-chars-long-due-helm-sub-chart-insallation-or-some-crazy-logic
---
34 changes: 34 additions & 0 deletions tests/artifacts/local-pvc-long-name.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
kind: PersistentVolumeClaim
apiVersion: v1
metadata:
name: hostpath-pvc-more-than-63-chars-long-due-helm-sub-chart-insallation-or-some-crazy-logic
spec:
storageClassName: openebs-hostpath
accessModes:
- ReadWriteOnce
resources:
requests:
storage: "5G"
---
apiVersion: v1
kind: Pod
metadata:
name: busybox-hp
namespace: default
spec:
containers:
- command:
- sh
- -c
- 'date > /mnt/store1/date.txt; hostname >> /mnt/store1/hostname.txt; sync; sleep 5; sync; tail -f /dev/null;'
image: busybox
imagePullPolicy: Always
name: busybox
volumeMounts:
- mountPath: /mnt/store1
name: demo-vol1
volumes:
- name: demo-vol1
persistentVolumeClaim:
claimName: hostpath-pvc-more-than-63-chars-long-due-helm-sub-chart-insallation-or-some-crazy-logic
---

0 comments on commit d70090d

Please sign in to comment.