Skip to content

Add hostname/ip information to VKC loadbalancer status#2412

Merged
robobario merged 1 commit intokroxylicious:mainfrom
robobario:include-hostname-or-address-in-ingress-status
Jul 9, 2025
Merged

Add hostname/ip information to VKC loadbalancer status#2412
robobario merged 1 commit intokroxylicious:mainfrom
robobario:include-hostname-or-address-in-ingress-status

Conversation

@robobario
Copy link
Copy Markdown
Member

@robobario robobario commented Jul 7, 2025

Type of change

  • Enhancement / new feature

#status

Description

When reporting the VKC status for an ingress:

If the ingress is associated with a Service with a loadBalancer object in its status we report each ingress point's ip or hostname in the VKC status.

for example I edit the downstream-tls example KafkaProxyIngress to:

kind: KafkaProxyIngress
apiVersion: kroxylicious.io/v1alpha1
metadata:
  name: cluster-ip
  namespace: my-proxy
spec:
  proxyRef:
    name: simple
  loadBalancer:
    bootstrapAddress: bootstrap.example.com
    advertisedBrokerAddressPattern: broker-$(nodeId).example.com

Then run minikube tunnel and I see in the status

  status:
    ... snip ...
    ingresses:
    - bootstrapServer: bootstrap.example.com:9083
      loadBalancerIngressPoints:
      - ip: 127.0.0.1
      name: cluster-ip
      protocol: TLS
    observedGeneration: 2

which corresponds to the ip from the status of the service:

kubectl get service -nmy-proxy simple-sni -o=jsonpath='{.status.loadBalancer.ingress[0]}'
{"ip":"127.0.0.1","ipMode":"VIP"}

Additional Context

The first time a user sets up a LoadBalancer KafkaProxyIngress they will be responsible for configuring their DNS to resolve some hostnames to the loadbalancer ip/hostname. That means we must instruct users on how to locate the shared SNI service and extract its details. It's a bit fiddly.

With this change, the ip/hostname the user needs to plug into their DNS will be visible on the VirtualKafkaCluster status.

Closes #2295

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • PR raised from a fork of this repository and made from a branch rather than main.
  • Write tests
  • Update documentation
  • Make sure all unit/integration tests pass
  • Make sure all Sonarcloud warnings are addressed or are justifiably ignored.
  • If applicable to the change, trigger the system test suite. Make sure tests pass.
  • If applicable to the change, trigger the performance test suite. Ensure that any degradations to performance numbers are understood and justified.
  • Ensure the PR references relevant issue(s) so they are closed on merging.
  • For user facing changes, update CHANGELOG.md (remember to include changes affecting the API of the test artefacts too).

NOTE: You must be a member of @kroxylicious/developers to trigger the system test and performance test suites. If you are not part of this group, comment on the PR requesting a trigger, tagging @kroxylicious/developers.

@robobario robobario requested a review from a team as a code owner July 7, 2025 04:54
@robobario robobario force-pushed the include-hostname-or-address-in-ingress-status branch 2 times, most recently from fd078a6 to 314267a Compare July 7, 2025 23:30
Copy link
Copy Markdown
Member

@SamBarker SamBarker left a comment

Choose a reason for hiding this comment

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

LGTM with a couple of nits.

.withProtocol(serverCert.isEmpty() ? Protocol.TCP : Protocol.TLS);
return Stream.of(builder.build());
Optional<ServiceBootstrap> bootstrap = bootstraps.stream().filter(
b -> b.bootstrap().ingressName().equals(ingress.getIngressRef().getName()) && b.bootstrap().clusterName().equals(name(cluster)))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be worth extracting a method reference. Something like bootstrapMatchesIngress or something else descriptive.

Comment on lines +304 to +305
A list containing ingress points for the load-balancer. Traffic
intended for this proxy ingress should be sent to these ingress points.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should be explicit that we expect users to arrange DNS etc based on these values. Though maybe thats too involved for field level documentation?

Copy link
Copy Markdown
Member Author

@robobario robobario Jul 9, 2025

Choose a reason for hiding this comment

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

Yeah it feels too involved, maybe I should remove the second sentence as it's misleading (can be misinterpreted as saying it's a hostname you can plug into a client) and stick to describing what it is, not how we intend people to use it.

@robobario robobario force-pushed the include-hostname-or-address-in-ingress-status branch from 314267a to fa9cc84 Compare July 9, 2025 01:20
When reporting the VKC status for an ingress:

If the ingress is associated with a Service with a `loadBalancer` object in its
status we report each `ingress` point's ip or hostname in the VKC status.

Why:

The first time a user sets up a LoadBalancer KafkaProxyIngress they will
be responsible for configuring their DNS to resolve some hostnames to
the loadbalancer ip/hostname. That means we must instruct users on how
to locate the shared SNI service and extract its details.

Signed-off-by: Robert Young <robeyoun@redhat.com>
Co-authored-by: Sam Barker <sam@quadrocket.co.uk>
@robobario robobario force-pushed the include-hostname-or-address-in-ingress-status branch from fa9cc84 to b6a1c3d Compare July 9, 2025 02:12
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jul 9, 2025

@robobario robobario enabled auto-merge July 9, 2025 02:43
@robobario robobario merged commit d3712aa into kroxylicious:main Jul 9, 2025
5 checks passed
k-wall added a commit that referenced this pull request Jul 16, 2025
Signed-off-by: Keith Wall <kwall@apache.org>
@k-wall k-wall added the kube/operator Relates to the Kubernetes operator label Jul 16, 2025
k-wall added a commit that referenced this pull request Jul 17, 2025
Signed-off-by: Keith Wall <kwall@apache.org>
k-wall added a commit that referenced this pull request Jul 17, 2025
Signed-off-by: Keith Wall <kwall@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kube/operator Relates to the Kubernetes operator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose load balancerip or hostname values on the VKC ingresses status

3 participants