add initialize conditions to MakePA to avoid potential race conditions

[nader-ziada]

add initialize conditions to MakePA to avoid potential race conditions
between revision and autoscaler

Fixes #16036

Proposed Changes

• Add initializeConditions in MakePA to avoid any potential race conditions

[codecov]

Codecov Report

❌ Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.06%. Comparing base (961ce6a) to head (efd0538).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/reconciler/testing/v1/factory.go	0.00%	18 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #16037      +/-   ##
==========================================
- Coverage   80.20%   80.06%   -0.14%     
==========================================
  Files         214      214              
  Lines       16892    16910      +18     
==========================================
- Hits        13548    13539       -9     
- Misses       2983     3011      +28     
+ Partials      361      360       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[nader-ziada]

/test istio-latest-no-mesh

[dprotaso]

@nader-ziada PTAL - I pushed a change where I was able to move the kubebuilder tag deeper into the status.

[nader-ziada]

@nader-ziada PTAL - I pushed a change where I was able to move the kubebuilder tag deeper into the status.

looks good, i kind of tried something similar to that, but got issues when deploying knative on the cluster, if this works, its cleaner

[dprotaso]

I think we want this change - though I'm wondering if we should just default only Ready=Unknown and not the subconditions.

I had a follow up question here: #16036 (comment)

[nader-ziada]

/test istio-latest-no-mesh

[nader-ziada]

I think we want this change - though I'm wondering if we should just default only Ready=Unknown and not the subconditions.

I had a follow up question here: #16036 (comment)

replied on issue, according to my understanding here: https://github.com/knative/serving/blob/main/pkg/reconciler/revision/reconcile_resources.go#L195

[nader-ziada]

/retest

[nader-ziada]

@dprotaso

serving/pkg/apis/serving/v1/revision_lifecycle.go

Line 187 in bb17bbb

if cond == nil {

I think this is still needed because in some failure cases, the pa is nil

[nader-ziada]

/retest

[dprotaso]

I think we should following Gateway API and on CREATE default the reason and message to

reason:"Pending"
message:"Waiting for controller"

'Deploying' makes it seem like we've taken action on it when we have not. The above makes it clear that it still needs to be reconciled.

[dprotaso]

Can we update this to?

I'm surprised we didn't need to change this

[nader-ziada]

i'll fix that, will probably need tests fixing as well

[nader-ziada]

done. PTAL

[dprotaso]

/lgtm
/approve

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, nader-ziada

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [dprotaso]
• pkg/apis/OWNERS [dprotaso]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dprotaso]

/retest

[nader-ziada]

follow up here: #16078