Add more sidecar tests and cleanup sidecar code #266
Conversation
k8s-ci-robot
added
release-note-none
|
snapshot_controller.go at 81.8% coverage now, was 61.3% |
Signed-off-by: Grant Griffiths <[email protected]>
ggriffiths
force-pushed
the
add_more_sidecar_ctrl_tests
branch
from
6f02362 to
fe1c355
Compare
| ctrl.updateContentErrorStatusWithEvent(content, v1.EventTypeWarning, "SnapshotCreationFailed", fmt.Sprintf("Failed to create snapshot with error %v", err)) | ||
| return err | ||
| } | ||
| ctrl.createSnapshot(content) |
There was a problem hiding this comment.
These functions will never return an error because they just schedule async operations. Each async operation creates an event upon any failures. Deletion seems to be implemented similarly (see above L66 and 72), except it always just passes the nil error further up the stack.
|
The issue #191 you referenced is not an issue. It was a PR from you that was merged. Do you want to open another issue to track this? |
|
I opened a new issue to track this. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ggriffiths, xing-yang 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 |
k8s-ci-robot
added
the
approved
Signed-off-by: Grant Griffiths [email protected]
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #267
Special notes for your reviewer:
n/a
Does this PR introduce a user-facing change?: