Add Gateway API and Envoy Gateway install to Calico Enterprise #3638
Conversation
| EnvoyGatewayJobContainerName = "envoy-gateway-certgen" | ||
| ) | ||
|
|
||
| func GatewayAPIResourcesGetter() func() *gatewayAPIResources { |
There was a problem hiding this comment.
This doesn't seem like something we need to have happen at run time since it will never change but it's redone multiple times when reconciliation happens. This seems to provide an unneeded source of panics that could be caught at build time.
Instead, could we just generate the structures here from the yaml at build time and save them to a file, similar to how we generate the version structures https://github.com/tigera/operator/tree/master/hack/gen-versions using go template?
There was a problem hiding this comment.
This approach of reading from YAML at runtime is one that I've already discussed (and agreed as a reasonable option) during the design phase.
There was a problem hiding this comment.
OK, even if we are going with this approach vs one that generates this at compile time, could this at least be done once at start up stored in some global variable or function?
I don't see why we'd need to run this function more than once when absolutely nothing would change, and we'd avoid missing any issues with CRD generation (since it would fail on startup, which is the next best think to failing at compile time).
This just seems to unnecessarily complicate the runtime render logic for trying to get around transferring CRDs defined in a yaml into go code.
There was a problem hiding this comment.
I think it's already doing what you think it should do. In other words, it only reads and parses the YAML once. The only reason for the slightly odd (to you) form is to have the resources cache and associated lock internal to the function, instead of being globals that other code might accidentally misuse.
The important point is to use a separate namespace that doesn't have a default-deny network policy. Therefore we don't need to configure allow policy to: 1. allow the gateway controller to egress to the Kubernetes API 2. allow traffic to and from gateways
| GatewayControllerDeployment *GatewayControllerDeployment `json:"gatewayControllerDeployment,omitempty"` | ||
|
|
||
| // Allow optional customization of the gateway certgen job. | ||
| GatewayCertgenJob *GatewayCertgenJob `json:"gatewayCertgenJob,omitempty"` |
There was a problem hiding this comment.
I think we generally write out api field names. I am assuming Certgen stands for Certificate Generation, meaning it is two words and both words should be capitalized in the field name as well.
There was a problem hiding this comment.
I'm following the Envoy resources here, which have "certgen" as a single word.
| "github.com/tigera/operator/pkg/render" | ||
| ) | ||
|
|
||
| const ResourceName string = "gatewayapi" |
There was a problem hiding this comment.
This variable is currently unused.
| // the CRDs are left in place even if the GatewayAPI CR is removed again. This is in case | ||
| // the customer uses a second (or more) implementation of the Gateway API in addition to the | ||
| // one that we are providing here. | ||
| crdComponent := render.NewPassthrough(render.GatewayAPICRDs(log)...) |
There was a problem hiding this comment.
There are some corner cases where this can be problematic, especially since we are using v1alpha CRDs, which are more likely to change:
- We don't detect if the CRDs are already provided through another mechanism. Potentially, we could get into a fight with another controller or gitops tool that is also reconciling CRDs, in which case we probably don't want to do anything here.
- If another controller applies CRDs of a different version, we should degrade.
- Perhaps we can annotate the CRDs we apply, so that we can detect these scenarios?
There was a problem hiding this comment.
I've invented a concept of a component handler having "create only" operation. PTAL ecb2315
There was a problem hiding this comment.
How is this going to work when we've updated our CRDs and need to apply the changes?
I think we need to instead know if we're managing the installation or the customer is doing it (as @rene-dekker points allude to).
@rene-dekker, how are we doing this for prometheus and eck? I think prometheus requires a different insallation step, but I can't remember what we do for eck.
There was a problem hiding this comment.
We discussed this offline and decided to follow up with the right approach for CRDs.
Ref. tigera#3638 (comment) The point here is to avoid any possible fighting between our operator and another component that might also be trying to install CRDs for the Gateway API, e.g. if a customer is using multiple Gateway API implementations at the same time. This change only stops us from fighting. It's possible to imagine more sophisticated logic, such as installing _our_ CRDs if they have versions which are later (or in some way "better") than the existing ones. But it's not obvious that that would be what our customers want - e.g. they might _need_ an older version - and even if it wasn't we could still be fighting with a less intelligent other component.
1. Follow our standard pattern for namespace creation. 2. Create pull secrets earlier, so as not to glitch the objects that may need them. 3. Fix comment about pull secrets not covering all envoy images; in fact they do!
Plus improve in a few other small ways.
Otherwise our operator code adds in the default Tigera registry, giving e.g.
gcr.io/unique-caldron-775/cnx/envoyproxy/envoy:distroless-v1.31.0
Add Gateway API and Envoy Gateway install to Calico Enterprise (cherry-pick of #3638 to the branch for v3.21ep1)
Description
For PR author
make gen-filesmake gen-versionsFor PR reviewers
A note for code reviewers - all pull requests must have the following:
kind/bugif this is a bugfix.kind/enhancementif this is a a new feature.enterpriseif this PR applies to Calico Enterprise only.