Closed BostjanBozic closed 1 year ago
Merging #396 (3799cf1) into main (9819182) will decrease coverage by
0.10%
. The diff coverage isn/a
.:exclamation: Current head 3799cf1 differs from pull request most recent head 412a3bc. Consider uploading reports for the commit 412a3bc to get more accurate results
@@ Coverage Diff @@
## main #396 +/- ##
==========================================
- Coverage 59.58% 59.48% -0.11%
==========================================
Files 19 19
Lines 1888 1888
==========================================
- Hits 1125 1123 -2
- Misses 737 738 +1
- Partials 26 27 +1
Impacted Files | Coverage Δ | |
---|---|---|
pkg/validation/validation.go | 81.20% <0.00%> (-1.35%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 9819182...412a3bc. Read the comment docs.
Initial impression from me is we might want to break this up into smaller more digestible pieces to review since the number of changes here is pretty big for a single PR
Also consider waiting to merge all this to after the upcoming release since this is a large change (size maybe not actual complexity)
@sunjayBhatia thanks for looking into it. I am up for waiting to do this after next release. Regarding breaking this into smaller pieces, I would try to keep most of changes in single PR (most of the changes are reshuffling file location and comments), but maybe we could have a separate one for Makefile
changes.
@sunjayBhatia Do you think it would be time to look into this now that v1.17.0 is released? I also saw there were updates to controller-runtime
, so I would maybe wait until this is pushed in kubebuilder and then review additional changes this creates?
This is stale, closing out.
Updates project schema to Kubebuilder v3 compliant schema. Following changes have been performed:
PROJECT
file is now Kubebuilder v3 compliantapi/v1alpha1
examples
folder has been moved toconfig/samples
to follow Kubebuilder v3 scaffoldingMakefile
has been revamped - addedhelp
, reshuffled command order, updatedtest
target to use newsetup-env
binary (could also be used for https://github.com/projectcontour/contour/issues/3832)Kustomize
templating based on Kubebuilder scaffolding.namePrefix
is not used, since it changes deployment name tocontour-operator-contour-operator
, which breakstest/e2e/operator_test.go
(same string used for deployment name and container name). If wanted, we can sort this out as well.nonroot
to UID65532
(default by Kubebuilder)controller-runtime
update fromv0.9.0-beta0
tov0.9.2
There are quite some minor tweaks, mostly to naming conventions, so it would be great if someone else can take a look as well and provide some feedback what we want to keep "the old way" and what in "kubebuilder v3" way.
Regarding failing
test-e2e
:latest
tag still includesnonroot
user and since new deployment includes parametersecurityContext.allowPrivilegeEscalation: false
, it fails on deployinglatest
version. AswaitForImage
is waiting for all pods with labelcontrol-plane: contour-operator
to have latest image, it reaches timeout (old pod is never terminated, as deletion is not triggered, since loop is waiting for new pod to become available).Updates: https://github.com/projectcontour/contour-operator/issues/392
cc: @danehans @youngnick
Signed-off-by: Bostjan Bozic bozic.bostjann@gmail.com