chore(appengine): Persistent volumes .persistentVolumeReclaimPolicy n…

[gburnard]

…ow set to Retain

Description

Persistent volumes .persistentVolumeReclaimPolicy now set to Retain, by default.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• Technical improvement or removal of technical debt.
• Minor change: comments, documentation, trivial fixes

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Unit Tests

Optional:

• Functional Tests
• Integration Tests

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• I have checked my code and corrected any misspellings

[conneryn]

I'm curious about the use-case behind this change. I assume this is coming from snapshot/volume management requirements, but is this something specific to AppEngine, or do we want this retention policy to be the default for ALL applications and their volumes?

If it's for all applications, could it be better to manage this:
a. in the provisioner/common,
c. by harbourmaster (periodically check for and modify volumes that are not set to Retain), or
b. by c6o system (ex: at install, create a new default custom storage class that applies this retention policy)?

[gburnard]

Good feedback @conneryn. @robblovell will likely be the best person to respond.

[robblovell]

assume this is coming from snapshot/volume management requirements, but is this something specific to AppEngine, or do we want this retention policy to be the default for ALL applications and their volumes?

Hey Connery,

Currently, when a volume is detached, the volume management system sets the volume to 'retain' as part of the process. At one point I had thought that it would assume it was 'retain', but decided after Grant's implementation to default the value to 'retain' to do this. So volume management no longer needs the value to be 'retain'.

However, I think it would be surprising to any app developer if they installed an app and detached the volume that the volume data would disappear. During development, that might be desirable, but during normal operation, I would expect databases and storage volumes to be retained unless explicitly deleted.

Because of this, I think we should keep logic to set volumes to 'retain' for all applications regardless of whether the storage policy for the cloud is something else like 'delete'.

[robblovell]

I think this should be just an App Engine feature for now.