Skip to content

Refactor ops-traefik #1

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.

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

Closed

Conversation

mrnicegyu11
Copy link
Owner

@mrnicegyu11 mrnicegyu11 commented May 12, 2025

What do these changes do?

  • Make traefik configurable with env-vars instead of CLI arguments --> Better interoperability with portainer
  • Remove per-cluster overwrite docker-compose files
  • Remove unused env-vars and python script

Related issue/s

Related PR/s

https://git.speag.com/oSparc/osparc-ops-deployment-configuration/-/merge_requests/1389/

Checklist

  • I tested and it works

@mrnicegyu11 mrnicegyu11 self-assigned this May 12, 2025
@mrnicegyu11 mrnicegyu11 marked this pull request as ready for review May 12, 2025 09:43
@mrnicegyu11 mrnicegyu11 requested a review from YuryHrytsuk May 13, 2025 07:22
@mrnicegyu11 mrnicegyu11 added the enhancement New feature or request label May 13, 2025
Copy link
Collaborator

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

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

Thanks.

As of now we need docker-compose.master.yml (and Co) to open some ports on a subset of deployments (service exposure) while keeping this services private on other deployments where we do not need it (security)

@@ -9,7 +9,7 @@ services:
- "/customEntrypoint.sh"
- "--api=true"
- "--api.dashboard=true"
- "--log.level=${OPS_TRAEFIK_LOGLEVEL}"
- "--log.level=${TRAEFIK_LOG_LEVEL}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: why removing the prefix? Since we have 2 traefiks, it makes sense, oder?

Copy link
Owner Author

Choose a reason for hiding this comment

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

will revert, yes

@mrnicegyu11 mrnicegyu11 requested a review from YuryHrytsuk May 13, 2025 11:34
@mrnicegyu11
Copy link
Owner Author

Now using jinja2 logic and env-vars, ports are only opened when needed. The DNS 8.8.8.8 is set everywhere.

configs:
- source: traefik_dynamic_config.yml
target: /etc/traefik/dynamic_conf.yml
env_file:
- .env
Copy link
Collaborator

@YuryHrytsuk YuryHrytsuk May 21, 2025

Choose a reason for hiding this comment

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

It is clear what env_file does.

Why do we now need env inside traefik? What change causes it?

Repository owner deleted a comment from mrnicegyu11 May 21, 2025
TRAEFIK_ENTRYPOINTS_HTTP_TRANSPORT_RESPONDINGTIMEOUTS_IDLETIMEOUT="21600s"
TRAEFIK_ENTRYPOINTS_HTTP_TRANSPORT_RESPONDINGTIMEOUTS_WRITETIMEOUT="21600s"
TRAEFIK_ENTRYPOINTS_HTTP_TRANSPORT_RESPONDINGTIMEOUTS_READTIMEOUT="21600s"
TRAEFIK_ENTRYPOINTS_RABBIT_ADDRESS=":5672"
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is redis port 🤔

Copy link
Owner Author

Choose a reason for hiding this comment

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

Does redis need to be exposed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

- target: 5433
published: 5433
{% endif %}
{% if OPS_TRAEFIK_EXPOSE_RABBITMQ|lower == "true" %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is redis port?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Does redis need to be exposed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

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

Thanks!

Left some comments

mrnicegyu11 and others added 4 commits May 28, 2025 15:49
* Update longhorn README

Document how to perform (kubernetes) node maintenance

* Update Longhorn README: disks config and maintenance

* Kubernets add local storage

Use topolvm as the most mature local storage csi.

* Update longhorn readme
@mrnicegyu11
Copy link
Owner Author

@YuryHrytsuk

re:
image

In order to remove duplication of hardcoded values, in a first step I have moved the configuration that used to be passed to traefik as CLI arguments into env-vars. This is supported by traefik according to https://doc.traefik.io/traefik/reference/static-configuration/env/

@@ -0,0 +1,198 @@
version: "3.8"
Copy link
Collaborator

@YuryHrytsuk YuryHrytsuk Jul 4, 2025

Choose a reason for hiding this comment

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

I think this will not work in CI (aka on machines) as our servers have different docker compose verison. It will probably cause version mismatch error and fail in deploy_ops CI

I propose to ditch this label altogether. version is deprecated and is not used anymore (except when define may cause errors)

See https://docs.docker.com/reference/compose-file/version-and-name/#version-top-level-element-obsolete

Copy link
Collaborator

@YuryHrytsuk YuryHrytsuk Jul 4, 2025

Choose a reason for hiding this comment

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

See example of no version definition in the newest metabase stack ITISFoundation#1093. Works like a charm 🚀

- traefik.http.middlewares.graylog_replace_regex.replacepathregex.regex=^/graylog/?(.*)$$
- traefik.http.middlewares.graylog_replace_regex.replacepathregex.replacement=/$${1}
- traefik.http.routers.graylog.middlewares=ops_whitelist_ips@swarm, ops_gzip@swarm, graylog_replace_regex
fluentd:
Copy link
Collaborator

Choose a reason for hiding this comment

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

hm, this is refactor OPS traefik PR or introduce fluentd / loki PR 🤔

Copy link
Owner Author

Choose a reason for hiding this comment

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

It is true, I made a bad merge and all was mixed :O I will make a fresh PR for traefik ,sorry @YuryHrytsuk

@YuryHrytsuk YuryHrytsuk self-requested a review July 4, 2025 08:19
Copy link
Collaborator

@YuryHrytsuk YuryHrytsuk left a comment

Choose a reason for hiding this comment

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

This is likely a very nice idea to use ENV instead of CLI Arguments as it is indeed easier to work with 👌

Left a few comments. Also seems like fluentd / loki changes go accidentally in but should be done in a separate PR

@mrnicegyu11
Copy link
Owner Author

I made a bad merge and the code got mixed up between unrelated PRs (traefik refactoring, loki, fluentd, etc.) :O I will make a fresh PR for traefik and close this one @YuryHrytsuk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants