Unproject workloads no longer targeted by a ServiceBinding #348
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #348 +/- ##
==========================================
- Coverage 78.36% 77.72% -0.64%
==========================================
Files 19 19
Lines 1627 1697 +70
==========================================
+ Hits 1275 1319 +44
- Misses 281 305 +24
- Partials 71 73 +2
☔ View full report in Codecov by Sentry. |
scothis
force-pushed
the
unbind-selector
branch
from
11846dc to
5bbd723
Compare
|
@scothis you might want to test against the branch from servicebinding/conformance#74 locally as well - should get rid of the flakiness. I'm currently doing so - awaiting results. |
When a previously bound workload is no longer targeted by a ServiceBinding it is now unprojected. Previously, the projection was orphaned as the controller only managed workload matching the reference on the ServiceBinding resource. This could happen for a few different reasons: - the name of the referenced workload was updated on the ServiceBinding - the label selector matching the workload was updated on the ServiceBinding - the labels on the workload were updated to no longer match the selector on the ServiceBinding. In order to find previously bound workloads, we now list all resources matching the workload refs GVK in the namespace. That list is filtered to resources that are currently projected, or match the new criteria. All matching workloads are unprojected, but only resources matching the current ref are re-projected. To avoid a much larger search area, the workloadRef apiVersion and kind fields are now immutable. Users who need to update either of these values will need to delete the ServiceBinding and create a new resource with the desired values. Signed-off-by: Scott Andrews <[email protected]>
scothis
force-pushed
the
unbind-selector
branch
from
2bfc37f to
482955c
Compare
|
@sadlerap the two test failures are verified flakes. The first workload was correctly never bound, while the second workload was correctly bound. acceptance-test-results-v1.21.14.zip |
resolver/cluster.go
Outdated
| sort.Slice(workloads, func(i, j int) bool { | ||
| return workloads[i].(metav1.Object).GetName() < workloads[j].(metav1.Object).GetName() | ||
| }) |
There was a problem hiding this comment.
Is sorting necessary here? From usage of this method, I don't see that it is (unless StashWorkloads requires input to be sorted by name).
There was a problem hiding this comment.
strictly speaking no, but for tests observing side effects the order matters. In practice the api server returns items in order, but there is no guarantee. I added this because I was seeing tests fail because the items were out of order.
|
|
||
| list := &unstructured.UnstructuredList{} | ||
| list.SetAPIVersion(workloadRef.APIVersion) | ||
| list.SetKind(fmt.Sprintf("%sList", workloadRef.Kind)) |
There was a problem hiding this comment.
For compatibility's sake, some resources might have a list resource that doesn't line up with the naming format we assume here. I don't know of any that would also be usable as a workload (since, to my knowledge, it really goes against convention), but it may be a thing we need to be aware of since it's allowed by the API.
Maybe add a todo here so we don't lose track of that? I don't think it needs to be a part of this PR, but it's something that we'll eventually need to support.
Signed-off-by: Scott Andrews <[email protected]>
When a previously bound workload is no longer targeted by a ServiceBinding it is now unprojected. Previously, the projection was orphaned as the controller only managed workload matching the reference on the ServiceBinding resource. This could happen for a few different reasons:
In order to find previously bound workloads, we now list all resources matching the workload refs GVK in the namespace. That list is filtered to resources that are currently projected, or match the new criteria. All matching workloads are unprojected, but only resources matching the current ref are re-projected.
To avoid a much larger search area, the workloadRef apiVersion and kind fields are now immutable. Users who need to update either of these values will need to delete the ServiceBinding and create a new resource with the desired values.
Resolves #345