Adds Initial Gateway Support #207
Conversation
danehans
requested review from
stevesloka and
sunjayBhatia
and removed request for
a team
|
needs rebase @danehans |
danehans
force-pushed
the
pkg_refactor_with_svc_apis_gateway_impl
branch
2 times, most recently
from
1550db9 to
043a831
Compare
danehans
force-pushed
the
pkg_refactor_with_svc_apis_gateway_impl
branch
2 times, most recently
from
1d77a64 to
3c83bb0
Compare
|
|
||
| // OwningGatewayNameLabel is the owner reference label used for a Gateway | ||
| // managed by the operator. The value should be the name of the Gateway. | ||
| OwningGatewayNameLabel = "contour.operator.projectcontour.io/owning-gateway-name" |
There was a problem hiding this comment.
nit: Do we need contour in front of the name? Also ending it with owning-gateway-name feels out of place.
Maybe make the owner label: projectcontour.io/operator?
There was a problem hiding this comment.
I wanted to provide consistency with the contour owning ns/name labels. Child resources of the Gateway, i.e. Envoy daemonset, would look like:
kind: DaemonSet
metadata:
name: envoy
namespace: foo-gateway-ns
labels:
contour.operator.projectcontour.io/owning-gateway-name: foo-gateway
contour.operator.projectcontour.io/owning-gateway-namespace: foo-gateway-ns
We could drop contour.operator.projectcontour.io/owning-gateway-namespace since the child resources of a gateway will always be in the same ns as the gateway. I can see loosing the contour prefix, but the Contour owning labels should be updated too for consistency. I'd prefer doing that in a follow-up PR. xref: #216
|
|
||
| // OwningGatewayNsLabel is the owner reference label used for a Gateway | ||
| // managed by the operator. The value should be the namespace of the Gateway. | ||
| OwningGatewayNsLabel = "contour.operator.projectcontour.io/owning-gateway-namespace" |
There was a problem hiding this comment.
Same here in previous comment.
There was a problem hiding this comment.
Same response as #207 (comment).
danehans
force-pushed
the
pkg_refactor_with_svc_apis_gateway_impl
branch
from
3c83bb0 to
846b9fb
Compare
@stevesloka note that this PR now includes a gateway e2e test. |
danehans
force-pushed
the
pkg_refactor_with_svc_apis_gateway_impl
branch
from
846b9fb to
09e4b8d
Compare
|
@stevesloka commit |
Signed-off-by: Daneyon Hansen <[email protected]>
danehans
force-pushed
the
pkg_refactor_with_svc_apis_gateway_impl
branch
from
09e4b8d to
31f5fbf
Compare
@skriss the Gateway CRDs need to be installed in the cluster before running the operator. Fortunately, they are installed as part of the operator deployment manifest. |
That's a good point @skriss. I can make a future issue, but we shouldn't require folks to install those CRDs to use the operator. |
|
@skriss @stevesloka does Contour read a CLI flag, i.e. |
|
Contour checks if the APIs exist in the cluster and if they do then they are enabled for use. |
There was a problem hiding this comment.
LGTM.
I have a small, non-blocking question though: What happens if a Route is used by multiple Gateways? Right now the ownership labels will just be stomped. I don't know if this will come up often, but it feels like something that we should cover, since I don't think the API says that you can't do that.
@youngnick the operator is not applying ownership labels to xRoute resources since it does not manage them. I expect every projectcontour/contour controller that admits a route, to adds its gateway ns/name to the route's status. |
Adds initial Gateway support per operator design spec.
Requires: #204
Requires: #198
Fixes: #106
Fixes: projectcontour/contour#3319