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

isDocker is not working on Kubernetes v1.23 #579

Open
2 tasks done
QuanTran91 opened this issue Jan 10, 2023 · 10 comments
Open
2 tasks done

isDocker is not working on Kubernetes v1.23 #579

QuanTran91 opened this issue Jan 10, 2023 · 10 comments

Comments

@QuanTran91
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.1.1

Plugin version

No response

Node.js version

16

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

20.04

Description

Plugin is docker is not work on EKS (Elastic Kubernetes Service) 1.23

Steps to Reproduce

Run app in EKS as pod
Observe the log that it is running on 127.0.0.1

Expected Behavior

No response

@climba03003
Copy link
Member

I would say it is a upstream issue. and you are allowed to pass the address by yourself.

@sinedied
Copy link
Contributor

I would say that isDocker works as expected since K8s doesn't implement a Docker engine, but it would be nice if the fastify-cli adds some detection logic to use 0.0.0.0 as default in K8s environments. I lost 2h yesterday because of this

@mcollina
Copy link
Member

Ah! That's the catch. Do you know how can we detect that we running on K8s?

@sinedied
Copy link
Contributor

@sinedied
Copy link
Contributor

Would you accept a PR for this?

@climba03003
Copy link
Member

Would you accept a PR for this?

Yes, and I see more complete check on this package https://github.com/ntfs32/is-kubernetes/blob/main/index.js
The download is 1 / week, so I have a bit worry on the stability. But, the detection look solid to me.

@sinedied
Copy link
Contributor

Ok, I'll propose a PR then. Though I would be against adding an external dependency for something that is a 1-liner, as the environment variable is enough (according to the docs, it's always present: https://kubernetes.io/docs/tutorials/services/connect-applications-service/#environment-variables).

sinedied added a commit to sinedied/fastify-cli that referenced this issue Jan 12, 2023
@climba03003
Copy link
Member

Though I would be against adding an external dependency for something that is a 1-liner, as the environment variable is enough

I have no idea will this change in future but multiple method for detection doesn't hurt.

sinedied added a commit to sinedied/fastify-cli that referenced this issue Jan 12, 2023
sinedied added a commit to sinedied/fastify-cli that referenced this issue Jan 12, 2023
@sinedied
Copy link
Contributor

Given that any change to this would break the entire k8s ecosystem, it's very unlikely.
I'd rather thrust the official doc than than an unknown repo using a detection methods based on synchronous file reads, which could delay the startup a bit.

I've sent the PR BTW.

@climba03003
Copy link
Member

I'd rather thrust the official doc than than an unknown repo using a detection methods based on synchronous file reads, which could delay the startup a bit.

The check is OR which means it only execute when the first one fail. When the environment is always exist, there will be no file read operation. Again, fallback detection methods doesn't hurt in anyway.

When a environment really doesn't set one, then the application is no-way out either. Even worse then consuming sometime for reading file-system.

sinedied added a commit to sinedied/fastify-cli that referenced this issue Jan 12, 2023
mcollina pushed a commit that referenced this issue Jan 12, 2023
* fix: use address 0.0.0.0 in Kubernetes env (#579)

* feat: add more k8s detection methods
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

No branches or pull requests

4 participants