Fix listsnapshot clean up

[wnxn]

#132

[wnxn]

/assign @lpabon

[pohly]

Can you explain in the commit message why this change is needed? In other words, what problem does it fix?

[wnxn]

Hi, @pohly
In some storage server, snapshot is related on a specific volume. If user delete the volume, the snapshot will
be deleted automatically at storage server side. For this reason, we must keep the order of deletion, that is deleting volume‘s snapshots before deleting the volume. It's found that the author might take the order of deletion into consideration when writing other snapshot test cases.
Just like this:

csi-test/pkg/sanity/controller.go

Lines 1251 to 1259 in 3e3231b

	By("cleaning up deleting the snapshot")
	delSnapReq := MakeDeleteSnapshotReq(sc, snapshot.GetSnapshot().GetId())
	_, err = c.DeleteSnapshot(context.Background(), delSnapReq)
	Expect(err).NotTo(HaveOccurred())
	By("cleaning up deleting the volume")
	delVolReq := MakeDeleteVolumeReq(sc, volume.GetVolume().GetId())
	_, err = c.DeleteVolume(context.Background(), delVolReq)
	Expect(err).NotTo(HaveOccurred())

[pohly]

But what breaks when the order is as it is now? Does the test fail?

I am trying to understand whether this change works around unexpected behavior of one particular storage backend or is actually broken.

@xing-yang can you perhaps comment on this?

[xing-yang]

For some storage systems, snapshot has to be deleted first before deleting the volume. If you delete a volume with dependent snapshot, it may cause error in some storage systems. For example, Cinder prevents you from deleting a volume with dependent snapshots. For Hostpath driver, this is not a problem as the snapshot does not depend on the volume. So this change looks good to me.

[xing-yang]

lgtm

[pohly]

@wnxn I'm fine with merging this then, but I'd still like to have this information recorded in the commit message. Please use "git commit --amend" and "git push ... +master" to update the PR.

[xing-yang]

Agree that it is better to add clarification in the commit message.

[wnxn]

Thanks for @pohly and @xing-yang 's review.
I add some clarification in commit message. Would you please review again?

[pohly]

/lgtm

Just one comment for future commits: please format commit messages with explicit line breaks in the body, they are easier to read in "git log" then.

[wnxn]

Hi, @pohly
Thanks for your advice. I will format messages body in my future commits.

[pohly]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pohly, wnxn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [pohly]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment