added environment variable CF_INSTANCE_PORTS #3230
Conversation
| @@ -138,9 +138,11 @@ func (b *ProcessEnvBuilder) buildPortEnv(ctx context.Context, cfApp *korifiv1alp | |||
| processPorts := ports.FromRoutes(cfRoutesForProcess.Items, cfApp.Name, cfProcess.Spec.ProcessType) | |||
| if len(processPorts) > 0 { | |||
| portString := strconv.FormatInt(int64(processPorts[0]), 10) | |||
| cfInstancePorts := fmt.Sprintf("[{\"internal\":%s},{\"internal\":%s}]", portString, portString) | |||
There was a problem hiding this comment.
Why do we need internal twice? I guess we need external as well?
There was a problem hiding this comment.
very good question! So i basically copied what i saw that a productive CF instance gave me by default. I got there:
CF_INSTANCE_PORTS env is: [{"internal":8080,"external_tls_proxy":61157,"internal_tls_proxy":61001},{"internal":8080,"internal_tls_proxy":61443}]
so my assumption is, that the first {} holds ALL ports and the second {} holds all "internal" ports. since I didn't know what the tls_proxy ports are in the korifi environment, I left them out for now.
There was a problem hiding this comment.
Could it be that in the example above you have had two routes for the same app? Docs for that env var do not seem to mention anything about the number of json entries.
There was a problem hiding this comment.
no, there is just one route when I do "cf routes". Could the route be hidden somehow? i didn't specifically create one...
There was a problem hiding this comment.
I have spun a traditional cf deployment, pushed dora from CATS and got
vcap@a0213476-fa1a-43c1-5ec8-99af:~$ echo $CF_INSTANCE_PORTS | jq .
[
{
"external": 61000,
"internal": 8080,
"external_tls_proxy": 61002,
"internal_tls_proxy": 61001
},
{
"external": 61000,
"internal": 8080,
"internal_tls_proxy": 61443
},
{
"external": 61001,
"internal": 2222,
"external_tls_proxy": 61003,
"internal_tls_proxy": 61002
}
]
weird...
The app has a single route:
❯ cf app dora
Showing health and status for app dora in org org1 / space space2 as admin...
name: dora
requested state: started
routes: dora.mel-c.korifi.cf-app.com
last uploaded: Thu 25 Apr 10:48:43 UTC 2024
stack: cflinuxfs4
buildpacks:
name version detect output buildpack name
ruby_buildpack 1.10.13 ruby ruby
type: web
sidecars:
instances: 1/1
memory usage: 256M
state since cpu memory disk logging details
#0 running 2024-04-25T10:48:55Z 0.7% 76.5M of 256M 130.2M of 1G 0B/s of unlimited
type: worker
sidecars:
instances: 0/0
memory usage: 256M
There are no running instances of this process.
❯ cf route mel-c.korifi.cf-app.com -n dora
Showing route dora.mel-c.korifi.cf-app.com in org org1 / space space2 as admin...
domain: mel-c.korifi.cf-app.com
host: dora
port:
path:
protocol: http
Destinations:
app process port app-protocol
dora web 8080 http1
There was a problem hiding this comment.
Talked to a CF developer:
proposal: once external_tls_proxy and internal_tls_proxy gets added, we could simply add the internal only (external ones not exposed if envoy is active). For now, internal should be enough but there seem to be apps, that rely on the existance of all fields
marsteg
force-pushed
the
cf_instance_ports
branch
from
8d9e04a to
6008735
Compare
|
@danail-branekov, @georgethebeatle: as discussed, i removed the 2nd port for now |
| @@ -350,6 +350,10 @@ var _ = Describe("EnvBuilder", func() { | |||
| "Name": Equal("PORT"), | |||
| "Value": Equal("1234"), | |||
| }), | |||
| MatchFields(IgnoreExtras, Fields{ | |||
| "Name": Equal("CF_INSTANCE_PORTS"), | |||
| "Value": ContainSubstring("\"internal\":1234"), | |||
There was a problem hiding this comment.
How about we use MatchJSON instead of ContainSubstring? thus we could ensure that the content of the value is a valid json
There was a problem hiding this comment.
Done! I suppose we could also create a struct and json.Marshall it... but i'm note sure if that's better as json shouldn't change?
marsteg
force-pushed
the
cf_instance_ports
branch
2 times, most recently
from
4b33d9a to
2869d9f
Compare
Is there a related GitHub Issue?
#3229
What is this change about?
Adding environment variable
CF_INSTANCE_PORTSto workloadsDoes this PR introduce a breaking change?
not, that i expect it
Acceptance Steps
Tag your pair, your PM, and/or team
@danail-branekov @georgethebeatle
Additional info
I've seen strange errors when executing
go test -v: 12:04:37.313765 45784 reflector.go:462] pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:229: watch of *v1.Secret ended with: an error on the server ("unable to decode an event from the watch stream: context canceled") has prevented the request from succeeding - i hope that this is some local issue...