Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

Migrate to Kubebuilder v3 schema #396

Closed
wants to merge 3 commits into from
Closed

Migrate to Kubebuilder v3 schema #396

wants to merge 3 commits into from

Conversation

BostjanBozic
Copy link
Contributor

@BostjanBozic BostjanBozic commented Jun 28, 2021

Updates project schema to Kubebuilder v3 compliant schema. Following changes have been performed:

  • PROJECT file is now Kubebuilder v3 compliant
  • Initial comment updates to api/v1alpha1
  • examples folder has been moved to config/samples to follow Kubebuilder v3 scaffolding
  • Makefile has been revamped - added help, reshuffled command order, updated test target to use new setup-env binary (could also be used for Resolve Latest Controller Runtime Envtest Failures contour#3832)
  • updated Kustomize templating based on Kubebuilder scaffolding. namePrefix is not used, since it changes deployment name to contour-operator-contour-operator, which breaks test/e2e/operator_test.go (same string used for deployment name and container name). If wanted, we can sort this out as well.
  • Specify user in container image (change from nonroot to UID 65532 (default by Kubebuilder)
  • controller-runtime update from v0.9.0-beta0 to v0.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:

  • Test is failing on updating operator image. I believe reason for this failure is that latest tag still includes nonroot user and since new deployment includes parameter securityContext.allowPrivilegeEscalation: false, it fails on deploying latest version. As waitForImage is waiting for all pods with label control-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: #392

cc: @danehans @youngnick

Signed-off-by: Bostjan Bozic [email protected]

@BostjanBozic BostjanBozic requested a review from a team as a code owner June 28, 2021 12:39
@BostjanBozic BostjanBozic requested review from skriss and youngnick and removed request for a team June 28, 2021 12:39
@codecov
Copy link

codecov bot commented Jun 28, 2021

Codecov Report

Merging #396 (3799cf1) into main (9819182) will decrease coverage by 0.10%.
The diff coverage is n/a.

❗ Current head 3799cf1 differs from pull request most recent head 412a3bc. Consider uploading reports for the commit 412a3bc to get more accurate results
Impacted file tree graph

@@            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%) ⬇️

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.

@sunjayBhatia
Copy link
Member

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

@sunjayBhatia
Copy link
Member

Also consider waiting to merge all this to after the upcoming release since this is a large change (size maybe not actual complexity)

@BostjanBozic
Copy link
Contributor Author

@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.

@BostjanBozic
Copy link
Contributor Author

@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?

@skriss
Copy link
Member

skriss commented Sep 21, 2022

This is stale, closing out.

@skriss skriss closed this Sep 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants