recipe: exec hook implementation #1932
Conversation
| Container string | ||
| } | ||
|
|
||
| // Parameters that are of interest are hook -- name, namespace, |
There was a problem hiding this comment.
If you want to document the function, then start the comment with the function name.
| Stderr: errBuf, | ||
| }) | ||
| if err != nil { | ||
| return fmt.Errorf("error executing command: %s", errBuf.String()) |
There was a problem hiding this comment.
ensure command is also printed
| return fmt.Errorf("error executing command: %s", errBuf.String()) | ||
| } | ||
|
|
||
| log.Info("executed exec command successfully", "pod", execPod.PodName, "namespace", execPod.Namespace, "command", execPod.Command, |
There was a problem hiding this comment.
Might have to move to debug log but can be done in the next PR
| */ | ||
| if e.Hook.LabelSelector != nil { | ||
| eps, err := e.getExecPodsUsingLabelSelector(client, log) | ||
| if err != nil { |
There was a problem hiding this comment.
It is possible that the user provides both labelSelector and NameSelector but only one of them match with the pods. We should ignore errors in the collection stage and execute on whatever was found.
|
|
||
| cmd, err := covertCommandToStringArray(e.Hook.Op.Command) | ||
| if err != nil { | ||
| log.Error(err, "error occurred during exec hook execution") |
There was a problem hiding this comment.
Print the command and also a return is missing
|
|
||
| cmd, err := covertCommandToStringArray(e.Hook.Op.Command) | ||
| if err != nil { | ||
| log.Error(err, "error occurred during exec hook execution") |
There was a problem hiding this comment.
same comment as above here. And maybe we should factor out the common parts of these two functions.
| "k8s.io/apimachinery/pkg/fields" | ||
| "k8s.io/client-go/kubernetes" | ||
| "k8s.io/client-go/tools/remotecommand" | ||
| "k8s.io/kubectl/pkg/scheme" |
There was a problem hiding this comment.
check if this is the right scheme pkg to use. I was thinking it would be under controller-runtime
There was a problem hiding this comment.
Updated, please check if the approach is fine.
|
Also, rename the file json_util_test.go to check_hook_test.go |
asn1809
force-pushed
the
asn-exec-hooks
branch
6 times, most recently
from
e832659 to
9b7729d
Compare
Incorporating review comments Fixing capture issue Signed-off-by: Annaraya Narasagond <[email protected]>
Adding unit test for hooks_factory. Adding unit test for exec hooks using fake client. Signed-off-by: Annaraya Narasagond <[email protected]>
For hook type "exec", commands can be provided which needs to be executed on the Pods as part of recipe workflow. Based on the either labelSelector or nameSelector, pods can be selected. If both are provided, OR logic will apply. Kubernetes core client is used in order to execute the command on the pods. The required permissions were added as part of #1946