Discussion: whether we should merge serving and eventing operators into one repository

[houshengbo]

As it has been brought during the Serving API meeting today on Jan 29, https://docs.google.com/document/d/1NC4klOdNaU-N-PsKLyXBqDKgNSHtxCDep29Ta2b5FK0, we need a broader discussion of this topic.

Please share your comments down below, regarding which option you would like to pick and why.

1. Keep serving and eventing operators separated as they are.
2. Merge serving and eventing operators into one operator, living in one repository.
3. Abstain.

Remember! Say why from any perspective. Thanks.

[duglin]

2 - merge. Easier UX and management. Customers will want (in essence) a snapshot of "Knative". A consistent set of components that have been tested by us and packaged together.

[trshafer]

2 - merge.

Agree with @duglin. I think eventing will become more expected when "installing knative" as eventing grows. The goal of a kubernetes operator is to automate what a human operator would do. If a human operator wants to update knative components, the human should only need to go to one place.

The dependency of serving from eventing, imo, means that the operator should be responsible for ensuring the two systems are compatible.

Sort of an implementation detail but I could still see two separate CRDs that the operator listens to depending on configuring and operating serving & eventing.

[salaboy]

I agree with @trshafer and @duglin I believe that a unified operator will simplify the user experience.

[trshafer]

I should provide some thoughts on why we would not want to merge the eventing and serving operator.

Eventing and serving could have different release cadences. If the operator is the way to install knative (everything), then there is a possibility that serving changes block a release of eventing. If eventing has their own operator then eventing has fewer dependencies on releasing.

[houshengbo]

Based on the nature that one version of operator installs one version of knative software, I do not know how to do the PATCH release for operator, when only serving/eventing does a PATCH release.

If serving release 0.13.1, but eventing does not have 0.13.1, does operator release the version 0.13.1 for 0.13.1 serving and 0.13.0 eventing?

Personally, I would like to see operator catch up all the releases of serving and eventing.

[anniefu]

2 - merge. In addition to above previously mentioned reasons, because eventing is dependent on serving version, the eventing operator would always have to have built-in logic regarding serving CR and versions anyway, and that could get very hard to maintain quickly

For user's clarity, it would be best if the CRDs were merged too since it's important to be aware of the serving version when making changes to the eventing CR. It also minimizes the potential race conditions in the reconcile loops if serving and eventing are both changed.

@houshengbo Would the operator release tied to specific versions of serving and eventing? I would expect an operator release to be compatible with multiple versions of serving & eventing to support upgrades.

[duglin]

w.r.t. eventing and serving have different patch releases... they should be the same. If eventing changes and needs 0.13.1 but serving doesn't, then serving's 0.13.1 release will look the same as 0.13.0.

vX.Y.Z should be a snapshot of the entire system at that point in time, even if a particular component didn't change from the previous version. That makes it very easy for a user to know what they're getting for any particular Knative-wide snapshot/version.

[evankanderson]

2 - merge.

I'd prefer to see the operator handle the difficulties of potentially needing to combine e.g. a 0.13.1 serving and a 0.13.2 eventing release to get an "up to date" 0.13 release, rather than asking every human running a Kubernetes cluster to do that.

[houshengbo]

@anniefu It is going to be a very complicated matrix, we have to maintain for each version of operator, able to install multiple(how many) versions of knative, in terms of single version testing and upgrade testing. It is a great feature support, but it is a lot of overhead as knative community has to maintain.

[matzew]

If serving release 0.13.1, but eventing does not have 0.13.1, does operator release the version 0.13.1 for 0.13.1 serving and 0.13.0 eventing?

wouldn't it be also worth a thought, that the operator might have it's own release cycle ? That way wwould not artificially need to make a serving 0.13.0 to be a 0.13.1, when the code is exactly the same

At some point ... IMO, it will not scale to release all the things in the same version schema...?

[aliok]

From a code maintenance perspective, it makes sense. I have pretty much the same doubts above because of the versioning though.

For user's clarity, it would be best if the CRDs were merged too since it's important to be aware of the serving version when making changes to the eventing CR. It also minimizes the potential race conditions in the reconcile loops if serving and eventing are both changed.

If the CRDs is a single, 2 controllers will be touching a CR. I think this would increase the potential race conditions. ...unless the controllers are also merged.

I don't have a very strong opinion on single vs multiple CRDs though. We can make both approaches work. Having separate CRDs perhaps makes more sense because it is simpler compared to a big CRD.

[markusthoemmes]

I feel strongly against merging the CRDs. We could in theory provide one top-level CRD that includes both the KnativeServing and the KnativeEventing CRD spec but I don't see a ton of value in that. I'd imagine it to look like this:

struct Knative {
  Serving KnativeServingSpec ...
  Eventing KnativeEventingSpec ...
}

The only thing it's controller would do is crank out the respective children and copy up the status of those children. Best of both worlds, but as I said: Not sure about the value of that.

The PATCH releases don't worry me a lot as (at least until now), they have only been manifest upgrades and didn't need any changes to the operator's code itself. The operator's versioning though, might need to be adjusted as was pointed out. It's worth noting that up until now, we have not cut a noop patch release if only one part of the system needed it. If we really want to unify our releases, we should probably do that. That'd also make the operator releasable on the same version always, which is preferable I suppose.

[pmorie]

During the WG meeting today, I took an AI to write up a feature track for this so we can understand exactly what is being proposed before moving forward; hope to have that completed in the next couple of days.

[matzew]

While I support the merge of the operators - I'd not want the CRs for serving/eventing to merge ...

[k4leung4]

@pmorie Any update on the feature track?

thanks

[aliok]

Feature track doc is here: https://docs.google.com/document/d/1wT6clZg_zuihtRv4dhOPtxkJTtxodV1Pm00I08KMrjI/edit#heading=h.n8a530nnrb

@k4leung4