8000 enable tortoise to move to Working from BackToNormal when there is no HPA by AVSBharadwaj · Pull Request #443 · mercari/tortoise · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

enable tortoise to move to Working from BackToNormal when there is no HPA #443

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.

8000 Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 20, 2025

Conversation

AVSBharadwaj
Copy link
Collaborator
@AVSBharadwaj AVSBharadwaj commented May 16, 2025

Issue: #444

What:

  • adds logic for Tortoise to move from BackToNormal to Working so that new recommendations can be applied
  • also adds test cases to test this change and well as existing phase changes

Why:

  • we observed in all vertical//VPA cases, tortoise when in BackToNormal phase i.e after switching from Emergency Mode would continue to stay in BacktoNormal due to the fact that the existing logic at line would execute only when there is a corresponding HPA which is evident from this line.
  • so for cases when it is all VPA, we handle the case to switch the above phase outside of the HPA service in the tortoise service itself.

@@ -135,6 +135,13 @@ func (s *Service) UpdateTortoisePhase(tortoise *v1beta3.Tortoise, now time.Time)
s.recorder.Event(tortoise, corev1.EventTypeNormal, event.Working, "Emergency mode is turned off. Tortoise starts to work on autoscaling normally. HPA.Spec.MinReplica will gradually be reduced")
tortoise.Status.TortoisePhase = v1beta3.TortoisePhaseBackToNormal
}
case v1beta3.TortoisePhaseBackToNormal:
if !hasHorizontal(tortoise) {
// If there's no HPA, we can transition directly to Working
Copy link

Choose a reason for hiding this comment

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

nit: comment not needed, we have the info in the event below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the 2 comments cover 2 cases of

  1. if HPA not present
  2. if HPA present

Copy link
Collaborator
@AdityaK011 AdityaK011 left a comment

Choose a reason for hiding this comment

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

LGTM

@AVSBharadwaj AVSBharadwaj merged commit 3e6f019 into main May 20, 2025
3 checks passed
@AVSBharadwaj AVSBharadwaj deleted the enable-backToNormal-to-working-when-no-HPA branch May 20, 2025 06:37
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.

3 participants
0