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

Jump to bottom

ReconcileRequeue for cluster resources was implemented #612

Merged
merged 1 commit into from
Nov 16, 2023
Merged

ReconcileRequeue for cluster resources was implemented #612

merged 1 commit into from
Nov 16, 2023

Conversation

DoodgeMatvey
Copy link
Contributor

Closes #606

@DoodgeMatvey DoodgeMatvey added the enhancement New feature or request label Nov 7, 2023
@DoodgeMatvey DoodgeMatvey self-assigned this Nov 7, 2023
@DoodgeMatvey DoodgeMatvey changed the title issue-606-resources-reconcile-requeue, ReconcileRequeue for cluster resources was implemented ReconcileRequeue for cluster resources was implemented Nov 7, 2023
tengu-alt
tengu-alt reviewed Nov 7, 2023
View reviewed changes
controllers/clusterresources/awsencryptionkey_controller.go Outdated
"github.com/instaclustr/operator/pkg/scheduler"
"sigs.k8s.io/controller-runtime/pkg/controller"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort imports, please and in all such cases

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed

OleksiienkoMykyta
OleksiienkoMykyta approved these changes Nov 7, 2023
View reviewed changes
controllers/clusterresources/awsendpointserviceprincipal_controller.go
@@ -34,7 +33,9 @@ import (
clusterresourcesv1beta1 "github.com/instaclustr/operator/apis/clusterresources/v1beta1"
"github.com/instaclustr/operator/pkg/instaclustr"
"github.com/instaclustr/operator/pkg/models"
"github.com/instaclustr/operator/pkg/ratelimiter"
"github.com/instaclustr/operator/pkg/scheduler"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have unsorted imports in the PR, please sort one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@DoodgeMatvey DoodgeMatvey force-pushed the issue-606-resources-reconcile-requeue branch from 3b336c1 to 17c459e Compare November 7, 2023 12:01
ribaraka
ribaraka requested changes Nov 15, 2023
View reviewed changes
controllers/clusterresources/nodereload_controller.go Outdated
@@ -181,7 +180,7 @@ func (r *NodeReloadReconciler) Reconcile(ctx context.Context, req ctrl.Request)
"status", nrs.Status,
)

return models.ReconcileRequeue, nil
return ctrl.Result{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part needs to be requeued.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

controllers/clusterresources/postgresqluser_controller.go Outdated
@@ -80,12 +82,12 @@ func (r *PostgreSQLUserReconciler) Reconcile(ctx context.Context, req ctrl.Reque
l.Info("PostgreSQL user secret is not found", "request", req)
r.EventRecorder.Event(u, models.Warning, models.NotFound,
"Secret is not found, please create a new secret or set an actual reference")
return models.ReconcileRequeue, nil
return ctrl.Result{}, nil
Copy link
Contributor

@ribaraka ribaraka Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it needs to be requeued as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

ribaraka
ribaraka reviewed Nov 15, 2023
View reviewed changes
controllers/clusterresources/redisuser_controller.go
Comment on lines -270 to +276
return models.ReconcileRequeue, nil
return ctrl.Result{}, err
}
}

return models.ExitReconcile, nil
return ctrl.Result{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CustomRateLimiter is not added to the Redis User controller

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@DoodgeMatvey DoodgeMatvey force-pushed the issue-606-resources-reconcile-requeue branch 2 times, most recently from 49e9f3d to e159ff1 Compare November 16, 2023 14:58
@DoodgeMatvey DoodgeMatvey force-pushed the issue-606-resources-reconcile-requeue branch from e159ff1 to 4f5f80a Compare November 16, 2023 16:45
@ribaraka ribaraka merged commit 22a74e3 into main Nov 16, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worryg0d worryg0d worryg0d approved these changes

@OleksiienkoMykyta OleksiienkoMykyta OleksiienkoMykyta approved these changes

@tengu-alt tengu-alt tengu-alt approved these changes

@ribaraka ribaraka ribaraka approved these changes

@RostislavPorohnya RostislavPorohnya Awaiting requested review from RostislavPorohnya

@testisnullus testisnullus Awaiting requested review from testisnullus

Assignees

@DoodgeMatvey DoodgeMatvey

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement ReconcileRequeue for cluster resources
5 participants
@DoodgeMatvey @ribaraka @tengu-alt @OleksiienkoMykyta @worryg0d