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

Traffic split should only be applied after a revision is ready once. #2735

Closed
markusthoemmes opened this issue Dec 17, 2018 · 3 comments
Closed
Assignees
Labels
area/API API objects and controllers area/networking kind/feature Well-understood/specified features, ready for coding.
Milestone

Comments

@markusthoemmes
Copy link
Contributor

Expected Behavior

When applying a traffic split to a service in release mode, I expect the traffic to be routed to the new revision only after the image has been built, a pod has been brought up and that pod is verified to work properly. In other words: When the Revision is considered Ready.

Actual Behavior

The traffic split is applied right away, causing the traffic to the new revision to go into a black hole until the revision finally comes up. This is especially notable, if that involves a build, where it can take comparably long to bring the revision up.

Steps to Reproduce the Problem

  1. Apply a service with a build (to exagarate the problem)
  2. Try to bring up a new revision (update the app) and apply a traffic split.
  3. Notice how traffic is stuck.

Additional Info

This doesn't need to apply to service rollouts only. I'm wondering if routes themselves should have this behavior, where traffic splits are only applied to Ready revisions. More precisely, we could maybe gate this behavior via the ContainerHealthy condition of the revision to decouple it from scale to 0 scenarios.

Having looked at it briefly, this could be a violation in terms of layering (Route controller vs. Revision controller). A more simple approach would be to only apply a split once the targeted service has endpoints available to overcome that layering violation.

@knative-prow-robot knative-prow-robot added area/API API objects and controllers area/networking kind/feature Well-understood/specified features, ready for coding. labels Dec 17, 2018
@dgerd
Copy link

dgerd commented Dec 20, 2018

This seems very similar to #2430 where one child resource has enacted the changes, but the other child resource has not. I think we should look at tackling both in Milestone 0.4. This one seems to be a bit more impacting than the other since traffic gets black holed rather than sent to the previous revision.

Thinking about this briefly I am in favor of the simple approach as the former approach would add significant logic and coupling in the Route controller that I am not entirely convinced that we want.

/milestone Serving 0.4

@knative-prow-robot knative-prow-robot added this to the Serving 0.4 milestone Dec 20, 2018
@tcnghia
Copy link
Contributor

tcnghia commented Jan 3, 2019

Is this only a problem with traffic split, or a problem with early routing in general?

@markusthoemmes
Copy link
Contributor Author

/assign

@tcnghia I'll add more info, I'll start working on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers area/networking kind/feature Well-understood/specified features, ready for coding.
Projects
None yet
Development

No branches or pull requests

4 participants