Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: only provision BackendConfig if available #76

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Sep 11, 2024

Add .Capablities checks for the cloud.google.com/v1 group to the lighttpd and krakend BackendConfig resources. This CRD is only available on GKE and blocks install on other cluster types otherwise.

Caveats:

  • This does not add any generic equivalent of the configuration in the BackendConfigs.
  • These checks may not be supported on all CD Helm integrations.

More on these after the jump.


No generic replacement

This particular resource appears like it'd only be useful on GKE. The generic equivalent of the healthcheck is the Pod template readiness or liveness check. krakend already uses an equivalent readiness check, but lighttp uses a command-based readiness check instead.

Some features of BackendConfig may be available in GWAPI configuration, but those configuration parameters will almost always be in Extended support, and won't be available across all GWAPI implementations.

GWAPI does not provide an healthcheck feature specific to individual Route resources AFAIK.

As an example of something that is available via both BackendConfig and GWAPI, https://gateway-api.sigs.k8s.io/geps/gep-1742 does appear to be in the released GWAPI APIs (based on https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.HTTPRouteTimeouts). We're not setting timeouts though, so I don't think there's any reason to add that or other Extended configuration to the HTTPRoutes yet.

CD integrations do not support standard Helm features that require API server communication

.Capabilities checks (and anything that involves talking to the Kubernetes API server, such as lookup) historically have not played nicely with CD Helm integrations that invoke helm template instead of helm install. Some CD applications have partial workarounds for this.

IIRC ArgoCD does a decent job of handling its own .Capabilities-style lookups and passing --api-versions to helm template. I don't recall other CD solutions that do, but I have less experience with them. I didn't find good info for FluxCD on a cursory search.

helm/helm#9426 maybe allows CD systems that use helm template (somewhat confusingly, you can use --dry-run with helm template, and IIRC the server communication works as expected with it--I recall playing with it a while ago). However, I haven't reviewed adoption of this feature across CD solutions. Some have feature issues linked to that Helm issue.

Add .Capablities checks for the cloud.google.com/v1 group to the
lighttpd and krakend BackendConfig resources. This CRD is only available
on GKE and blocks install on other cluster types otherwise.
@bcfriesen
Copy link
Contributor

Thanks - I was not aware of the .Capabilities check, that is neat.

Since this may not be compatible with CD for the reasons you describe, I wonder if there is a more static/prescriptive way to handle this, like if one invokes Helm and specifically asks for a GKE deployment, then create the BackendConfig object, otherwise don't create it.

@bcfriesen
Copy link
Contributor

Thanks - I was not aware of the .Capabilities check, that is neat.

Since this may not be compatible with CD for the reasons you describe, I wonder if there is a more static/prescriptive way to handle this, like if one invokes Helm and specifically asks for a GKE deployment, then create the BackendConfig object, otherwise don't create it.

The other thing we could do is just dump the GKE-isms altogether from this Helm chart. I found GKE to be useful as a starting point, having no other ready access to a "standard" K8s environment. But unless someone is considering deploying this in GKE, it may not be worth the trouble of adding flexibility to these Helm charts to enable it where possible.

OTOH, your proposed fix is 4 LOC, which seems straightforward to me, so that seems good enough for now.

@bcfriesen bcfriesen merged commit cbb07a8 into OpenCHAMI:main Sep 13, 2024
@rainest rainest deleted the feat/backend-capabilities-check branch September 16, 2024 17:23
@rainest
Copy link
Contributor Author

rainest commented Sep 16, 2024

Since this may not be compatible with CD for the reasons you describe, I wonder if there is a more static/prescriptive way to handle this.

You can combine (or (<capabilities check succeeds>) (.Values.somekey == true)) to make opportunistic installs when info is available, while leaving a user opt-in switch if it's not.

Main issue there is wanting to avoid creating configuration surface if you can and organizing it if you can't. There's no true "standard" Kubernetes in practice--environments are always going to have a mix of core API resources and non-core resources to provide a complete system.

We'll most likely want to take advantage of common non-core resources (major cloud provider resources, Prometheus- or Istio-specific resources, etc.) when we can, but Helm doesn't provide great tooling for layering additional functionality over the base. You can make a GKE/EKS/AKS/whatever-specific section, but I don't have a great perspective on whether to try and do that or whether to try and somehow integrate that configuration alongside the associated core resources' configuration.

<insert vague thoughts around maybe handling this better with kustomize overlays, but neither having a way to integrate that with Helm nor a complete plan to do it without Helm>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants