Fixing port names for Istio service mesh auto config

[marcleblanc2]

• Customer was blocked on an issue with our Helm chart + Istio

• It turns out that Istio uses the first part of a port name, and service definitions, to configure its service mesh sidecar proxy containers, based on the expected traffic type; docs: https://istio.io/latest/docs/ops/configuration/traffic-management/protocol-selection/#explicit-protocol-selection

• The http- prefix in the port name wasn't updated when we switched to grpc

• There was an old unused port in the gitserver service definition, which I couldn't find any history for, it's been there for 7+ years, so I added gRPC port 3178 in its place

• Making these updates automagically fixes Istio issues, no longer requiring the Envoy patch in charts/sourcegraph/examples/envoy

• Helpful background reading on Istio here https://istio.io/latest/docs/ops/deployment/architecture/

Checklist

• Follow the manual testing process
• Update changelog
• Update Kubernetes update doc

This change should be invisible, but it's probably worth mentioning in release notes, where do you recommend I mention this?

Test plan

Developed with a customer, on their self-hosted instance with Istio

Tested on a k3s instance:

• Deployed the 6.9.0 release
• Upgraded the Helm install with this branch marc-test-fix-gitserver-ports-for-istio, which is the same code changes, but on top of the v6.9.0 release commit instead of main, to test for any issues for existing customers as they upgrade
• Configured a code host
• Cloned some repos
• Ran some searches
• Checked pod logs
• No errors / issues

[marcleblanc2]

Hey team, this change should be invisible, but it's probably worth mentioning in release notes, where do you recommend I mention this?

Do you figure any additional testing is needed?

[DaedalusG]

I think the best place we have for notes about changes specific to deployment types are these old pages: https://sourcegraph.com/docs/admin/updates/kubernetes#notes-2

Pretty good testing imo, although I wouldn't mind if @keegancsmith laid eyes upon this change

[keegancsmith]

Please announce it somewhere. It could actually break if someone, but I think in practice that is so tiny that we should rather fix it like you are doing. Thanks!

[keegancsmith]

yes I think this is safe to remove. If I try to remember the historical context it was some sort of workaround for service discovery in kubernetes before statefulsets existed.

And just for fun, this number comes from me although I see other people added it. "10810" is an old south african programmer joke. When a south african reads it out loud it is "one ou ate one ou". An ou is slang for a guy. So its one guy eating another guy... enjoy your TIL for today.

[marcleblanc2]

I love it, thank you for the TIL!

[marcleblanc2]

It could actually break if someone, but I think in practice that is so tiny that we should rather fix it like you are doing.
@keegancsmith I think I'm missing something after "if someone," am I?

[keegancsmith]

> It could actually break if someone, but I think in practice > that is so tiny that we should rather fix it like you are > doing. @keegancsmith I think I'm missing something after "if someone," am I?

@marcleblanc2 if someone targetted the names / configured istio (or something else) based on the names? I think that is possible but unlikely / easy to fix for the admin.

[marcleblanc2]

@marcleblanc2 if someone targetted the names / configured istio (or something else) based on the names? I think that is possible but unlikely / easy to fix for the admin.

Noted, thank you! Deep Search provided similar feedback, not likely, but some self-hosted customers may require manual intervention as part of their upgrade:

• Hardcoded references to port names (e.g., http instead of grpc)
• Custom NetworkPolicies referencing specific port names
• Custom monitoring that filters by port name

I'll include these in an update to https://sourcegraph.com/docs/admin/updates/kubernetes#notes-2