projectcontour / contour-operator

Experimental repository to explore an operator for deploying Contour
Apache License 2.0
43 stars 34 forks source link

Drop GatewayClass/Gateway controllers #422

Closed sunjayBhatia closed 3 years ago

sunjayBhatia commented 3 years ago

When a Contour is created, the Operator will simply pass along any GatewayClass configuration to the Contour instances, no longer only creating a Contour when a GatewayClass/Gateway are present. Removes all logic in the Operator around Gateway API Resource reconciliation, leaves it all up to Contour. Also removes all finalizer logic the Operator was adding to GatewayClasses/Gateways.

Now, in order to use Gateway API, a user will create a Contour config CRG with the Gateway Controller Name they wish Contour (not the Operator) to reconcile. The Operator will simply create a Contour instance and Envoy DaemonSet when this resource is created and Contour will handle reconciling GatewayClasses/Gateways/*Routes.

Future work to add on to this:

Draft for now, based on https://github.com/projectcontour/contour-operator/pull/420 and https://github.com/projectcontour/contour-operator/pull/421 and requires https://github.com/projectcontour/contour/pull/3881 to be merged before this

sunjayBhatia commented 3 years ago

Note: tested locally with the Contour image from https://github.com/projectcontour/contour/pull/3881 and it works, this CI will fail until that is merged

codecov[bot] commented 3 years ago

Codecov Report

Merging #422 (003c904) into main (e0dc4af) will increase coverage by 2.49%. The diff coverage is 74.28%.

:exclamation: Current head 003c904 differs from pull request most recent head e7115b0. Consider uploading reports for the commit e7115b0 to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##             main     #422      +/-   ##
==========================================
+ Coverage   74.70%   77.19%   +2.49%     
==========================================
  Files          35       29       -6     
  Lines        2704     2228     -476     
==========================================
- Hits         2020     1720     -300     
+ Misses        520      375     -145     
+ Partials      164      133      -31     
Impacted Files Coverage Δ
internal/objects/contour/contour.go 84.00% <0.00%> (-2.05%) :arrow_down:
pkg/validation/validation.go 75.00% <ø> (-10.34%) :arrow_down:
internal/controller/controller.go 71.31% <66.66%> (ø)
internal/objects/configmap/configmap.go 72.72% <92.30%> (-3.18%) :arrow_down:
internal/objects/deployment/deployment.go 92.79% <100.00%> (-0.43%) :arrow_down:
internal/operator/operator.go 43.75% <100.00%> (-31.90%) :arrow_down:
internal/status/status.go 43.24% <0.00%> (-16.22%) :arrow_down:
internal/status/conditions.go 69.23% <0.00%> (-7.70%) :arrow_down:
internal/objects/rbac.go 58.33% <0.00%> (-2.39%) :arrow_down:
... and 7 more

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 e0dc4af...e7115b0. Read the comment docs.

stevesloka commented 3 years ago

@sunjayBhatia looks like the codegen job is failing. I merged the mirror PR in contour so was rerunning tests here to validate.

sunjayBhatia commented 3 years ago

@sunjayBhatia looks like the codegen job is failing. I merged the mirror PR in contour so was rerunning tests here to validate.

do you want to review all the changes together? I also had separated out the two pre-requisites for this which were a bit big for just this one PR (links in description)

stevesloka commented 3 years ago

Ah I merged #421 and there are some conflicts now @sunjayBhatia.

sunjayBhatia commented 3 years ago

Ah I merged #421 and there are some conflicts now @sunjayBhatia.

wdyt about merging the testing one without a second approval, I think its probably less controversial imo

stevesloka commented 3 years ago

I'm fine to merge this if you're ok with it @sunjayBhatia. This should fixup a bunch of issues currently.