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.

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

NonAdminBackupReconciler structure should not have references to context etc... #52

Closed
mpryc opened this issue Apr 25, 2024 · 3 comments
Assignees

Comments

@mpryc
Copy link
Collaborator

mpryc commented Apr 25, 2024

Current implementation uses references to the Context and other parts. This should be removed, because in case we have more then one worker that reconciles we will have race condition on setting those references:

// NonAdminBackupReconciler reconciles a NonAdminBackup object
type NonAdminBackupReconciler struct {
	client.Client
	Scheme         *runtime.Scheme
	Log            logr.Logger
	Context        context.Context
	NamespacedName types.NamespacedName
}

This is change that will require quite a bit of refactoring code, because currently other functions are using the embedded to that structure references.

@mpryc mpryc moved this to Ready for Review in OADP Apr 30, 2024
@mpryc mpryc self-assigned this Apr 30, 2024
@mateusoliveira43
Copy link
Contributor

@mpryc this is in ready for review in our board. What is the related work?

@mpryc
Copy link
Collaborator Author

mpryc commented May 14, 2024

I think we can close this as it was resolved with:

#54

@mpryc mpryc closed this as completed May 14, 2024
@mpryc
Copy link
Collaborator Author

mpryc commented May 14, 2024

One note. We have and will continue the need to have the reference to context in the structure, however as in the #54 we removed re-assignment of this context within Reconcile loop:

r.Context = ctx

@mateusoliveira43 mateusoliveira43 moved this from Ready for Review to Merged / Ready for build in OADP May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged / Ready for build
Development

No branches or pull requests

2 participants