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

Only reconcile ing tcp for apps with tcp route #1079

Merged
merged 7 commits into from
Dec 20, 2024
Merged

Conversation

ianstanton
Copy link
Contributor

@ianstanton ianstanton commented Dec 16, 2024

  • Fix bug reconcile_ingress_tcp in app services. If any app service had a route with "ingressType": "tcp", we would create IngressRouteTCP resources for all app services in the spec.
    • Example: The follwoing appServices definition would create an IngressRouteTCP for both fdb-api and postgrest.
    • We would expect it to only create one for fdb-api because it has a route with "ingressType": "tcp", but we should not create one for postgrest
    "appServices": [
                      {
                          "name": "postgrest",
                          "image": "postgrest/postgrest:v10.0.0",
                          "routing": [
                              {
                                  "port": 3000,
                                  "ingressPath": "/",
                                  "ingressType": "http"
                              }
                          ],
                          ...
                      },
                      {
                          "name": "fdb-api",
                          "image": "ghcr.io/ferretdb/ferretdb",
                          "routing": [
                              {
                                  "port": 27018,
                                  "ingressPath": "/ferretdb/v1",
                                  "entryPoints": [
                                      "ferretdb"
                                  ],
                                  "ingressType": "tcp"
                              }
                          ],
                          ...
                   ]
    

Signed-off-by: Ian Stanton <ian@tembo.io>
@ianstanton ianstanton marked this pull request as ready for review December 17, 2024 16:52
@ianstanton
Copy link
Contributor Author

Looking into a test failure in functional_test_app_service after pulling in recent changes

@ianstanton
Copy link
Contributor Author

There are some changes that improve test flakiness in #1081. Going to pull that in once it is merged and see how things look. Though this failure doesn't seem to be a flake 🤔

@ianstanton
Copy link
Contributor Author

I think I see the issue here. This test was only passing because we were incorrectly deleting IngressRouteTCPs. That bug has since been fixed and we need to apply an update in this code

@nhudson
Copy link
Collaborator

nhudson commented Dec 19, 2024

There are some changes that improve test flakiness in #1081. Going to pull that in once it is merged and see how things look. Though this failure doesn't seem to be a flake 🤔

I closed it, but will refactor the PR a bit to make it a bit more clean.

@nhudson
Copy link
Collaborator

nhudson commented Dec 19, 2024

@ianstanton you should be able to merge upstream once I merge #1089

Signed-off-by: Ian Stanton <ian@tembo.io>
Signed-off-by: Ian Stanton <ian@tembo.io>
@ianstanton ianstanton merged commit debfb55 into main Dec 20, 2024
9 checks passed
@ianstanton ianstanton deleted the fix-app-svc-ing branch December 20, 2024 00:28
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.

3 participants