Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Reconcile xRoutes when the Services they reference are modified #247

Merged
merged 17 commits into from
Jul 1, 2022

Conversation

nathancoleman
Copy link
Member

@nathancoleman nathancoleman commented Jun 24, 2022

Fixes #230

Changes proposed in this PR:

Re-reconcile HTTPRoutes and TCPRoutes when Kubernetes Service objects that the route(s) reference as backends are created, modified, or deleted.

How I've tested this PR:

  1. Set up service(s) w/ backends using the Learn guide setup on Kind

  2. Verify everything is working
  3. Delete all services referenced by HTTPRoutes
    $ kubectl delete service/echo-1 --namespace default
    $ kubectl delete service/echo-2 --namespace default
    $ kubectl delete service/nginx --namespace default
  4. Verify that listener is removed from gateway
    • https://localhost:8443 should fail to bring up Hashicups
    • https://localhost:8443/echo should fail to bring up echo service
    • The following should show no :8443 listener
      $ kubectl port-forward pod/<gateway_pod_name> 8081:19000
      $ curl localhost:8081/listeners
  5. Restore services
    $ kubectl apply -f two-services
  6. Verify everything is working again after a few seconds for syncing

How I expect reviewers to test this PR:

See above

Checklist:

  • Tests added
  • CHANGELOG entry added

    Run make changelog-entry for guidance in authoring a changelog entry, and
    commit the resulting file, which should have a name matching your PR number.
    Entries should use imperative present tense (e.g. Add support for...)

@nathancoleman nathancoleman changed the title Service watch Reconcile xRoutes when the Services they reference are modified Jun 24, 2022
@nathancoleman
Copy link
Member Author

Planning to add more test coverage next week

@nathancoleman nathancoleman marked this pull request as ready for review July 1, 2022 13:24
Copy link
Contributor

@mikemorris mikemorris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming currently-skipped test passes and adding the additional routes to the HTTP route controller tests still passes as expected.

Suggested changes for HTTP route controller apply to TCP route controller too.

.changelog/247.txt Outdated Show resolved Hide resolved
internal/commands/server/k8s_e2e_test.go Outdated Show resolved Hide resolved
internal/k8s/controllers/http_route_controller.go Outdated Show resolved Hide resolved
// is affected. No need to check other refs, skip ahead to next HTTPRoute.
if refNamespace == service.Namespace && ref.Name == gateway.ObjectName(service.Name) {
matches = append(matches, route)
break nextRoute
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL a use case for goto syntax that doesn't feel too gross.

internal/k8s/controllers/http_route_controller_test.go Outdated Show resolved Hide resolved
@@ -1,5 +1,4 @@
//go:build e2e
// +build e2e
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the old format which was replaced by the go:build format in go 1.17 (docs)

@nathancoleman nathancoleman merged commit 06c12cb into main Jul 1, 2022
@nathancoleman nathancoleman deleted the service-watch branch July 1, 2022 18:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When deployment/service is deleted, after n hrs gateway listener is removed and never re-added
2 participants