Adding check if disk description is empty

[slavina-rumenova]

In order to prevent velero backups of being marked as PartialyFailed for disks, having no description set (as it is an optional argument), adding a check if the disk description is empty before parsing the description json. If empty only the velero tags are set on the snapshot.

Change fixes vmware-tanzu/velero#3149.

Signed-off-by: Slavina Sobadjieva slavina.rumenova@gmail.com

[carlisia]

This lgtm and thank you.

I wonder if it'd be useful to add a warning on LN 294 that the description is not being used.

[Helter-skelter42]

Any update on this one ? :)

[jravetch]

We are running into this issue with disks that do not have a description. When can we expect a new release for the plugin?

[carlisia]

I am going to push an image for the next release of this plugin (v1.2.0) tomorrow. It should still be compatible with the latest versions of Velero. There'll be a Velero RC for v1.6 this week.

[jravetch]

@carlisia Thank you! This commit e677e89 states that plugin 1.2.0 is only compatible with velero 1.6.0. Also can you add to the changelog the bug that is fixed by this PR?

[carlisia]

The v1.2.0 image for this plugin is available.

@jravetch thanks for the callout, I'm making a note to add this to the release changelog. I haven't cut that yet, we are testing things first.

We would like for people to upgrade Velero as they upgrade their plugins but there is no known compatibility issues between the new v1.2.0 and the earlier versions of Velero.

[jravetch]

@carlisia I updated the deployment to use the new plugin version, but I am still getting the same error. Below is the output when I describe the latest backup run

Phase:  PartiallyFailed (run `velero backup logs velero-schedule-config-20210317194135` for more information)

Errors:    1             
Warnings:  0
                            
Namespaces:
  Included:  *                     
  Excluded:  kube-system, velero
                
Resources:
  Included:        *
  Excluded:        <none>
  Cluster-scoped:  auto                    
                                                                                                                                                                                                                                                                                                                                                          
Label selector:  <none>                                                                                              
                                                                                                                   
Storage Location:  default                                                                                                                                                                                                                                                                                                                                

Velero-Native Snapshot PVs:  auto

TTL:  720h0m0s

Hooks:  <none>

Backup Format Version:  1.1.0

Started:    2021-03-17 19:41:35 +0000 UTC
Completed:  2021-03-17 19:43:45 +0000 UTC

Expiration:  2021-04-16 19:41:35 +0000 UTC

Total items to be backed up:  9587
Items backed up:              9587

Velero-Native Snapshots:  14 of 14 snapshots completed successfully (specify --details for more information)

Why isn't the backup format version changed? Am I missing something?

[carlisia]

Let me look into it. Why did you expect the backup format version to be different?

Also, let me see what your backup log says, please paste all of it for me.

[jravetch]

Oh I thought the format version was tied to the plugin version. Here's the output of the logs, grepping only for errors.

time="2021-03-17T19:42:43Z" level=error msg="unable to decode disk's description as JSON, so only applying Velero-assigned tags to snapshot" backup=velero/velero-schedule-config-20210317194135 cmd=/plugins/velero-plugin-for-gcp error="invalid character 'p' looking for beginning of value" logSource="/go/src/velero-plugin-for-gcp/velero-plugin-for-gcp/volume_snapshotter.go:283" pluginName=velero-plugin-for-gcp

Checking the description of the disk, it's not in JSON, just a string. So the plugin is still throwing the error and marking the backup as partially failed.

[carlisia]

Just for sanity, what's the output of kubectl describe pod velero | grep Image:?

[jravetch]

    Image:          velero/velero-plugin-for-gcp:v1.2.0
    Image:          velero/velero:v1.5.3

[jravetch]

So i believe if this line is a string, but not JSON, the condition will still fail.

[carlisia]

Unfortunately I didn't verify that a test be added for this new condition, which would've served as a double purpose of clarifying the intent of the change.

The conditions that determine if Velero tag will be applied or not are listed here:

https://github.com/vmware-tanzu/velero-plugin-for-gcp/blob/main/velero-plugin-for-gcp/volume_snapshotter_test.go#L93-L134

[jravetch]

So there is already a test case. Can we change the error log to just info instead?

log.Info("unable to decode disk's description as JSON, so only applying Velero-assigned tags to snapshot")