You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This issue collects several improvements and general ideas that were either out of the scope for the upgrade to kubebuilder v2 or were things we noticed while doing the upgrade.
Manager resource type: Transition the deployment model from using StatefulSet to use multiple Deployment replicas + leader election. This was removed from the upgrade to minimize risk as we didn't have the time to test all the functionality around leader election.
Metrics: The managers are producing new Prometheus metrics that we should dedicate some time to understand and see how we get some value from them. We might have duplicate metrics in the sense that some of the API webhook calls are already covered by these default metrics so our custom metrics might be no longer be needed.
Organizational: Review placing webhooks under a root webhooks/ folder as we do for controllers/. This will be going against kubebuilder v2 convention but it should improve code layout and discoverability. Kubebuilder has reasons for doing this so we should make sure we understand the trade-offs if we were to do this.
Project layout: Review how we structure the different components (API groups). It's not easy to use just one of the components (reduce adoption and increase maintainability complexity). During the upgrade to kubebuilder v2, we were forced to upgrade all the components at once.
Acceptance test improvements:
Make which acceptance test to run selectable vs running all of the runners.
Rethink how we build the binaries and docker image so it's not required to compile and rebuild all the binaries and all parts of the docker image to run an individual acceptance test where only a small change has been made.
Webhooks: Upgraded webhooks to use admissionReviewVersions/v1. We're using the v1beta1.
CRDS validation and defaulting: https://book.kubebuilder.io/reference/markers/crd-validation.html . Adopt new CRDS validation and defaulting. We've our custom validation and defaulting that doesn't follow the same conditions and not play nice with OpenAPI Schema either.
Kubectl: Make kubectl explain work for our CRDS. This is supported now in Kubernetes but it is not working with Theatre.
- Kubernetes upgrades: Upgrading to kubernetes 1.18 is blocked at the moment. CRDS generated with controller-gen are incompatible with this version. See details here kubernetes/kubernetes#91395.
- Improvement testing: There are a few flaky tests that we should review and improve; appear to primarily be a result of the timeouts we set for eventually and on slow/busy machines tests fail.
Manifests: Adding a jsonnet mixin that helps define the CRDS objects and also to deploy theatre as well. We might want to keep Kustomize as well which play nice with controller-gen.
Logging: Zap is the new default however it doesn't really behave the way we would like it to (primarily the Error logging and stack traces is annoying). Other binaries such as theatre-envconsul and theatre-console are still using go-kit/log. We should review our logger choice.
Opensource maturity: We should decide on how mature we want this project to be from an opensource perspective and then ensure we move towards it.
Reviewing the docs from an outsider looking in with no context would be a great start. Some sections of the docs are incomplete and assume context that someone external would not have. Approaching this from the divio style would perhaps be useful. https://documentation.divio.com/
There aren't docker images available to the public. Making them open will help drive adoption.
The text was updated successfully, but these errors were encountered:
This issue collects several improvements and general ideas that were either out of the scope for the upgrade to kubebuilder v2 or were things we noticed while doing the upgrade.
Manager resource type: Transition the deployment model from using
StatefulSet
to use multipleDeployment
replicas + leader election. This was removed from the upgrade to minimize risk as we didn't have the time to test all the functionality around leader election.Metrics: The managers are producing new Prometheus metrics that we should dedicate some time to understand and see how we get some value from them. We might have duplicate metrics in the sense that some of the API webhook calls are already covered by these default metrics so our custom metrics might be no longer be needed.
Organizational: Review placing webhooks under a root
webhooks/
folder as we do forcontrollers/
. This will be going against kubebuilder v2 convention but it should improve code layout and discoverability. Kubebuilder has reasons for doing this so we should make sure we understand the trade-offs if we were to do this.Project layout: Review how we structure the different components (API groups). It's not easy to use just one of the components (reduce adoption and increase maintainability complexity). During the upgrade to kubebuilder v2, we were forced to upgrade all the components at once.
Acceptance test improvements:
Make which acceptance test to run selectable vs running all of the runners.
Rethink how we build the binaries and docker image so it's not required to compile and rebuild all the binaries and all parts of the docker image to run an individual acceptance test where only a small change has been made.
Webhooks: Upgraded webhooks to use
admissionReviewVersions/v1
. We're using thev1beta1
.CRDS validation and defaulting: https://book.kubebuilder.io/reference/markers/crd-validation.html . Adopt new CRDS validation and defaulting. We've our custom validation and defaulting that doesn't follow the same conditions and not play nice with OpenAPI Schema either.
Kubectl: Make
kubectl explain
work for our CRDS. This is supported now in Kubernetes but it is not working with Theatre.- Kubernetes upgrades: Upgrading to kubernetes 1.18 is blocked at the moment. CRDS generated withcontroller-gen
are incompatible with this version. See details here kubernetes/kubernetes#91395.- Improvement testing: There are a few flaky tests that we should review and improve; appear to primarily be a result of the timeouts we set for eventually and on slow/busy machines tests fail.Manifests: Adding a
jsonnet
mixin that helps define the CRDS objects and also to deploy theatre as well. We might want to keepKustomize
as well which play nice withcontroller-gen
.Logging: Zap is the new default however it doesn't really behave the way we would like it to (primarily the Error logging and stack traces is annoying). Other binaries such as
theatre-envconsul
and theatre-console are still using go-kit/log. We should review our logger choice.Graceful shutdown of manager in integration tests: We have deviated slightly from the standard
suite_test.go
integration test layout as the recommended way doesn't shut down the manager gracefully. We should keep an eye on how this is fixed and update our tests. envtest - Failed to Watch error on every test run kubernetes-sigs/controller-runtime#904Opensource maturity: We should decide on how mature we want this project to be from an opensource perspective and then ensure we move towards it.
Reviewing the docs from an outsider looking in with no context would be a great start. Some sections of the docs are incomplete and assume context that someone external would not have. Approaching this from the divio style would perhaps be useful. https://documentation.divio.com/
There aren't docker images available to the public. Making them open will help drive adoption.
The text was updated successfully, but these errors were encountered: