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

feat: calculate the time we need to wait for the first applied but not ready binding to be ready. #888

Merged
merged 4 commits into from
Aug 22, 2024

Conversation

britaniar
Copy link
Contributor

@britaniar britaniar commented Jul 18, 2024

Description of your changes

Fixes #

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

@britaniar britaniar force-pushed the waitTime branch 2 times, most recently from 3cb80bd to 4b8bdb0 Compare July 18, 2024 22:53
@britaniar britaniar marked this pull request as ready for review July 26, 2024 19:00
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller_test.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller_test.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller_test.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@ryanzhang-oss ryanzhang-oss left a comment

Choose a reason for hiding this comment

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

we should piggy back the min wait time in the pickBindingsToRoll function as it goes through all bindings already

@britaniar britaniar force-pushed the waitTime branch 2 times, most recently from 2e91e34 to 176ae52 Compare July 30, 2024 22:59
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
@britaniar britaniar force-pushed the waitTime branch 2 times, most recently from b9ce336 to b8e31db Compare August 8, 2024 18:21
Copy link
Contributor

@ryanzhang-oss ryanzhang-oss left a comment

Choose a reason for hiding this comment

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

it would good to have an integration test to test this

pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Show resolved Hide resolved
@britaniar britaniar force-pushed the waitTime branch 2 times, most recently from b1f5e3c to dade247 Compare August 22, 2024 01:26
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
pkg/controllers/rollout/controller.go Outdated Show resolved Hide resolved
@ryanzhang-oss ryanzhang-oss merged commit 1f6234a into Azure:main Aug 22, 2024
11 checks passed
@britaniar britaniar deleted the waitTime branch August 22, 2024 22:59
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