tumblr / k8s-sidecar-injector

Kubernetes sidecar injection service

Apache License 2.0

345 stars 75 forks source link

add annotation namespace #31

Closed like-inspur closed 4 years ago

like-inspur commented 5 years ago

1、current deployment only has livenessProbe，but don't have readiness 2、configmap read config form installation config, so it's better to config this variable 3、execurte need ruby env, so tell reader to install ruby first

yahoocla commented 5 years ago

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! :smile:

like-inspur commented 4 years ago

42

byxorna commented 4 years ago

@like-inspur thanks for citing this from the other issue. I dropped comments for things we would like changed before merging this. Would you be able to address them?

Alternatively, if you would like to break your change in https://github.com/tumblr/k8s-sidecar-injector/pull/31/commits/7105418fbae87dea986caeb0c767e55779054fc7 out into a separate PR, we can merge that easily and keep it uncoupled from the rest of this PR? Thanks in advance!

like-inspur commented 4 years ago

@byxorna I have remove other change and only save this, you can merge directly

byxorna commented 4 years ago

@like-inspur thanks for rebasing this PR, and thank you for the contribution! Your changes will be available in the tumblr/k8s-sidecar-injector:latest image in probably 1h (based on docker hub build queue length). https://hub.docker.com/repository/docker/tumblr/k8s-sidecar-injector/builds