Skip to content

Conversation

@furkansahin
Copy link
Contributor

@furkansahin furkansahin commented Nov 20, 2024

Add id column to loadbalancers_vms

Prepare load_balancers_vms models for health probes

We are moving the health checks into load_balancers_vms. The main reason
these are not handled as part of VM monitoring is because the VM might
be up and running but the load balancer makes a decision based on the
application state. It is safer to keep this separate. This code will
also interract with load balancer and keeping a code path as part of a
VM that is not part of a load balancer doesn't seem clever. We are
making a couple of logic changes here.

  1. The state of the load balancer node is only updated when the monitor
    is sure their state is changed. This way, we don't have to look into
    counters at the time of update.
  2. IPv4 and IPv6 availability is handled properly. Before, these were
    kind of mixed up and causing some logical errors. Now, they are
    distinctly checked and properly declared.

This commit itself doesn't simply stop the health probe strands. We will
need to handle their migration manually by destroying the strands.

Switch using monitor and remove health probes prog

With this commit, we are removing the prog and completely switching to
monitor for health probes.

Inference endpoint doesn't use the counter anymore

We are removing the logic that checks the counter in inverence endpoint
availability check. The main reason is, now, if the state of the
load_balancers_vms is switched to "down" we are already sure that the
counter hit to the threshold.

Remove state_counter and update keys in load_balancers_vms

The primary key is changed to id, state_counter is not needed and id
cannot be null anymore.

@furkansahin
Copy link
Contributor Author

Instead of spending time on #2658, I wanted to move this.

@furkansahin furkansahin changed the title [WIP] Integrate Load Balancer Health Probes into monitoring Integrate Load Balancer Health Probes into monitoring Jan 27, 2025
@furkansahin furkansahin force-pushed the lb_hp_via_monitor branch 3 times, most recently from e34a2cf to b3f47ad Compare January 28, 2025 16:05
@furkansahin furkansahin marked this pull request as ready for review January 28, 2025 16:07
Copy link
Member

@byucesoy byucesoy 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 IMO apart from few minor comments.

@furkansahin furkansahin force-pushed the lb_hp_via_monitor branch 2 times, most recently from 583873e to 6f55d98 Compare January 29, 2025 11:31
We are moving the health checks into load_balancers_vms. The main reason
these are not handled as part of VM monitoring is because the VM might
be up and running but the load balancer makes a decision based on the
application state. It is safer to keep this separate. This code will
also interract with load balancer and keeping a code path as part of a
VM that is not part of a load balancer doesn't seem clever. We are
making a couple of logic changes here.
1. The state of the load balancer node is only updated when the monitor
is sure their state is changed. This way, we don't have to look into
counters at the time of update.
2. IPv4 and IPv6 availability is handled properly. Before, these were
kind of mixed up and causing some logical errors. Now, they are
distinctly checked and properly declared.

This commit itself doesn't simply stop the health probe strands. We will
need to handle their migration manually by destroying the strands.
With this commit, we are removing the prog and completely switching to
monitor for health probes.
We are removing the logic that checks the counter in inverence endpoint
availability check. The main reason is, now, if the state of the
load_balancers_vms is switched to "down" we are already sure that the
counter hit to the threshold.
The primary key is changed to id, state_counter is not needed and id
cannot be null anymore.
@furkansahin furkansahin merged commit 26d174a into main Jan 29, 2025
6 checks passed
@furkansahin furkansahin deleted the lb_hp_via_monitor branch January 29, 2025 15:34
@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants