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

Expose EPP via sidecar proxy #2680

Merged
merged 1 commit into from
Feb 19, 2025
Merged

Expose EPP via sidecar proxy #2680

merged 1 commit into from
Feb 19, 2025

Conversation

jianglai
Copy link
Collaborator

@jianglai jianglai commented Feb 18, 2025

Resurrect the EPP load balancer which connects to the sidecar proxy within the same pod as nomulus. This is so that we have the freedom to choose between the standalone proxy and the sidecar one in case one doesn't work as expected.

Also switch to "Performance" nodes with does not have CPU or memory limits. I have observed that with the default nodes, after the service ran for a day or so the pods start to crash due to lack of memory from the nodes, which exacerbates the problem with sessions stickness.

Lastly, for the standalone proxies to work properly after a release of Nomulus, a GCB job is added to force restart the proxies (and therefore forcing the clients to reconnect and re-login, to re-establish sessions).


This change is Reviewable

@jianglai jianglai changed the title epp Expose EPP via saidcar proxy Feb 18, 2025
Copy link
Collaborator Author

@jianglai jianglai left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jianglai)

@jianglai jianglai requested a review from gbrodman February 18, 2025 19:16
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jianglai)


a discussion (no related file):
semi nit: typo in PR title

@jianglai jianglai changed the title Expose EPP via saidcar proxy Expose EPP via sidecar proxy Feb 18, 2025
@jianglai jianglai requested a review from gbrodman February 18, 2025 20:28
Copy link
Collaborator Author

@jianglai jianglai left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gbrodman)


a discussion (no related file):

Previously, gbrodman wrote…

semi nit: typo in PR title

oops. fixed.

Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jianglai)

@jianglai jianglai force-pushed the epp branch 4 times, most recently from 1bc4142 to 342ac66 Compare February 18, 2025 23:28
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 9 files at r2, all commit messages.
Reviewable status: 5 of 10 files reviewed, 1 unresolved discussion (waiting on @jianglai)


release/cloudbuild-restart-proxies.yaml line 1 at r2 (raw file):

# This will do rolling restarts of all proxies. This forces the client to reconnect

why do we need this?

@jianglai jianglai requested a review from gbrodman February 19, 2025 00:47
Copy link
Collaborator Author

@jianglai jianglai left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 9 files at r2.
Reviewable status: 8 of 10 files reviewed, 1 unresolved discussion (waiting on @gbrodman)


release/cloudbuild-restart-proxies.yaml line 1 at r2 (raw file):

Previously, gbrodman wrote…

why do we need this?

Because otherwise the proxy would hold on to the EPP connections but the nomulus instances with the session information would be gone. The client would still think they are logged in but they cannot do anything. Forcing the proxies to restart will force the clients to reconnect and re-login.

This is not ideal, of course, but it is better than the alternative.

Copy link
Collaborator Author

@jianglai jianglai left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 9 files at r2.
Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @gbrodman)

@jianglai jianglai force-pushed the epp branch 2 times, most recently from ac14d82 to 39fe6ce Compare February 19, 2025 01:57
Copy link
Collaborator Author

@jianglai jianglai left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gbrodman)

@jianglai jianglai force-pushed the epp branch 4 times, most recently from 69e8c41 to 540d40c Compare February 19, 2025 15:46
Copy link
Collaborator Author

@jianglai jianglai left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 9 files at r2, 4 of 4 files at r5, 2 of 2 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gbrodman)

Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 9 files at r2, 1 of 1 files at r3, 1 of 3 files at r4, 4 of 4 files at r5, 1 of 2 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @jianglai)

@jianglai jianglai added this pull request to the merge queue Feb 19, 2025
Merged via the queue into google:master with commit 3f2a42a Feb 19, 2025
9 checks passed
@jianglai jianglai deleted the epp branch February 19, 2025 20:03
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