Set OwnerReference on Plugin resources #845
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zubron
force-pushed
the
set-owner-reference-818
branch
from
5f73888 to
17b70f1
Compare
|
Marking this as a draft until I add some tests for |
zubron
force-pushed
the
set-owner-reference-818
branch
from
17b70f1 to
f9a01da
Compare
Codecov Report
@@ Coverage Diff @@
## master #845 +/- ##
==========================================
+ Coverage 46.29% 46.67% +0.38%
==========================================
Files 75 75
Lines 4800 4831 +31
==========================================
+ Hits 2222 2255 +33
Misses 2448 2448
+ Partials 130 128 -2
Continue to review full report at Codecov.
|
johnSchnake
reviewed
Aug 16, 2019
pkg/plugin/aggregation/update.go
Outdated
| func GetAggregatorPodName(client kubernetes.Interface, namespace string) (string, error) { | ||
| ap, err := GetAggregatorPod(client, namespace) | ||
|
|
||
| if _, ok := err.(NoPodWithLabelError); ok { |
There was a problem hiding this comment.
nit: when doing multi-step error checking like this I tend to prefer a switch statement so it is a single block and the if err != nil section doesn't drift away from the call generating the error.
if err != nil {
switch err.(type) {
case * NoPodWithLabelError:
fmt.Println(err.Error())
default:
fmt.Println("ah!?")
}
}
It is one extra layer of indentation, but effectively one unit of error handling. (again, just a nit)
zubron
force-pushed
the
set-owner-reference-818
branch
from
f9a01da to
4147bfb
Compare
zubron
force-pushed
the
set-owner-reference-818
branch
from
4147bfb to
f01c97a
Compare
johnSchnake
approved these changes
Aug 16, 2019
When creating Pod or DaemonSet resources when running plugins, we didn't mark them as owned by the Sonobuoy aggregator pod. This meant that if the aggregator pod was deleted, the resources created by that pod would be orphaned and would have to be deleted manually. This change sets the OwnerReference for all resources to be the Sonobuoy aggregator pod so that when the aggregator is deleted, all child resources are also deleted. Signed-off-by: Bridget McErlean <[email protected]>
zubron
force-pushed
the
set-owner-reference-818
branch
from
687e71a to
86d4c6f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
When creating Pod or DaemonSet resources when running plugins, we didn't
mark them as owned by the Sonobuoy aggregator pod. This meant that if
the aggregator pod was deleted, the resources created by that pod would
be orphaned and would have to be deleted manually. This change sets the
OwnerReference for all resources to be the Sonobuoy aggregator pod so
that when the aggregator is deleted, all child resources are also
deleted.
Signed-off-by: Bridget McErlean [email protected]
Which issue(s) this PR fixes
Release note: