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

Set DeleteOnTermination for in BlockDeviceMappings to false in case of volumes with RootDiskType #114

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

dergeberl
Copy link
Contributor

What this PR does / why we need it:

In case of a RootDiskType is specified the volume will be created before creating the server. Also while deletion the server gets deleted first and than also the volume is getting deleted.
In the BlockDeviceMapping currently we set DeleteOnTermination: true this is wrong because the volume gets deleted by the mcm.

Special notes for your reviewer:

Release note:

Set DeleteOnTermination for in BlockDeviceMappings to false in case of volumes with RootDiskType.

@dergeberl dergeberl requested review from a team as code owners February 20, 2024 13:54
@gardener-robot gardener-robot added the needs/review Needs review label Feb 20, 2024
@gardener-robot
Copy link

@dergeberl Thank you for your contribution.

@gardener-robot gardener-robot added the size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) label Feb 20, 2024
@gardener-robot-ci-3
Copy link
Contributor

Thank you @dergeberl for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

Copy link
Contributor

@kon-angelo kon-angelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Feb 22, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 22, 2024
@kon-angelo
Copy link
Contributor

@dergeberl I understand that since we already have code to delete the volume, that setting this flag is redundant and this change is fine. Out of curiosity, did you encounter and issue with that so far, or just removing the redundancy ?

@dergeberl
Copy link
Contributor Author

Out of curiosity, did you encounter and issue with that so far, or just removing the redundancy ?

We had rare problems while deletion of a volume. During that we found this redundancy. I am not sure if this was the root cause of that problem but I wanted to change it to have clear responsibilities who deleted the volume.

@kon-angelo kon-angelo merged commit e7da468 into gardener:master Feb 22, 2024
6 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Feb 22, 2024
@dergeberl dergeberl deleted the DeleteOnTerminationWithVolume branch February 22, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants