openshift / aws-efs-operator

Operator to manage AWS EFS on OpenShift
Apache License 2.0
3 stars 23 forks source link

Fix pv[c]Ensurable caching bug #18

Closed 2uasimojo closed 4 years ago

2uasimojo commented 4 years ago

We were neglecting to clear the pvEnsurable and pvcEnsurable caches when deleting their respective objects. This resulted in behavior where creating a SharedVolume, deleting it, then recreating one with the same name but a different spec would result in the old spec being used.

Fix this by explicitly clearing the cache entry if we attempt to delete the resource (even if that deletion fails).

On the test side, completely rework the test case to be fake- rather than mock-driven.

(The same principle should be applied throughout so we can hopefully get rid of the mocked client entirely. That'll be the subject of (a) different PR(s), tracked by OSD-4098.)

Jira: None

codecov-commenter commented 4 years ago

Codecov Report

Merging #18 into master will increase coverage by 0.12%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #18      +/-   ##
==========================================
+ Coverage   74.11%   74.23%   +0.12%     
==========================================
  Files          18       18              
  Lines         618      621       +3     
==========================================
+ Hits          458      461       +3     
  Misses        153      153              
  Partials        7        7              
Impacted Files Coverage Δ
...controller/sharedvolume/sharedvolume_controller.go 85.71% <100.00%> (+0.25%) :arrow_up:
2uasimojo commented 4 years ago

Tested this live as well. Works.

jharrington22 commented 4 years ago

/approve /lgtm

openshift-ci-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, jharrington22

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: - ~~[OWNERS](https://github.com/openshift/aws-efs-operator/blob/master/OWNERS)~~ [2uasimojo,jharrington22] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment