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

Add routing label to sandbox pods #1045

Merged
merged 3 commits into from
Jan 30, 2025
Merged

Add routing label to sandbox pods #1045

merged 3 commits into from
Jan 30, 2025

Conversation

roypaulin
Copy link
Collaborator

When a sandbox pod is deleted, the operator does not put back the routing label. This adds the ClientRoutingLabelReconciler in the sandbox controller. It also adds an annotation in the configMap to sometimes disable routing, like in the case of online upgrade where routing to the sandbox must not be allowed until replication is done.

Copy link
Collaborator

@cchen-vertica cchen-vertica left a comment

Choose a reason for hiding this comment

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

Looks good. Only one concern: if we enable client-proxy and restart a client-proxy pod of a sandbox, the client-proxy pod should still have the routing label. Your code seems covered this case, but we don't have an e2e test for it. You can leave a comment in your jira to let QA test this case.

@@ -1052,6 +1052,13 @@ func (r *OnlineUpgradeReconciler) redirectConnectionsToReplicaGroupB(ctx context
if verrors.IsReconcileAborted(res, err) {
return res, err
}
// Now that routing to the sandbox is allowed, we no longer need
// disableRouting annotation so we can safely turn it off.
sbMan := MakeSandboxConfigMapManager(r.VRec, r.VDB, r.sandboxName, "" /*no uuid*/)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to put space in the comment: change "/no uuid/" to "/* no uuid */". Otherwise, the new golint in new operator-sdk will fail the build.

@roypaulin roypaulin merged commit 019f778 into main Jan 30, 2025
41 checks passed
@roypaulin roypaulin deleted the roypaulin/routing branch January 30, 2025 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants