Skip to content
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

ignore FLATNOTES_PORT on kubernetes #188

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sparksis
Copy link

@sparksis sparksis commented May 26, 2024

Fix for #187

@sparksis
Copy link
Author

Validate truth table with test.sh

resulting in output:

Using docker, not explicitly overwritten
FLATNOTES_PORT=
KUBERNETES_PORT=
Selected Port: 8080


Using docker, explicitly overwritten
FLATNOTES_PORT=1234
KUBERNETES_PORT=
Selected Port: 1234


Using kubernetes, not explicitly overwritten
FLATNOTES_PORT=tcp://127.0.0.1:80
KUBERNETES_PORT=tcp://127.0.0.1:80
Warning: ignoring FLATNOTES_PORT=tcp://127.0.0.1:80 on kubernetes
Selected Port: tcp://127.0.0.1:80


Using kubernetes, explicitly overwritten
FLATNOTES_PORT=1234
KUBERNETES_PORT=tcp://127.0.0.1:80
Warning: ignoring FLATNOTES_PORT=1234 on kubernetes
Selected Port: 1234


Not a real scenario
FLATNOTES_PORT=
KUBERNETES_PORT=tcp://127.0.0.1:80
Selected Port: 8080

@reddec
Copy link

reddec commented May 28, 2024

Could be fixed in manifest by enableServiceLinks: false

apiVersion: apps/v1
kind: Deployment
metadata:
  name: "..."
spec:
  selector:
    matchLabels:
      kind: "..."
  template:
    metadata:
      labels:
        kind: "..."
    spec:
      enableServiceLinks: false

@sparksis
Copy link
Author

Indeed, would you like me to update the readme in this PR instead?

@dullage
Copy link
Owner

dullage commented May 30, 2024

Thanks for the PR.

The setting of the environment variables in entrypoint.sh is a workaround for Synology users. Both are set in the Dockerfile, which should be enough, but when Synology users updated (from a version that didn't contain these env vars) they weren't being loaded until the container was recreated (see #178).

This makes me hesitant to merge this PR as it would be a workaround for something caused by another workaround. I might dig into the Synology issue more first, as it might be a bug that's on their radar (in which case I could take this out of entrypoint.sh once it's fixed).

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