✨ (WIP) Extract the logic to add cluster annotations to the driver interface and add unit and integration tests #750
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dtclxy64 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…and add unit and integration tests Signed-off-by: dtclxy64 <70486866+dtclxy64@users.noreply.github.com>
dtclxy64
force-pushed
the
feature/spoke-agent-registration-aws-irsa
branch
from
e5444b4 to
cea6a14
Compare
Signed-off-by: dtclxy64 <70486866+dtclxy64@users.noreply.github.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #750 +/- ##
==========================================
+ Coverage 63.33% 63.39% +0.05%
==========================================
Files 187 187
Lines 17921 17928 +7
==========================================
+ Hits 11351 11366 +15
+ Misses 5635 5628 -7
+ Partials 935 934 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -71,6 +71,9 @@ type RegisterDriver interface { | |||
| // InformerHandler returns informer of the related object. If no object needs to be watched, the func could | |||
| // return nil, nil. | |||
| InformerHandler(option any) (cache.SharedIndexInformer, factory.EventFilterFunc) | |||
|
|
|||
| // AddClusterAnnotations adds cluster annotations for non-CSR drivers | |||
| AddClusterAnnotations(clusterAnnotations map[string]string, managedClusterArn string, managedClusterRoleSuffix string) | |||
There was a problem hiding this comment.
I could understand the intension. But I wonder it is suitable for an interface since it seems quite specific to a certain implementation. I am thinking whether it makes more sense to have a method like
ClusterDecorator(cluster *v1.ManagedCluster) *v1.ManagedCluster
and call it in the creatingCluster controller.
I can try to create an example PR to show how it looks like.
There was a problem hiding this comment.
@qiujian16 We are incorporating the changes from #752 into our changes. We'll let you know once we are ready for another around of reviews. Thanks for creating the example PR to demonstrate the changes.
And refactor creating to controller to call decorators Signed-off-by: Jian Qiu <jqiu@redhat.com>
dtclxy64
changed the title
|
PR needs rebase. 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-sigs/prow repository. |
Summary
Extract the logic to add cluster annotations to the driver interface.
Add unit and integration tests.
Related issue(s)
Fixes # #514