Skip to content

⚗️🎨 Add clearly identifyable container hostname ⚠️ #8215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

⚗️🎨 Add clearly identifyable container hostname ⚠️ #8215

wants to merge 3 commits into from
+14 −14

Conversation

@mrnicegyu11
Copy link
Member

@mrnicegyu11 mrnicegyu11 commented Aug 14, 2025 •
edited
Loading

What do these changes do?

For every container adds a clearly identifiable hostname. This aids debugging, as it will show up e.g. in postgres-clients in adminer, etc.

Since the hostname is re-used in many places where it is not directly obvious (postgres-client names, potentially in rabbit connection etc.) there is a small but non-zero chance that this PR breaks some stuff. We can see on master.

Related issue/s

#8199

How to test

Dev-ops

Observe that container hostname on deployments is short enough (<64 chars, valid DNS FQDN)

@mrnicegyu11 mrnicegyu11 added this to the Voyager milestone Aug 14, 2025
@mrnicegyu11 mrnicegyu11 self-assigned this Aug 14, 2025
@mrnicegyu11 mrnicegyu11 added the t:enhancement Improvement or request on an existing feature label Aug 14, 2025
@codecov
Copy link

codecov bot commented Aug 14, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.09%. Comparing base (cebad23) to head (f533eba).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8215      +/-   ##
==========================================
+ Coverage   86.79%   88.09%   +1.30%     
==========================================
  Files        1910     1910              
  Lines       73396    73396              
  Branches     1301     1301              
==========================================
+ Hits        63704    64659     +955     
+ Misses       9305     8350     -955     
  Partials      387      387
Flag Coverage Δ
integrationtests 64.31% <ø> (+0.08%) ⬆️
unittests 86.72% <ø> (-0.22%) ⬇️
Components Coverage Δ
pkg_aws_library 93.93% <ø> (ø)
pkg_celery_library 87.37% <ø> (ø)
pkg_dask_task_models_library 79.62% <ø> (ø)
pkg_models_library 93.03% <ø> (ø)
pkg_notifications_library 85.26% <ø> (ø)
pkg_postgres_database 88.02% <ø> (ø)
pkg_service_integration 70.19% <ø> (ø)
pkg_service_library 71.72% <ø> (ø)
pkg_settings_library 90.46% <ø> (ø)
pkg_simcore_sdk 85.05% <ø> (+0.05%) ⬆️
agent 93.81% <ø> (ø)
api_server 93.20% <ø> (ø)
autoscaling 95.89% <ø> (ø)
catalog 92.34% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.37% <ø> (+0.56%) ⬆️
datcore_adapter 97.94% <ø> (ø)
director 76.14% <ø> (ø)
director_v2 90.94% <ø> (+12.93%) ⬆️
dynamic_scheduler 96.27% <ø> (ø)
dynamic_sidecar 90.12% <ø> (ø)
efs_guardian 89.60% <ø> (ø)
invitations 91.44% <ø> (ø)
payments 92.60% <ø> (ø)
resource_usage_tracker 92.50% <ø> (+0.37%) ⬆️
storage 86.46% <ø> (-0.09%) ⬇️
webclient ∅ <ø> (∅)
webserver 88.27% <ø> (+0.04%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cebad23...f533eba. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI reviewed Aug 14, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds service-specific prefixes to container hostnames for better identification and debugging purposes. Each service now has a clearly identifiable hostname that includes a short prefix followed by the node hostname and task slot.

  • Adds abbreviated service prefixes to all container hostnames (e.g., "api-", "dv2-", "sw-")
  • Maintains the existing Docker Swarm templating pattern with {{.Node.Hostname}}-{{.Task.Slot}}
  • Improves debugging capabilities by making containers easily identifiable in logs and monitoring tools

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

bisgaard-itis
bisgaard-itis approved these changes Aug 14, 2025
View reviewed changes
Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Is there a good reason for not using the entire service name instead of a shorthand? That would avoid potential guessing.

@mergify
Copy link
Contributor

mergify bot commented Aug 14, 2025 •
edited
Loading

🧪 CI Insights

Here's what we observed from your CI run for f533eba.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on base branch Retries 🔍 CI Insights 📄 Logs
CI unit-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

@mrnicegyu11
Copy link
Member Author

mrnicegyu11 commented Aug 14, 2025

Is there a good reason for not using the entire service name instead of a shorthand? That would avoid potential guessing.

Yes sadly there is: docker hostnames have a maximum length (afaik 64chars), so there is a virtue in being short, in case the hostname of the machine might be long. Any container with a hostname >max_length will not start

sanderegg
sanderegg reviewed Aug 14, 2025
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

I am very scarred of such a wide change.
Many of these services do not connect with the DB at all.

I am happy to do a PR to add the application name on top of the hostname directly when connecting to the DB via the postgres clients instead. Just let me know

@mrnicegyu11
Copy link
Member Author

mrnicegyu11 commented Aug 14, 2025

I am very scarred of such a wide change. Many of these services do not connect with the DB at all.

I am happy to do a PR to add the application name on top of the hostname directly when connecting to the DB via the postgres clients instead. Just let me know

I am actually not so scared of this change but if you prefer not to have it, or to make your proposed variant, this is fine for me :)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 15, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@mrnicegyu11
Copy link
Member Author

mrnicegyu11 commented Aug 15, 2025

Closing in favor of #8220

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sanderegg sanderegg sanderegg left review comments

Copilot code review Copilot Copilot left review comments

@bisgaard-itis bisgaard-itis bisgaard-itis approved these changes

@YuryHrytsuk YuryHrytsuk Awaiting requested review from YuryHrytsuk YuryHrytsuk is a code owner

@pcrespov pcrespov Awaiting requested review from pcrespov

Assignees

@mrnicegyu11 mrnicegyu11

Labels

t:enhancement Improvement or request on an existing feature

Projects

None yet

Milestone

Voyager

Development

Successfully merging this pull request may close these issues.

3 participants

@mrnicegyu11 @sanderegg @bisgaard-itis