Contour Ingress & Gateway #468
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eaudetcobello
requested changes
Jun 19, 2024
Co-authored-by: eaudetcobello <155978570+eaudetcobello@users.noreply.github.com>
neoaggelos
reviewed
Jun 19, 2024
neoaggelos
reviewed
Jun 20, 2024
|
Ah interesting the api-group seems to be simply projectcontour.io: |
neoaggelos
approved these changes
Jun 21, 2024
Comment on lines
+41
to
45
| # Get gateway node port | ||
| gateway_http_port = None | ||
| util.stubbornly(retries=5, delay_s=2).on(session_instance).until( | ||
| lambda p: "cilium-gateway-my-gateway" in p.stdout.decode() | ||
| lambda p: "my-gateway" in p.stdout.decode() | ||
| ).exec(["k8s", "kubectl", "get", "service", "-o", "json"]) |
There was a problem hiding this comment.
this code here can be dangerous. we repeatedly get the list of services until my-gateway appears on the text. this is right, as the service might take some time to appear (perhaps the 5 retries with 2 seconds backoff is also very short).
However, we then retrieve them again to parse the service info and retrieve the nodePort. Note that this nodePort might not be available upon service creation, so there is a race here: If the test retrieves the service before the ports are configured, then our test will flake.
Instead, we should do all of the logic inside the util.stubbornly. Currently, it's using a lambda, but we should probably write this as a proper function instead, then do the retries until we get the nodePort we were after.
If we don't get the nodePort in time, that's OK, the timeout here will tell us about the failure
Comment on lines
+18
to
+19
| lambda p: "ingress" in p.stdout.decode() | ||
| ).exec(["k8s", "kubectl", "get", "service", "-A", "-o", "json"]) |
There was a problem hiding this comment.
same here, we fetch services until we see some arbitrary output, then fetch them again. this should all be done inside the util.stubbornly here
eaudetcobello
approved these changes
Jun 21, 2024
louiseschmidtgen
added a commit
that referenced
this pull request
Jul 4, 2024
* put a little note in docs on disabling network * first draft of contour ingress and gateway * put contour chart * minor fixes * put some fakes * more wip * helm client create ns for contour * gateway * testy yamls * gateway provisioner * ck-gateway * more gateway * gateway provider manifest to chart * remove that * add mini tls delegation chart * some cleanup * set use-proxy-protocol * yamling, put shared crds in their own chart * forgot this yaml * some more comments * gateway class renamed ck-gateway, some renaming * testing adjustments & cleanup * fix ingress class name * fix cilium ingress class * try improve tests * lint * lint2 * lint * fix test * change contour chart version * update component contour * update ingress test * little typo * ingressClassName patch * typo * cleanup cilium ingress class * update dir * cleanup * make contour charts updatable * fix gateway yamls * comments * lost braket, * beautify * comments * move tlscertdelegation * lint * undo move enabled config * update charts * match on label my-gateway * improve label check * linter * make ingress check more precise * update helm pull for contour * wait for contour common crds * add missing returns * change name in api resources await * change chart helm pull * missed a little name change * comment update Co-authored-by: eaudetcobello <155978570+eaudetcobello@users.noreply.github.com> * update resources for group version * rewrite wait for crds for two groups * update wait for crds * the rest of my comment * maybe now I understand api group conventions * Revert "maybe now I understand api group conventions" This reverts commit 854f205. * correct api-resource name * update chart helm pull * remove resource.group check optional param * cleanup * make it two loops * add images * comments --------- Co-authored-by: eaudetcobello <155978570+eaudetcobello@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Contour Ingress & Gateway
Ingress
When we
k8s enable ingresswe get a static deployment of contour:And we get this IngressClass:
When you disable ingress, those will get removed.
When you
k8s set default-tls-secret=bananasyou will get a TLSCertificateDelegation resource (from the mini /ck-ingress-tls chart) which you can use to point to your bananas secret. See hereNow for
k8s set enable-proxy-protocol=truethis sets a config arg for the static deployment of contour. Contour gets restarted and voila there is an extra Arg:Gateway
Now when you do
k8s enable gatewayyou get this dynamic gateway provisioner.This helm chart lives in ck-gateway. I also added a gateway class to the chart with which people can create their gateway resource.
Additionally, another chart ck-gateway-contour installs:
You can
k8s disable gatewayand it will remove all gateway resources.Shared CRDS
Gateway and Ingress share some common contour CRDs. These are moved to their own chart which lives in ck-contour-common.
Changes Cilium
Deploy a second gatewayclass named ck-gateway.
List of images (Moonray):