Bug 1985015: Eliminate instanceCreate volume leak #192
Conversation
InstanceCreate is a method that creates a server. If the server is set to boot from volume, and an image ID is passed, the method creates the root volume prior to creating the server. The root volume is set to be destroyed when the associated server is destroyed. However, if the server fails to create (for example, because of quota issues), the volume is never associated to a server and the automatic deletion is never triggered. At every round of retry, a new volume will be created, possibly until volume quota is reached (or server creation is successful). This results in a leakage of unused volumes. With this change, a newly created root volume is explicitly deleted in the event of an early failure of InstanceCreate. That is to say: if InstanceCreate creates a volume, that volume will be deleted if any of the next steps fail and cause the server never to be created. Note that this patch leaves unmodified the lifespan of a volume associated to a server, regardless if the server ever reaches an ACTIVE state. Co-Authored-By: Matthew Booth <[email protected]>
openshift-ci
bot
added
the
bugzilla/severity-high
|
@pierreprinetti: This pull request references Bugzilla bug 1985015, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
added
the
bugzilla/invalid-bug
openshift-ci
bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mandre 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:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/bugzilla refresh |
openshift-ci
bot
added
the
bugzilla/valid-bug
|
@pierreprinetti: This pull request references Bugzilla bug 1985015, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 6 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
removed
the
bugzilla/invalid-bug
|
@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: eurijon. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/retest |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
aravindhp
added
the
cherry-pick-approved
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
5 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
18 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@pierreprinetti: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@pierreprinetti: All pull requests linked via external trackers have merged: Bugzilla bug 1985015 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/cherry-pick release-4.7 |
|
@pierreprinetti: new pull request created: #194 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
* Remove GetKubeConfig from the machine actuator `GetKubeConfig` is only needed in the deployer, therefore we can do away with the code we have in the machine actuator. This helps with issues openshift#174 and openshift#175 * Don't mount sshkeys on the manager's pod We've removed the last use for the sshkeys secret/mount in the manager's POD, which means we can now remove this code and stop mounting these keys. Fixes openshift#175 * Remove KeyPair related functions from the machine service We're not managing keypairs from the actuator anymore. The functions that used to manage keypairs have been removed in this commit.
5 participants
InstanceCreate is a method that creates a server. If the server is set
to boot from volume, and an image ID is passed, the method creates the
root volume prior to creating the server. The root volume is set to be
destroyed when the associated server is destroyed.
However, if the server fails to create (for example, because of quota
issues), the volume is never associated to a server and the automatic
deletion is never triggered. At every round of retry, a new volume will
be created, possibly until volume quota is reached (or server creation
is successful). This results in a leakage of unused volumes.
With this change, a newly created root volume is explicitly deleted in
the event of an early failure of InstanceCreate. That is to say: if
InstanceCreate creates a volume, that volume will be deleted if any of
the next steps fail and cause the server never to be created.
Note that this patch leaves unmodified the lifespan of a volume
associated to a server, regardless if the server ever reaches an ACTIVE
state.
Co-Authored-By: Matthew Booth [email protected]