knative / knative/serving

Revision controller needs some cleanup/organization

Open
#10,217 3 comments 0 reactions 1 assignee Claimed by @dprotaso View on GitHub
area/API kind/cleanup triage/accepted
Dominant language
Go
Stars
6.1k
Forks
1.2k
Avg merge
2d 7h
Merged PRs (30d)
2

Description

/area API
/kind cleanup

_Just revisited the revision controller after sometime. I figured I should write down my thoughts before I'm off on vacation starting now._

The revision controller is very fragile wrt. ordering of reconciling its child resources. Currently deployments are reconciled prior to pod autoscalers. Thus I've noticed:
1. Phases reading into the revision status may be reading stale data
- The deployment phase looks at the Active condition, then after the PodAutoscaler phase may update it
- `MakePA` looks at the revisions conditions to reconcile. Afterwards PA conditions are propagated to revision. Maybe this causes some churn?
1. Last `Mark*()` method wins - there's code in the PA phase to guard against overriding false conditions that it assumes the deployment phase has set
1. Re-ordering phases exposes assumptions:
- deployment phase (when deployment is not found) marks the revision as 'Deploying' - but the PA phase doesn't.
- test fixtures aren't accurate - ie. deployment was not ready when testing the steady state but still passed because of PA's ready state trumped all (see 2. above)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.