tf-operator should ensure that CRD exists

[ddysher]

From what I experimented, tf-operator requires crd to be created via admin manually; however, i believe in most of crd-based system, they are responsible to make sure crd exists, e.g. rook,, volume snapshot, etc

Note apart from creating crd upon startup, a robust operator should also make sure it always exists, though how to implement this varies, e.g. recreate if crd is deleted, using authz

/cc @gaocegege @codeflitting

[gaocegege]

@ddysher We have some discussions about it. In the beginning, we follow what etcd-operator does, which create the CRD in the operator. We think the logic should not be in the operator since it is not the duty of the operator. Do you think we need to add it in the operator?

[ddysher]

yes, here is my thought process, FYI

• can tf-operator work without it? My answer is no. If there is no crd, tf-operator can do nothing, which
  makes it an integral part of tf-operator.
• is there any other component depends on the crd? My answer is no. No other component should modify or delete crd, so tf-operator should take full ownership of it (there is no conflict)
• what is the management interface and overhead? Kubernetes provides decent APIs, managing crd won't complicate tf-operator.

can you point me to the counter-argument?

@ddysher We have some discussions about it. In the beginning, we follow what etcd-operator does, which create the CRD in the operator. We think the logic should not be in the operator since it is not the duty of the operator.

[gaocegege]

Make sense. We will do it.

/assign @codeflitting

[ddysher]

/cc @jlewi I think Jeremy prefers installation via ksonnet (though it's not mutually exclusive with the suggested approach).

[jlewi]

I prefer the CRD to not be created by the operator; but I think its a good idea for the operator to check if it exists.

I think there is potentially a 1 to many relationship of CRD definitions to operator deployments.

CRD's are not cluster scoped so you can only have a single CRD definition. I think (K8s 1.11 adds support for multiple versions of a CRD).

Nonetheless, you could potentially have multiple implementations of an operator running with the implementations handling different resources; e.g. each implementation could be claiming resources in different namespaces.

I think it makes sense for the operator to check if the CRD exists but I don't think it should be created by the operator. I think this creates a false expectation that the CRD is always in sync with the operator image but this is not the case because the CRD could already have existed.

The CRD (TFJob API) should be much more stable then the operator image. So we might update the operator to get some bug fixes but the TFJob API should not be changed. I think by not having the operator create the CRD we make this behavior much clearer.

If the CRD is created separately, then its clear then updating the operator will not update the CRD definition.

[ScorpioCPH]

I think its a good idea for the operator to check if it exists.

SGTM, tfjob informers will throw an error if the CRD doesn't exist, we can return the error if needed.

[ddysher]

CRD is like table/collection in database, application is responsible to manage it. Most crd based operators I've seen so far create crds, which provides better user experience.

I think it makes sense for the operator to check if the CRD exists but I don't think it should be created by the operator. I think this creates a false expectation that the CRD is always in sync with the operator image but this is not the case because the CRD could already have existed.

[jlewi]

@codeflitting Do you want to add a check to TFJobOperator to see if the CRD exists?

[jlewi]

@codeflitting @ddysher is someone from Caicloud willing to pick this up?