Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Create GitHub client only if it is necessary #92

Merged
merged 3 commits into from
Feb 16, 2024
Merged

🌱 Create GitHub client only if it is necessary #92

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
17d2f86
Create GitHub client only if it is necessary
chess-knight Feb 16, 2024
abeed44
Defer unlocking of mutex
chess-knight Feb 16, 2024
162b12c
Create a GitHub client before mutex
chess-knight Feb 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions internal/controller/openstackclusterstackrelease_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,21 +104,6 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context,
}
}()

gc, err := r.GitHubClientFactory.NewClient(ctx)
if err != nil {
conditions.MarkFalse(openstackclusterstackrelease,
apiv1alpha1.GitAPIAvailableCondition,
apiv1alpha1.GitTokenOrEnvVariableNotSetReason,
clusterv1beta1.ConditionSeverityError,
err.Error(),
)
record.Warnf(openstackclusterstackrelease, "GitTokenOrEnvVariableNotSet", err.Error())
logger.Error(err, "failed to create Github client")
return ctrl.Result{}, nil
}

conditions.MarkTrue(openstackclusterstackrelease, apiv1alpha1.GitAPIAvailableCondition)

// name of OpenStackClusterStackRelease object is same as the release tag
releaseTag := openstackclusterstackrelease.Name

Expand All @@ -142,13 +127,28 @@ func (r *OpenStackClusterStackReleaseReconciler) Reconcile(ctx context.Context,
// acquire lock so that only one reconcile loop can download the release
r.openStackClusterStackRelDownloadDirectoryMutex.Lock()

defer r.openStackClusterStackRelDownloadDirectoryMutex.Unlock()

gc, err := r.GitHubClientFactory.NewClient(ctx)
if err != nil {
conditions.MarkFalse(openstackclusterstackrelease,
apiv1alpha1.GitAPIAvailableCondition,
apiv1alpha1.GitTokenOrEnvVariableNotSetReason,
clusterv1beta1.ConditionSeverityError,
err.Error(),
)
record.Warnf(openstackclusterstackrelease, "GitTokenOrEnvVariableNotSet", err.Error())
logger.Error(err, "failed to create Github client")
return ctrl.Result{}, nil
}

conditions.MarkTrue(openstackclusterstackrelease, apiv1alpha1.GitAPIAvailableCondition)
matofeder marked this conversation as resolved.
Show resolved Hide resolved

if err := downloadReleaseAssets(ctx, releaseTag, releaseAssets.LocalDownloadPath, gc); err != nil {
logger.Error(err, "failed to download release assets")
return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil
}

r.openStackClusterStackRelDownloadDirectoryMutex.Unlock()

record.Eventf(openstackclusterstackrelease, "ClusterStackReleaseAssetsReady", "successfully downloaded ClusterStackReleaseAssets %q", releaseTag)
// requeue to make sure release assets can be accessed
return ctrl.Result{Requeue: true}, nil
Expand Down
Loading