Catalog: Use EndpointSlice and propagate Kubernetes Topology information to synced consul service

[jukie]

Changes proposed in this PR

• Switch to EndpointSlice vs Endpoints
  • Necessary for accessing endpoint zone information but it's also the preferred API to use. Ref: https://kubernetes.io/docs/concepts/services-networking/endpoint-slices/#motivation
• Adds topology zone from Endpoint to Cosul service meta
• Adds topology region from Endpoint's associated Node to Consul service meta
• Adds get, list, and watch permissions for EndpointSlice to sync catalog's ClusterRole
• Removes get, list, and watch permissions for Endpoints from sync catalog's ClusterRole

How I've tested this PR

• Existing and new tests
• Built and deployed catalog service to verify

How I expect reviewers to test this PR

• Enable service sync and validate the registered instances have a listed meta entry for external-k8s-topology-zone and external-k8s-topology-region

Checklist

• Tests added
• CHANGELOG entry added

[david-yu]

@jukie Are there similarities between this PR and #3522? Not sure if you're also able to incorporate some of those changes into this PR, if not that's ok.

[jukie]

@david-yu Similar end result yes, but that PR specifically targets NodePort services and uses underlying node labels. My change doesn't currently include pulling in region information as that's not directly available on EndpointSlice.

[jukie]

My PR now includes region info for NodePort services as well so encompasses everything from the mentioned PR plus the addition of zone info for all other service types

[david-yu]

Thank you, I will try to have someone review sometime later next week.

[jukie]

@david-yu took it further to include all service types now

[jmurret]

👋 @jukie thank you so much for this work! It looks great. I can see that the some of the unit tests in catalog package in control-plane are failing locally and in CI (you may not have access to see CI runs). Do these pass for you? If you have any time to resolve them, that would be a great help. Otherwise, I can try to take a look at the end of the week. 🙏

[jukie]

Yes these succeeded for me but I haven't re-tested after pulling in latest commits of main. Will doublecheck that in a few hours, thanks for taking a look!

[jukie]

Found the problem. The nodes being referenced by the endpoints in the test also need to exist so we can grab region info.

[jmurret]

Updated somebroken helm chart tests andalso fixed logic in sync-catalog-clusterrole. TestSyncCatalogNamespaces and TestSyncCatalogNamespaces acceptance tests are currently failing. will try to take a look tomorrow.

[jmurret]

It passed. It was called with this below. basically packages are split into groupings to go on runners and then are used to set the PACKAGES envvar here. So, all packages will have their tests run this way.

PACKAGES="api-gateway ingress-gateway sync example consul-dns"
  echo "Running packages: ${PACKAGES}"
  for PKG in ${PACKAGES}
  do
    FULLPKG="github.com/hashicorp/consul-k8s/acceptance/tests/${PKG}"
    echo "Testing package: ${FULLPKG}"
    go run gotest.tools/gotestsum@v1.11.0 \
      --format=github-actions \
      --jsonfile="jsonfile-${PKG////-}" \
      --packages="${PKG}" \
      --junitfile="/tmp/test-results/acceptance/${PKG}-gotestsum-report.xml" \
      -- "${FULLPKG}" -p 1 -timeout 2h -failfast \
      -use-kind \
      -kube-contexts="kind-dc1,kind-dc2,kind-dc[3](https://github.com/hashicorp/consul-k8s-workflows/actions/runs/8327109167/job/22784276823#step:32:3),kind-dc[4](https://github.com/hashicorp/consul-k8s-workflows/actions/runs/8327109167/job/22784276823#step:32:4)" \
      -enable-enterprise \
      -enable-multi-cluster \
      -debug-directory=/tmp/test-results/debug \
      -consul-image=docker.mirror.hashicorp.services/hashicorppreview/consul-enterprise:1.19-dev \
      -consul-k8s-image=docker.io/hashicorppreview/consul-k8s-control-plane:1.[5](https://github.com/hashicorp/consul-k8s-workflows/actions/runs/8327109167/job/22784276823#step:32:5).0-dev-pr-5606e615d94262ac878a0b3be374b7f19565256a \
      -consul-dataplane-image=docker.mirror.hashicorp.services/hashicorppreview/consul-dataplane:1.5-dev \
      -enable-enterprise -enable-multi-cluster -enable-transparent-proxy
  done

[jukie]

@kolorful did you have a use case for region or would topology zone be good enough?

[kolorful]

@jukie does that make any difference? I don't have usecase region yet, but figured why not include region when adding az cause we might need it later on.

[jukie]

Main difference is the zone information being available on the endpoints directly. It's probably not a big deal but was curious if there was any concern over the amount of extra api server calls to get each node. That already happens for NodePort type services so again, probably not a big deal to expand that to all service types.

[jmurret]

@jukie thanks for raising your scalability concerns about also grabbing node information while processing endpoint slices. I think we would want a way to turn that off via configuration/helm chart since we don't have load tests asserting its performance. I propose we remove that from this PR since there is no current requirement for Region and when it is needed we can add the appropriate helm configuration. SOund ok?

This PR looks great. I think the two things needed to approve and merge are:

• remove the Region/node lookups
• make the endpointslice pointer change that you mentioned.

Thanks again for your efforts on this!

[jukie]

Done. I've also taken another stab at dropping endpoints permission from the catalog cluster role because I really don't think we need it (all tests are passing for me without) but you can add that back in again if preferred.

[jukie]

@jmurret could please kick off the acceptance tests again?

[kolorful]

@jukie I see, thanks for explaining. I think not having region info is fine, because usually it's part of the zone info anyway.

[jmurret]

🚀 🙏 thank you @jukie for sticking with this and getting it done. We appreciate it!

[jukie]

Awesome! Will this automatically land on the -dev release for the image and helm charts?

[jmurret]

@jukie there is no -dev channel/artifact for dev releases. you'll have to use the helm chart on main until the next 1.4 patch release. the values.yaml has a link to an internal dev preview that I think you should have access to:

consul-k8s/charts/consul/values.yaml

Line 69 in 5279204

image: docker.mirror.hashicorp.services/hashicorppreview/consul:1.19-dev