Skip to content

Commit b0d84b8

Browse files
cnvergencecnvergence
committed
refactor reconcile code, add update func for bound schemas
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com> On-behalf-of: @SAP karol.szwaj@sap.com
1 parent a99179c commit b0d84b8

2 files changed

Lines changed: 45 additions & 72 deletions

File tree

‎backend/controllers/serviceexportrequest/serviceexportrequest_controller.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,9 @@ func NewAPIServiceExportRequestReconciler(
105105
createBoundSchema: func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error {
106106
return cl.Create(ctx, schema)
107107
},
108+
updateBoundSchema: func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error {
109+
return cl.Update(ctx, schema)
110+
},
108111
deleteServiceExportRequest: func(ctx context.Context, cl client.Client, ns, name string) error {
109112
return cl.Delete(ctx, &kubebindv1alpha2.APIServiceExportRequest{
110113
ObjectMeta: metav1.ObjectMeta{

‎backend/controllers/serviceexportrequest/serviceexportrequest_reconcile.go‎

Lines changed: 42 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,21 @@ type reconciler struct {
4747

4848
getBoundSchema func(ctx context.Context, cl client.Client, namespace, name string) (*kubebindv1alpha2.BoundSchema, error)
4949
createBoundSchema func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error
50+
updateBoundSchema func(ctx context.Context, cl client.Client, schema *kubebindv1alpha2.BoundSchema) error
5051

5152
getServiceExport func(ctx context.Context, cache cache.Cache, ns, name string) (*kubebindv1alpha2.APIServiceExport, error)
5253
createServiceExport func(ctx context.Context, cl client.Client, resource *kubebindv1alpha2.APIServiceExport) error
5354
deleteServiceExportRequest func(ctx context.Context, cl client.Client, namespace, name string) error
5455
}
5556

5657
func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error {
58+
export, err := r.getServiceExport(ctx, cache, req.Namespace, req.Name)
59+
if err != nil && !apierrors.IsNotFound(err) {
60+
return fmt.Errorf("failed to get APIServiceExport: %w", err)
61+
}
5762
// We must ensure schemas are created in form of boundSchemas first for the validation.
5863
// Worst case scenario if validation fails, we will reuse schemas for same consumer once issues are fixed.
59-
if err := r.ensureBoundSchemas(ctx, cl, cache, req); err != nil {
64+
if err := r.ensureBoundSchemas(ctx, cl, export, req); err != nil {
6065
conditions.SetSummary(req)
6166
return fmt.Errorf("failed to ensure bound schemas: %w", err)
6267
}
@@ -66,7 +71,7 @@ func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cach
6671
return fmt.Errorf("failed to validate APIServiceExportRequest: %w", err)
6772
}
6873

69-
if err := r.ensureExports(ctx, cl, cache, req); err != nil {
74+
if err := r.ensureExports(ctx, cl, export, req); err != nil {
7075
conditions.SetSummary(req)
7176
return fmt.Errorf("failed to ensure exports: %w", err)
7277
}
@@ -76,54 +81,11 @@ func (r *reconciler) reconcile(ctx context.Context, cl client.Client, cache cach
7681
return fmt.Errorf("failed to ensure APIServiceNamespaces: %w", err)
7782
}
7883

79-
if err := r.setBoundSchemaOwnerRefs(ctx, cl, cache, req); err != nil {
80-
conditions.SetSummary(req)
81-
return fmt.Errorf("failed to adopt BoundSchemas: %w", err)
82-
}
83-
8484
conditions.SetSummary(req)
8585

8686
return nil
8787
}
8888

89-
func (r *reconciler) setBoundSchemaOwnerRefs(ctx context.Context, cl client.Client, cache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error {
90-
logger := klog.FromContext(ctx)
91-
92-
export, err := r.getServiceExport(ctx, cache, req.Namespace, req.Name)
93-
if err != nil {
94-
if apierrors.IsNotFound(err) {
95-
return nil
96-
}
97-
return fmt.Errorf("failed to get APIServiceExport: %w", err)
98-
}
99-
100-
for _, res := range req.Spec.Resources {
101-
name := res.ResourceGroupName()
102-
boundSchema, err := r.getBoundSchema(ctx, cl, req.Namespace, name)
103-
if err != nil {
104-
if apierrors.IsNotFound(err) {
105-
continue
106-
}
107-
return fmt.Errorf("failed to get BoundSchema %s: %w", name, err)
108-
}
109-
110-
if err := controllerutil.SetControllerReference(export, boundSchema, cl.Scheme()); err != nil {
111-
return fmt.Errorf("failed to set owner reference on BoundSchema %s: %w", name, err)
112-
}
113-
114-
if err := cl.Update(ctx, boundSchema); err != nil {
115-
return fmt.Errorf("failed to update BoundSchema %s with owner reference: %w", name, err)
116-
}
117-
118-
logger.V(6).Info("Set owner reference on BoundSchema",
119-
"boundSchema", name,
120-
"export", export.Name,
121-
"namespace", req.Namespace)
122-
}
123-
124-
return nil
125-
}
126-
12789
// getExportedSchemas will list all schemas, exported by current backend.
12890
// Important: getExportedSchemas is using client.Client to list resources, not cache.
12991
// This is due to fact we use dynamic client and unstructured.Unstructured to get schemas and it
@@ -174,13 +136,12 @@ func (r *reconciler) getExportedSchemas(ctx context.Context, cl client.Client) (
174136
return boundSchemas, nil
175137
}
176138

177-
func (r *reconciler) ensureBoundSchemas(ctx context.Context, cl client.Client, _ cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error {
139+
func (r *reconciler) ensureBoundSchemas(ctx context.Context, cl client.Client, export *kubebindv1alpha2.APIServiceExport, req *kubebindv1alpha2.APIServiceExportRequest) error {
178140
exportedSchemas, err := r.getExportedSchemas(ctx, cl)
179141
if err != nil {
180142
return err
181143
}
182144

183-
// Ensure all bound schemas exist
184145
for _, res := range req.Spec.Resources {
185146
if len(res.Versions) == 0 {
186147
continue
@@ -193,35 +154,49 @@ func (r *reconciler) ensureBoundSchemas(ctx context.Context, cl client.Client, _
193154
boundSchema.Spec.InformerScope = r.informerScope
194155
boundSchema.ResourceVersion = ""
195156

196-
obj, err := r.getBoundSchema(ctx, cl, boundSchema.Namespace, boundSchema.Name)
197-
if err != nil && !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "no matches for kind") {
157+
if err := r.createOrUpdateBoundSchema(ctx, cl, export, boundSchema); err != nil {
198158
return err
199159
}
160+
}
161+
}
162+
}
200163

201-
// TODO(mjudeikis): https://github.com/kube-bind/kube-bind/issues/297
202-
if obj != nil {
203-
continue
204-
}
164+
return nil
165+
}
205166

206-
// If namespaced isolation is configured for cluster-scoped objects,
207-
// we need to rewrite the BoundSchema's scope accordingly. For all
208-
// other isolation strategies, as well as for namespaced schemas,
209-
// no changes are necessary.
210-
if boundSchema.Spec.Scope == apiextensionsv1.NamespaceScoped && r.isolation == kubebindv1alpha2.IsolationNamespaced {
211-
boundSchema.Spec.Scope = apiextensionsv1.ClusterScoped
212-
}
167+
func (r *reconciler) createOrUpdateBoundSchema(ctx context.Context, cl client.Client, export *kubebindv1alpha2.APIServiceExport, desired *kubebindv1alpha2.BoundSchema) error {
168+
logger := klog.FromContext(ctx)
213169

214-
if err := r.createBoundSchema(ctx, cl, boundSchema); err != nil {
215-
return err
216-
}
217-
}
170+
existing, err := r.getBoundSchema(ctx, cl, desired.Namespace, desired.Name)
171+
if err != nil && !apierrors.IsNotFound(err) && !strings.Contains(err.Error(), "no matches for kind") {
172+
return err
173+
}
174+
175+
if existing != nil {
176+
if export == nil {
177+
return nil
178+
}
179+
if err := controllerutil.SetControllerReference(export, existing, cl.Scheme()); err != nil {
180+
return fmt.Errorf("failed to set owner reference on BoundSchema %s: %w", desired.Name, err)
181+
}
182+
if err := r.updateBoundSchema(ctx, cl, existing); err != nil {
183+
return fmt.Errorf("failed to update BoundSchema %s with owner reference: %w", desired.Name, err)
218184
}
185+
logger.V(6).Info("Updated owner reference on existing BoundSchema",
186+
"boundSchema", desired.Name,
187+
"export", export.Name,
188+
"namespace", desired.Namespace)
189+
return nil
219190
}
220191

221-
return nil
192+
if desired.Spec.Scope == apiextensionsv1.NamespaceScoped && r.isolation == kubebindv1alpha2.IsolationNamespaced {
193+
desired.Spec.Scope = apiextensionsv1.ClusterScoped
194+
}
195+
196+
return r.createBoundSchema(ctx, cl, desired)
222197
}
223198

224-
func (r *reconciler) ensureExports(ctx context.Context, cl client.Client, cache cache.Cache, req *kubebindv1alpha2.APIServiceExportRequest) error {
199+
func (r *reconciler) ensureExports(ctx context.Context, cl client.Client, existingExport *kubebindv1alpha2.APIServiceExport, req *kubebindv1alpha2.APIServiceExportRequest) error {
225200
logger := klog.FromContext(ctx)
226201

227202
var schemas []*kubebindv1alpha2.BoundSchema
@@ -249,12 +224,7 @@ func (r *reconciler) ensureExports(ctx context.Context, cl client.Client, cache
249224
schemas = append(schemas, boundSchema)
250225
}
251226

252-
if _, err := r.getServiceExport(ctx, cache, req.Namespace, req.Name); err != nil {
253-
if !apierrors.IsNotFound(err) {
254-
return err
255-
}
256-
} else {
257-
// already exists; nothing to do
227+
if existingExport != nil {
258228
conditions.MarkTrue(req, kubebindv1alpha2.APIServiceExportRequestConditionExportsReady)
259229
return nil
260230
}

0 commit comments

Comments
 (0)