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

Copy collector enhanced #237

Merged
merged 26 commits into from
Aug 24, 2020
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
f77f09f
Copy collector enhanced
manavellamnimble Aug 5, 2020
07bc145
Recursive (un)tar and redact
manavellamnimble Aug 11, 2020
829f311
Error Handling
manavellamnimble Aug 12, 2020
dba0209
Comments added
manavellamnimble Aug 12, 2020
5bfdffd
Merge pull request #1 from manavellamnimble/Copy_tar
manavellamnimble Aug 12, 2020
d5ab4e3
New command and Tar function
manavellamnimble Aug 14, 2020
03c3f07
New command and Tar function
manavellamnimble Aug 14, 2020
85976fc
Merge pull request #2 from manavellamnimble/Copy_tar
manavellamnimble Aug 14, 2020
425344a
Dirs Included in tar
manavellamnimble Aug 14, 2020
9750a79
Merge pull request #3 from manavellamnimble/Copy_tar
manavellamnimble Aug 14, 2020
6cdc808
Dirs Included in tar
manavellamnimble Aug 14, 2020
eec86f5
Merge pull request #4 from manavellamnimble/Copy_tar
manavellamnimble Aug 14, 2020
3a8126e
tar.gz and .tgz included
manavellamnimble Aug 16, 2020
2c10570
Merge pull request #5 from manavellamnimble/Copy_tar
manavellamnimble Aug 16, 2020
ffc8154
tar.gz and .tgz included
manavellamnimble Aug 16, 2020
d59e311
Merge pull request #6 from manavellamnimble/Copy_tar
manavellamnimble Aug 16, 2020
257e246
tar.gz and .tgz included
manavellamnimble Aug 17, 2020
340e0d4
Merge pull request #7 from manavellamnimble/Copy_tar
manavellamnimble Aug 17, 2020
ca6e3e4
Merge pull request #8 from manavellamnimble/Copy_tar
manavellamnimble Aug 18, 2020
af004be
tar.gz and .tgz included
manavellamnimble Aug 18, 2020
0225123
Copy into folder
manavellamnimble Aug 19, 2020
880f435
Merge pull request #9 from manavellamnimble/Copy_tar
manavellamnimble Aug 19, 2020
58c6229
Copy into folder
manavellamnimble Aug 19, 2020
f73aff0
Merge pull request #10 from manavellamnimble/Copy_tar
manavellamnimble Aug 19, 2020
b4a6a86
command modified
manavellamnimble Aug 21, 2020
6c222eb
Merge pull request #11 from manavellamnimble/Copy_tar
manavellamnimble Aug 21, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 60 additions & 4 deletions pkg/collect/copy.go
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"fmt"
"path/filepath"
"strings"

troubleshootv1beta1 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta1"
corev1 "k8s.io/api/core/v1"
Expand All @@ -14,6 +15,7 @@ import (
)

func Copy(c *Collector, copyCollector *troubleshootv1beta1.Copy) (map[string][]byte, error) {

client, err := kubernetes.NewForConfig(c.ClientConfig)
if err != nil {
return nil, err
Expand Down Expand Up @@ -56,12 +58,13 @@ func Copy(c *Collector, copyCollector *troubleshootv1beta1.Copy) (map[string][]b
}

func copyFiles(c *Collector, client *kubernetes.Clientset, pod corev1.Pod, copyCollector *troubleshootv1beta1.Copy) (map[string][]byte, map[string]string) {
files := make(map[string][]byte)
container := pod.Spec.Containers[0].Name
if copyCollector.ContainerName != "" {
container = copyCollector.ContainerName
}

command := []string{"cat", copyCollector.ContainerPath}
command := []string{"find", copyCollector.ContainerPath, "-type", "f"}

req := client.CoreV1().RESTClient().Post().Resource("pods").Name(pod.Name).Namespace(pod.Namespace).SubResource("exec")
scheme := runtime.NewScheme()
Expand Down Expand Up @@ -96,6 +99,9 @@ func copyFiles(c *Collector, client *kubernetes.Clientset, pod corev1.Pod, copyC
Stderr: &stderr,
Tty: false,
})

filepaths := strings.Split(output.String(), "\n")

if err != nil {
errors := map[string]string{
filepath.Join(copyCollector.ContainerPath, "error"): err.Error(),
Expand All @@ -109,9 +115,59 @@ func copyFiles(c *Collector, client *kubernetes.Clientset, pod corev1.Pod, copyC
return nil, errors
}

return map[string][]byte{
copyCollector.ContainerPath: output.Bytes(),
}, nil
for _, file := range filepaths {
if file != "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be better written has

if file == "" {
  continue
}

The code is less conditionally indented and more readable this way

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeLingoBot capture continue in conditional in loop

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule captured at https://dash.codelingo.io/repos/replicatedhq/troubleshoot/rules/1392

CodeLingoBot Help

When responding to PR-level comments, I understand the following commands:

  • capture to capture the context for a fresh rule.

  • review to trigger a fresh review.

When responding to comments on specific lines of code, I understand the following commands:

  • capture to capture the context for a fresh rule.

  • review to trigger a fresh review.

  • ignore to ignore the parent review comment.

  • unignore to stop ignoring a parent review comment.

command := []string{"cat", file}

req := client.CoreV1().RESTClient().Post().Resource("pods").Name(pod.Name).Namespace(pod.Namespace).SubResource("exec")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessarily verbose.

req := client.Corev1().Pods(pod.Namespace).Get(pod.Name)

Is cleaner and how we use the clientset

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CodeLingoBot capture unnecessary use of client-go restClient

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule captured at https://dash.codelingo.io/repos/replicatedhq/troubleshoot/rules/1393

CodeLingoBot Help

When responding to PR-level comments, I understand the following commands:

  • capture to capture the context for a fresh rule.

  • review to trigger a fresh review.

When responding to comments on specific lines of code, I understand the following commands:

  • capture to capture the context for a fresh rule.

  • review to trigger a fresh review.

  • ignore to ignore the parent review comment.

  • unignore to stop ignoring a parent review comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Marc! I didn't have much contact with the client set yet, so I left that line as it was. The way you suggest returns another type of variable and requires another type of variable too instead of pod.Name. I didn't investigate it further because I am sure I will have to deal with the clientset in later PR's, and I didn't wanted to delay the copy collector.
That said I can either change it later, once I've dealt with the client set, or look into it now, as you prefer!

scheme := runtime.NewScheme()
if err := corev1.AddToScheme(scheme); err != nil {
return nil, map[string]string{
filepath.Join(copyCollector.ContainerPath, "error"): err.Error(),
}
}
parameterCodec := runtime.NewParameterCodec(scheme)
req.VersionedParams(&corev1.PodExecOptions{
Command: command,
Container: container,
Stdin: true,
Stdout: false,
Stderr: true,
TTY: false,
}, parameterCodec)

exec, err := remotecommand.NewSPDYExecutor(c.ClientConfig, "POST", req.URL())
if err != nil {
return nil, map[string]string{
filepath.Join(copyCollector.ContainerPath, "error"): err.Error(),
}
}

output := new(bytes.Buffer)
var stderr bytes.Buffer
err = exec.Stream(remotecommand.StreamOptions{
Stdin: nil,
Stdout: output,
Stderr: &stderr,
Tty: false,
})
files[file] = output.Bytes()

if err != nil {
errors := map[string]string{
filepath.Join(copyCollector.ContainerPath, "error"): err.Error(),
}
if s := output.String(); len(s) > 0 {
errors[filepath.Join(copyCollector.ContainerPath, "stdout")] = s
}
if s := stderr.String(); len(s) > 0 {
errors[filepath.Join(copyCollector.ContainerPath, "stderr")] = s
}
return nil, errors
}
}
}
return files, nil
}

func getCopyErrosFileName(copyCollector *troubleshootv1beta1.Copy) string {
Expand Down