migtools / mig-controller

OpenShift Migration Controller
Apache License 2.0
22 stars 42 forks source link

Investigate if protocol buffers can be activated for controller-runtime #644

Open alaypatel07 opened 4 years ago

alaypatel07 commented 4 years ago

AFAIK, protocol buffer client is only available for native kube resources and a handful of OpenShift resources (not sure about the exact list). It is not available for Custom Resources.

{"level":"info","ts":1598323532.655684,"logger":"plan|tldm2","msg":"","error":"object *v1.BackupStorageLocationList does not implement the protobuf marshalling interface and cannot be encoded to a protobuf message","stacktrace":"\ngithub.com/konveyor/mig-controller/pkg/controller/migplan.PlanStorage.ensureBSL()\n\t/Users/alay/Documents/OpenSource/Golang/src/github.com/konveyor/mig-controller/pkg/controller/migplan/storage.go:140\ngithub.com/konveyor/mig-controller/pkg/controller/migplan.ReconcileMigPlan.ensureStorage()\n\t/Users/alay/Documents/OpenSource/Golang/src/github.com/konveyor/mig-controller/pkg/controller/migplan/storage.go:57\ngithub.com/konveyor/mig-controller/pkg/controller/migplan.(*ReconcileMigPlan).Reconcile()\n\t/Users/alay/Documents/OpenSource/Golang/src/github.com/konveyor/mig-controller/pkg/controller/migplan/migplan_controller.go:246\ngithub.com/konveyor/mig-controller/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem()\n\t/Users/alay/Documents/OpenSource/Golang/src/github.com/konveyor/mig-controller/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:215\ngithub.com/konveyor/mig-controller/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1()\n\t/Users/alay/Documents/OpenSource/Golang/src/github.com/konveyor/mig-controller/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:158\ngithub.com/konveyor/mig-controller/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil.func1()\n\t/Users/alay/Documents/OpenSource/Golang/src/github.com/konveyor/mig-controller/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133\ngithub.com/konveyor/mig-controller/vendor/k8s.io/apimachinery/pkg/util/wait.JitterUntil()\n\t/Users/alay/Documents/OpenSource/Golang/src/github.com/konveyor/mig-controller/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:134\ngithub.com/konveyor/mig-controller/vendor/k8s.io/apimachinery/pkg/util/wait.Until()\n\t/Users/alay/Documents/OpenSource/Golang/src/github.com/konveyor/mig-controller/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:88\nruntime.goexit()\n\t/usr/local/Cellar/go/1.14.2_1/libexec/src/runtime/asm_amd64.s:1373"}

But we need to investigate if controller-runtime has a way to do it without native kube support.

djwhatle commented 4 years ago

@alaypatel07 is it possible that we are already using protobuf for native k8s resources?

djwhatle commented 4 years ago

No mention of protobuf in the kubebuilder book. https://book.kubebuilder.io/

alaypatel07 commented 4 years ago

@djwhatle I think this is highly unlikely that it's enabled by default. At the very least we need to set up a header as follows to request apiserver for protobuf content type [1]

kubeConfig.AcceptContentTypes = "application/vnd.kubernetes.protobuf,application/json"

This might be an esoteric topic that most folks running controller-runtime clients don't need to worry about. Some digging on Tuesday revealed that the version on controller-runtime client we use hard coded application/json for watch events so its a bug for which we might need to upgrade the client-go dependencies to make it work with protobuf.

I will start digging more once its proved that optimizing encoding/decoding is something we want short term, deferring this until then.

  1. https://github.com/openshift/cluster-etcd-operator/blob/0596212a6173daf0ae91486819da24d16b4d438f/vendor/github.com/openshift/library-go/pkg/controller/controllercmd/builder.go#L224
djwhatle commented 3 years ago

More info on this https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/client/apiutil/apimachinery.go#L42-L49

Seems possibly promising, we may be able to use protobuf for all of the built in resources (which I think we expect will cost us the most memory)

func init() {
    // Currently only enabled for built-in resources which are guaranteed to implement Protocol Buffers.
    // For custom resources, CRDs can not support Protocol Buffers but Aggregated API can.
    // See doc: https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/#advanced-features-and-flexibility
    if err := clientgoscheme.AddToScheme(protobufScheme); err != nil {
        panic(err)
    }
}
alaypatel07 commented 3 years ago

From this https://github.com/kubernetes-sigs/controller-runtime/commit/7c26bc082e53eb784746b32b7067d4d1974f064f it looks like the minimum version of controller-runtime needed is v0.7.0, we are way off that number using v0.1.11 https://github.com/konveyor/mig-controller/blob/ec65b20e15db531681fcfcdff55d69e790f77b45/go.mod#L47. But if we can find a way to use the protobuf for core and OpenShift resources, it would be a huge perf booster