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

Added support to enable and disable enableServiceLinks. #628

Merged
merged 1 commit into from
Jun 22, 2021

Conversation

sxd
Copy link
Contributor

@sxd sxd commented Jun 12, 2021

This option expose internally some KUBERNETES_* environment variables
that doesn't allow the runner to use KinD (Kubernetes in Docker) since it will
try to connect to the Kubernetes cluster where the runner it's running.

This option it's set by default to true in any Kubernetes deployment.

For more information on this option you can visit:
https://kubernetes.io/docs/concepts/services-networking/connect-applications-service/#accessing-the-service

This fix #626 by adding the option to control the behavior

Signed-off-by: Jonathan Gonzalez V jonathan.abdiel@gmail.com

@mumoshu
Copy link
Collaborator

mumoshu commented Jun 12, 2021

@sxd Thanks a lot for your efforts!

To be extra sure, the workaround is to unset all the KUBERNETES_* envvars before running kind and kubectl, right?

And this patch makes the workaround unnecessary, while making the use of https://github.com/helm/kind-action possible as the workaround doesn't work for kind-action because you can't unset envvars that kind-action reads?

@sxd
Copy link
Contributor Author

sxd commented Jun 12, 2021

@mumoshu exactly! that's exactly what this fix =) please let me know if you need more information! =)

@toast-gear toast-gear added the enhancement New feature or request label Jun 14, 2021
@toast-gear
Copy link
Collaborator

Thank for the PR, we are going to hold off merging this whilst #629 is being developed

@sxd
Copy link
Contributor Author

sxd commented Jun 14, 2021

@toast-gear There's anything I can do to help with this? just to speed up the merge, can I help with something on #629 ?

Regards!

@toast-gear
Copy link
Collaborator

Test test test! It's difficult to do full CI testing due to the project being as open source as you can get (i.e. no money for infrastructure) so the more people can test the new stateful runners in a diverse set of scenarios the quicker we can merge it

This option expose internally some `KUBERNETES_*` environment variables
that doesn't allow the runner to use KinD (Kubernetes in Docker) since it will
try to connect to the Kubernetes cluster where the runner it's running.

This option it's set by default to `true` in any Kubernetes deployment.

Signed-off-by: Jonathan Gonzalez V <jonathan.gonzalez@enterprisedb.com>
@mumoshu
Copy link
Collaborator

mumoshu commented Jun 22, 2021

#629 has been merged and I've resolved the conflict happened on this PR. Let's merge and see how it goes! Thanks again for your contribution @sxd ☺️

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.

Ability to control enableServiceLinks in RunnerDeployment pod spec
3 participants