-
Notifications
You must be signed in to change notification settings - Fork 984
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
Feat: Support Running Sidecar with a Command. #2449
base: master
Are you sure you want to change the base?
Conversation
This PR addresses issue zalando#2448 . Some containers may not have entry points, if this is the case they would need to be run using a command. This change extends the definition of sidecar so that there is an optional command field. If the field is present then the container will be run using that command. This is a two line change that is fully backward compatible.
Hi @Jan-M , @sdudoladov , @FxKu , @idanovinda , @hughcapet, @jopadi Could you please review this PR? |
Hi @FxKu could you please review this PR? It is similar to a PR you recently reviewed. Thanks |
Any updates? |
@@ -220,6 +220,7 @@ type Sidecar struct { | |||
DockerImage string `json:"image,omitempty"` | |||
Ports []v1.ContainerPort `json:"ports,omitempty"` | |||
Env []v1.EnvVar `json:"env,omitempty"` | |||
Command []string `json:"command,omitempty" protobuf:"bytes,3,rep,name=command"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is protobuf:"bytes,3,rep,name=command"
necessary? We have it nowhere else. Is it similar to filter inputs using pattern in the CRD schema?
@tabbyl21 sorry for not reacting for a looong time. We are usually a bit reluctant to changes that offer more config mean and adding complexity. After taking a closer look I get now why this change is needed. Being able to run any command via sidecar just sounded too scary to me first (well, maybe because it is). If you could answer my one question we could merge it then. Try to run codegen (./hack/update-codegen.sh) to see if something changes. |
This PR addresses issue #2448 . Some containers may not have entry points, if this is the case they would need to be run using a command. This change extends the definition of sidecar so that there is an optional command field. If the field is present then the container will be run using that command. This is a two line change that is fully backward compatible.