WARNING: THIS SITE IS A MIRROR OF GITHUB.COM / IT CANNOT LOGIN OR REGISTER ACCOUNTS / THE CONTENTS ARE PROVIDED AS-IS / THIS SITE ASSUMES NO RESPONSIBILITY FOR ANY DISPLAYED CONTENT OR LINKS / IF YOU FOUND SOMETHING MAY NOT GOOD FOR EVERYONE, CONTACT ADMIN AT ilovescratch@foxmail.com
Skip to content

Commit 996654b

Browse files
tkan145tkan145
authored
Merge pull request #1116 from tkan145/improve-application-cr
THREESCALE-11948 Fix nil pointer in ApplicationCR
2 parents 9aaa8eb + 9a516f4 commit 996654b

12 files changed

+881
-610
lines changed

‎controllers/capabilities/application_controller.go‎

Lines changed: 57 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,11 @@ const (
5454
// +kubebuilder:rbac:groups=capabilities.3scale.net,resources=applications/status,verbs=get;update;patch
5555

5656
func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
57-
_ = context.Background()
5857
reqLogger := r.Logger().WithValues("application", req.NamespacedName)
5958
reqLogger.Info("Reconcile Application", "Operator version", version.Version)
6059

6160
application := &capabilitiesv1beta1.Application{}
62-
err := r.Client().Get(context.TODO(), req.NamespacedName, application)
61+
err := r.Client().Get(ctx, req.NamespacedName, application)
6362
if err != nil {
6463
if apierrors.IsNotFound(err) {
6564
// Request object not found, could have been deleted after reconcile request.
@@ -79,6 +78,26 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
7978
}
8079
reqLogger.V(1).Info(string(jsonData))
8180
}
81+
82+
// Ignore deleted Applications, this can happen when foregroundDeletion is enabled
83+
// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion
84+
if application.GetDeletionTimestamp() != nil {
85+
if controllerutil.ContainsFinalizer(application, applicationFinalizer) {
86+
err = r.removeApplicationFrom3scale(application)
87+
if err != nil {
88+
r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to delete application", "%v", err)
89+
return ctrl.Result{}, err
90+
}
91+
92+
if controllerutil.RemoveFinalizer(application, applicationFinalizer) {
93+
if err = r.UpdateResource(application); err != nil {
94+
return ctrl.Result{}, err
95+
}
96+
}
97+
}
98+
return ctrl.Result{}, nil
99+
}
100+
82101
// get Account
83102
accountResource := &capabilitiesv1beta1.DeveloperAccount{}
84103
projectMeta := types.NamespacedName{
@@ -89,83 +108,51 @@ func (r *ApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request)
89108
err = r.Client().Get(r.Context(), projectMeta, accountResource)
90109
if err != nil {
91110
if apierrors.IsNotFound(err) {
92-
// If the application CR is marked for deletion and the dev account associated doesn't exist, remove the application CR since there is nothing else to do with application.
93-
if application.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(application, applicationFinalizer) {
94-
controllerutil.RemoveFinalizer(application, applicationFinalizer)
95-
err = r.UpdateResource(application)
96-
if err != nil {
97-
return ctrl.Result{}, err
98-
}
99-
100-
return ctrl.Result{}, nil
101-
}
102111
reqLogger.Error(err, "error developer account not found ")
103112
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, application, nil, "", err)
104113
statusResult, statusUpdateErr := statusReconciler.Reconcile()
105114
if statusUpdateErr != nil {
106-
if err != nil {
107-
return ctrl.Result{}, fmt.Errorf("failed to reconcile application: %v. Failed to update application status: %w", err, statusUpdateErr)
108-
}
109-
110-
return ctrl.Result{}, fmt.Errorf("failed to update application status: %w", statusUpdateErr)
115+
return ctrl.Result{}, fmt.Errorf("failed to reconcile application: %v. Failed to update application status: %w", err, statusUpdateErr)
111116
}
112117
if statusResult.Requeue {
113118
return statusResult, nil
114119
}
115120
}
116-
}
117-
118-
// get providerAccountRef from account
119-
providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), accountResource.Namespace, accountResource.Spec.ProviderAccountRef, r.Logger())
120-
if err != nil {
121121
return ctrl.Result{}, err
122122
}
123123

124-
insecureSkipVerify := controllerhelper.GetInsecureSkipVerifyAnnotation(application.GetAnnotations())
125-
threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount, insecureSkipVerify)
124+
metadataUpdated, err := r.reconcileMetadata(application, accountResource)
126125
if err != nil {
126+
r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to update ownerReferences in application", "%v", err)
127127
return ctrl.Result{}, err
128128
}
129-
130-
// Ignore deleted Applications, this can happen when foregroundDeletion is enabled
131-
// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion
132-
if application.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(application, applicationFinalizer) {
133-
err = r.removeApplicationFrom3scale(application, req, *threescaleAPIClient)
134-
if err != nil {
135-
r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to delete application", "%v", err)
136-
return ctrl.Result{}, err
137-
}
138-
139-
controllerutil.RemoveFinalizer(application, applicationFinalizer)
129+
if metadataUpdated {
140130
err = r.UpdateResource(application)
141131
if err != nil {
142132
return ctrl.Result{}, err
143133
}
144134

135+
// No need requeue because the reconcile will trigger automatically since updating the Application CR
145136
return ctrl.Result{}, nil
146137
}
147138

148-
if application.GetDeletionTimestamp() != nil {
149-
return ctrl.Result{}, nil
139+
// get providerAccountRef from account
140+
providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), accountResource.Namespace, accountResource.Spec.ProviderAccountRef, r.Logger())
141+
if err != nil {
142+
return ctrl.Result{}, err
150143
}
151144

152-
metadataUpdated, err := r.reconcileMetadata(application, accountResource)
145+
insecureSkipVerify := controllerhelper.GetInsecureSkipVerifyAnnotation(application.GetAnnotations())
146+
threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount, insecureSkipVerify)
153147
if err != nil {
154-
r.EventRecorder().Eventf(application, corev1.EventTypeWarning, "Failed to update ownerReferences in application", "%v", err)
155148
return ctrl.Result{}, err
156149
}
157-
if metadataUpdated {
158-
err = r.UpdateResource(application)
159-
if err != nil {
160-
return ctrl.Result{}, err
161-
}
162150

163-
// No need requeue because the reconcile will trigger automatically since updating the Application CR
164-
return ctrl.Result{}, nil
165-
}
151+
applicationEntity, reconcileErr := r.applicationReconciler(application, accountResource, threescaleAPIClient)
166152

167-
statusReconciler, reconcileErr := r.applicationReconciler(application, req, threescaleAPIClient, providerAccount.AdminURLStr, accountResource)
153+
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, application, applicationEntity, providerAccount.AdminURLStr, reconcileErr)
168154
statusResult, statusUpdateErr := statusReconciler.Reconcile()
155+
169156
if statusUpdateErr != nil {
170157
if reconcileErr != nil {
171158
return ctrl.Result{}, fmt.Errorf("failed to sync application: %v. Failed to update application status: %w", reconcileErr, statusUpdateErr)
@@ -236,53 +223,47 @@ func (r *ApplicationReconciler) reconcileMetadata(application *capabilitiesv1bet
236223
return changed, nil
237224
}
238225

239-
func (r *ApplicationReconciler) applicationReconciler(applicationResource *capabilitiesv1beta1.Application, req ctrl.Request, threescaleAPIClient *threescaleapi.ThreeScaleClient, providerAccountAdminURLStr string, accountResource *capabilitiesv1beta1.DeveloperAccount) (*ApplicationStatusReconciler, error) {
226+
func (r *ApplicationReconciler) applicationReconciler(applicationResource *capabilitiesv1beta1.Application, accountResource *capabilitiesv1beta1.DeveloperAccount, threescaleAPIClient *threescaleapi.ThreeScaleClient) (*controllerhelper.ApplicationEntity, error) {
240227
// get product
241228
productResource := &capabilitiesv1beta1.Product{}
242229
projectMeta := types.NamespacedName{
243230
Name: applicationResource.Spec.ProductCR.Name,
244-
Namespace: req.Namespace,
231+
Namespace: applicationResource.Namespace,
245232
}
246233

247234
err := r.Client().Get(r.Context(), projectMeta, productResource)
248235
if err != nil {
249236
if apierrors.IsNotFound(err) {
250-
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, applicationResource, nil, "", err)
251-
return statusReconciler, err
237+
return nil, err
252238
}
253239
}
254240

255241
err = r.checkExternalResources(applicationResource, accountResource, productResource)
256242
if err != nil {
257-
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, applicationResource, nil, "", err)
258-
return statusReconciler, err
243+
return nil, err
259244
}
260245

261-
reconciler := NewApplicationReconciler(r.BaseReconciler, applicationResource, accountResource, productResource, threescaleAPIClient)
262-
ApplicationEntity, err := reconciler.Reconcile()
263-
if err != nil {
264-
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, applicationResource, nil, providerAccountAdminURLStr, err)
265-
return statusReconciler, err
266-
}
267-
statusReconciler := NewApplicationStatusReconciler(r.BaseReconciler, applicationResource, ApplicationEntity, providerAccountAdminURLStr, err)
268-
return statusReconciler, err
246+
reconciler := NewApplicationReconciler(r.BaseReconciler, applicationResource, *accountResource.Status.ID, *productResource.Status.ID, threescaleAPIClient)
247+
return reconciler.Reconcile()
269248
}
270249

271-
func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabilitiesv1beta1.Application, req ctrl.Request, threescaleAPIClient threescaleapi.ThreeScaleClient) error {
250+
func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabilitiesv1beta1.Application) error {
272251
logger := r.Logger().WithValues("application", client.ObjectKey{Name: application.Name, Namespace: application.Namespace})
273252

274253
// get Account
275254
account := &capabilitiesv1beta1.DeveloperAccount{}
276255
projectMeta := types.NamespacedName{
277256
Name: application.Spec.AccountCR.Name,
278-
Namespace: req.Namespace,
257+
Namespace: application.Namespace,
279258
}
280259

281260
err := r.Client().Get(r.Context(), projectMeta, account)
282261
if err != nil {
283262
if apierrors.IsNotFound(err) {
284263
logger.Info("Account resource not found. Ignoring since object must have been deleted")
264+
return nil
285265
}
266+
return err
286267
}
287268

288269
// Attempt to remove application only if application.Status.ID is present
@@ -296,6 +277,18 @@ func (r *ApplicationReconciler) removeApplicationFrom3scale(application *capabil
296277
return fmt.Errorf("could not remove application because ID is missing in the satus of developer account %s", account.Name)
297278
}
298279

280+
// get providerAccountRef from account
281+
providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), account.Namespace, account.Spec.ProviderAccountRef, r.Logger())
282+
if err != nil {
283+
return err
284+
}
285+
286+
insecureSkipVerify := controllerhelper.GetInsecureSkipVerifyAnnotation(application.GetAnnotations())
287+
threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount, insecureSkipVerify)
288+
if err != nil {
289+
return err
290+
}
291+
299292
err = threescaleAPIClient.DeleteApplication(*account.Status.ID, *application.Status.ID)
300293
if err != nil && !threescaleapi.IsNotFound(err) {
301294
return err

0 commit comments

Comments
 (0)