Skip to content

Conversation

daniel-de-vera
Copy link
Contributor

@daniel-de-vera daniel-de-vera commented Aug 5, 2025

This PR reorganizes the configuration of the HotROD services to make them easier to use in the context of local Signadot sandboxes. Specifically, it:

  • Refactors service configurations for improved compatibility with local development workflows.
  • Updates the README with step-by-step instructions for running the frontend service locally using a sandbox.

@daniel-de-vera
Copy link
Contributor Author

@scott-cotton, if this gets approved, I would tag a new version for e2e tests.. WDYT?

Copy link

signadot bot commented Aug 5, 2025

Sandbox pr-284-route created

Routing Key: hwk9t6ugf3xfy
Last Updated: 2025-08-05 17:06 UTC


🧪 Tests Summary:

Category Details Links
Executions ✅ 2/2 tests completed View Executions
Diffs ✅ No high/medium relevance differences found View Diffs
Checks ✅ 2 checks passed View Checks

View Full Report


To customize these notifications, visit Signadot Settings.

@scott-cotton
Copy link
Member

@scott-cotton, if this gets approved, I would tag a new version for e2e tests.. WDYT?

Certainly at some point. Does that require modifying the e2es? I guess the get-env etc tests could exploit the changes...

@daniel-de-vera
Copy link
Contributor Author

@scott-cotton, if this gets approved, I would tag a new version for e2e tests.. WDYT?

Does that require modifying the e2es?

No, I don't think so (but I didn't test it to confirm it).

@scott-cotton
Copy link
Member

Would it be reasonably possible to replace <NAMESPACE> with an env var $NAMESPACE by replacing the address env vars with

  • $HOSTNAME
  • $NAMESPACE
  • $PORT
    ?

If so, I think that would be more natural.

wdyt?

@daniel-de-vera
Copy link
Contributor Author

daniel-de-vera commented Aug 6, 2025

Would it be reasonably possible to replace <NAMESPACE> with an env var $NAMESPACE by replacing the address env vars with

  • $HOSTNAME
  • $NAMESPACE
  • $PORT
    ?

If so, I think that would be more natural.

wdyt?

I'm not sure I fully got your proposal, can you expand? (fyi, I tried several approaches, I really wanted to solve this within Kustomize, but the only solution I found was using vars which is deprecated, replacements doesn't work in this case because the namespace is resolved not at build time but afterwards by kubectl).

Also, one problem I did face was that when you use XXX_PORT that conflicts with the env vars injected by K8S, e.g. LOCATION_PORT=tcp://10.97.205.202:8081 (because LOCATION is also the service name).

@scott-cotton
Copy link
Member

Would it be reasonably possible to replace <NAMESPACE> with an env var $NAMESPACE by replacing the address env vars with

  • $HOSTNAME
  • $NAMESPACE
  • $PORT
    ?

If so, I think that would be more natural.
wdyt?

I'm not sure I fully got your proposal, can you expand? (fyi, I tried several approaches, I really wanted to solve this within Kustomize, but the only solution I found was using vars which is deprecated, replacements doesn't work in this case because the namespace is resolved not at build time but afterwards by kubectl).

Also, one problem I did face was that when you use XXX_PORT that conflicts with the env vars injected by K8S, e.g. LOCATION_PORT=tcp://10.97.205.202:8081 (because LOCATION is also the service name).

Did you try to use k8s dependent env vars?

@daniel-de-vera
Copy link
Contributor Author

Did you try to use k8s dependent env vars?

Oh, great, that looks exactly what I needed. I will try it 😊

@daniel-de-vera
Copy link
Contributor Author

Did you try to use k8s dependent env vars?

Oh, great, that looks exactly what I needed. I will try it 😊

@scott-cotton, check this out:

$ eval "$(signadot sandbox get-env local-hotrod-location)"
SIGNADOT_BASELINE_NAMESPACE: command not found
SIGNADOT_BASELINE_NAMESPACE: command not found
SIGNADOT_BASELINE_NAMESPACE: command not found


$ signadot sandbox get-env local-hotrod-location
export SIGNADOT_BASELINE_KIND="Deployment"                                             # constant
export SIGNADOT_BASELINE_NAMESPACE="hotrod-dev-istio"                                  # fieldRef: metadata.namespace
export SIGNADOT_BASELINE_NAME="location"                                               # constant
export MYSQL_ADDR="mysql.$(SIGNADOT_BASELINE_NAMESPACE):3306"                          # constant
export MYSQL_PASS="abc"                                                                # constant
export REDIS_ADDR="redis.$(SIGNADOT_BASELINE_NAMESPACE):6379"                          # constant
export OTEL_EXPORTER_OTLP_ENDPOINT="http://jaeger.$(SIGNADOT_BASELINE_NAMESPACE):4318" # constant
export SIGNADOT_SANDBOX_NAME="local-hotrod-location"                                   # constant (override)
export SIGNADOT_SANDBOX_ROUTING_KEY="d3gbpykjdpy5u"                                    # constant (override)

So, basically we can't use this approach because our CLI does not supports dependent variable 😔🤦
Are you ok on keeping things as is? (and I send a draft PR with the changes to support dependent variables, that I already have and fully agree is the best way to do it, and once we fix the CLI we merge it).

@scott-cotton
Copy link
Member

So, basically we can't use this approach because our CLI does not supports dependent variable 😔🤦

Surprising, I would have thought that k8s would have resolved that for the pods. Not sure why they don't.

@scott-cotton
Copy link
Member

Are you ok on keeping things as is? (and I send a draft PR with the changes to support dependent variables, that I already have and fully agree is the best way to do it, and once we fix the CLI we merge I

Did you fix the CLI/libconnect, hotrod, or both?

@daniel-de-vera
Copy link
Contributor Author

Are you ok on keeping things as is? (and I send a draft PR with the changes to support dependent variables, that I already have and fully agree is the best way to do it, and once we fix the CLI we merge I

Did you fix the CLI/libconnect, hotrod, or both?

No, just hotrod. I didn't touch the CLI (I guess we should create an issue for this).

Copy link
Member

@scott-cotton scott-cotton left a comment

Choose a reason for hiding this comment

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

LGTM

@davixcky davixcky merged commit 3a55d11 into main Aug 11, 2025
10 checks passed
@davixcky davixcky deleted the hotrod-config-reorg branch August 11, 2025 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants