knative / serving-operator

DEPRECATED: Development continues in https://github.com/knative/operator/
Apache License 2.0
39 stars 45 forks source link

The creation of CRD should be made part of the operator installation #187

Closed houshengbo closed 5 years ago

houshengbo commented 5 years ago

We currently install CRD as prerequisite of the operator installation. This induced that the CRD yaml is not included in the operator release yaml. We should make sure the operator verifies the existence of the CRD and install the CRD automatically when we install the operator.

evankanderson commented 5 years ago

One problem we've run into historically with CRD installation is that installation of the CRD, which creates a new type in the apiserver, can take longer to reconcile than it takes for kubectl to proceed with creating the remaining items, including any custom objects of the newly defined type (which then fail).

What will the installation process look like after this bundling?

On Wed, Oct 9, 2019 at 12:15 PM Vincent notifications@github.com wrote:

We currently install CRD as prerequisite of the operator installation. This induced that the CRD yaml is not included in the operator release yaml. We should make sure the operator verifies the existence of the CRD and install the CRD automatically when we install the operator.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/knative/serving-operator/issues/187?email_source=notifications&email_token=AB4XEN3NIK57DX3HCEPILD3QNYUV7A5CNFSM4I7DUVJ2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HQXINHQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4XENY6S454HHZYBHRIF23QNYUV7ANCNFSM4I7DUVJQ .

-- Evan Anderson argent@google.com

houshengbo commented 5 years ago

@evankanderson I submitted a PR https://github.com/knative/serving-operator/pull/186. Before launching the controller, we verify the existence of the CRDs at the very beginning(only the existence in this PR. If we want to verify specific property, we can add later). If the CRDs cannot be available during a certain Timeout, operator quits.

jcrossley3 commented 5 years ago

From what I can tell, the operator already gracefully handles the race condition, so I think it's fine to include the CRD as a part of installation without adding the 30 required files for #186. I don't think the operator should ever quit on its own. It should keep doing what it already does: rate-limited retries.