Conversation
This command takes the image of the container of the current pod and runs the specified command in a one-off kubectl run command. If the pod has multiple containers and none is specified, it will fail asking for the specific container.
|
I might have done some things wrong, so I'm really happy to receive feedback on this! |
src/cmd.rs
Outdated
| .arg("--context") | ||
| .arg(&kluster.name) | ||
| .arg("run") | ||
| .arg("one-off-shell") |
There was a problem hiding this comment.
Could be named more specific eg. <pod_name>-one-off
src/cmd.rs
Outdated
| ); | ||
|
|
||
| command!( | ||
| OneOff, |
There was a problem hiding this comment.
OneOff should be probably Run to mimic the kubectl command.
|
Would be useful to have the env variables of the specified pod set as well. |
|
The challenge with the env variables is, that there might be some variables from secrets and |
|
This looks great! I'm just gonna test it out and then will leave comments and/or merge |
nicklan
left a comment
nicklan
left a comment
There was a problem hiding this comment.
looks pretty good, modulo needing to fix the name
src/cmd.rs
Outdated
| clickwrite!(writer, "Couldon't get container image\n"); | ||
| return; | ||
| }; | ||
| let pod_name = format!("one-off-shell-{}", container_image.as_str()); |
There was a problem hiding this comment.
this needs to truncate to 63 characters, and also needs to replace any invalid or uppercase characters. from k8s: must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')
peterfication
force-pushed
the
add-one-off
branch
from
8782157 to
64b5b4e
Compare
From k8s: ``` must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*' ```
peterfication
force-pushed
the
add-one-off
branch
from
64b5b4e to
7078ddb
Compare
|
@nicklan thanks for pointing that out. I added a method to sanitise the pod name. |
|
I realised that the current implementation only allows one run command of the same image at the same time. A random sequence could be added to the K8s name so it does not fail for more run commands for the same pod/image at the same time. |
nicklan
left a comment
nicklan
left a comment
There was a problem hiding this comment.
Thanks for the fixup, it works on some stuff now, but short pod names cause a panic.
| let pod_name_regex = Regex::new(r"[^a-z0-9-]").unwrap(); | ||
| let result = pod_name_regex.replace_all(&s.to_ascii_lowercase().to_string(), "-").to_string(); | ||
|
|
||
| if &result[0..1] == "-" || &result[58..59] == "-" { |
There was a problem hiding this comment.
You can't hard-code up to 59 here since the pod name might be less than 59 characters which will cause a panic. You'll need to look at the string length and go off that.
There was a problem hiding this comment.
Oh no that's not good. I also realized that you can't run a command for the same pod twice. So while fixing this I would also introduce a random string to accommodate that use case.
There was a problem hiding this comment.
Yeah, that's true. The other option is that you can delete the pod when it's done. Maybe the right thing to do would be to default to adding a random string, but have an option like --name that lets you set the name you want to use
2 participants
This command takes the image of the container of the current pod
and runs the specified command in a one-off kubectl run command.
If the pod has multiple containers and none is specified, it will
fail asking for the specific container.