Optimize DryRun

[vagababov]

While investigating https://github.com/knative/pkg/issues/1509 we stumbled upon problem that DryRun validation of podspecs is not really working at scale, since we're overwhelming the RateLimiter in the k8s API client that is used to invoke dry run.

Right now we moved it again to be disabled by default, but it is possible we can investigate the avenues to avoid unnecessary dry runs. E.g. avoid them when pod-spec hasn't changed in updates or completely ignore when the only update is status update.

In what area(s)?

/area API

/assign @whaught

[whaught]

https://github.com/knative/serving/blob/2644ad1fbed6eb753080b4789595182cfdb23563/pkg/webhook/validate_unstructured.go#L104

I believe we ignore no-op updates. That said there may still be common-cases where we can avoid dry-run on Create.

[github-actions[bot]]

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

[vagababov]

/lifecycle frozen

we can't really enable this right now

[evankanderson]

/triage needs-user-input

Is this still important?

[evankanderson]

@dprotaso

Is this still important (/remove-triage needs-user-input, /triage accepted, /help) or should we close it?

[dprotaso]

/triage accepted /good-first-issue

Moving to Icebox and labeling this as a good first issue.

My only opinion here is to have PodSpec dry run occur when the user has request dry-runs for our CRD types - Service and Configuration. For example using kubectl create or kubectl apply there is a --dry-run=server flag. Users shouldn't need the annotation on the Knative Service.

note: I haven't tested whether the dry-run flag works for CRs. If not an alternative could be to add support for dry-run in kn

To implement this we know from the AdmissionRequest whether it's a DryRun or not

[knative-prow[bot]]

@dprotaso: This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-good-first-issue command.

In response to this:

/triage accepted /good-first-issue

Moving to Icebox and labeling this as a good first issue.

My only opinion here is to have PodSpec dry run occur when the user has request dry-runs for our CRD types - Service and Configuration. For example using kubectl create or kubectl apply there is a --dry-run=server flag. Users shouldn't need the annotation on the Knative Service.

note: I haven't tested whether the dry-run flag works for CRs. If not an alternative could be to add support for dry-run in kn

To implement this we know from the AdmissionRequest whether it's a DryRun or not

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[ashutosh887]

I would Like to work on this @dprotaso @ReToCode @vagababov Please assign

[dprotaso]

/assign @ashutosh887

[dprotaso]

@ashutosh887 are you still working on this?

[dprotaso]

/unassign @ashutosh887

[xiangpingjiang]

@dprotaso

/triage accepted /good-first-issue

Moving to Icebox and labeling this as a good first issue.

My only opinion here is to have PodSpec dry run occur when the user has request dry-runs for our CRD types - Service and Configuration. For example using kubectl create or kubectl apply there is a --dry-run=server flag. Users shouldn't need the annotation on the Knative Service.

note: I haven't tested whether the dry-run flag works for CRs. If not an alternative could be to add support for dry-run in kn

To implement this we know from the AdmissionRequest whether it's a DryRun or not

When run kubectl apply -f deployment.yaml --dry-run=server -oyaml , It will return the deployment yaml, do we also need implement this for Service and Configuration?

[Priyansurout]

/assign

[dprotaso]

@Priyansurout are you still working on this?

[Priyansurout]

@dprotaso no sir