Script for collecting window and ubuntu worker logs

[arun-amzn]

*Issue #, if available: NA

*Description of changes: Script for collecting logs required for debugging Windows worker nodes

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[mogren]

@nithu0115 👀

[nithu0115]

@arun-amzn, could you please create new directory for windows instead of updating existing one to make it less cumbersome ?

[arun-amzn]

@arun-amzn, could you please create new directory for windows instead of updating existing one to make it less cumbersome ?

Should I go with creating a sub-directory for windows in log-collector-script and keep the script and readme over there. I think it better to have SSM document supports both OS. Or new directory for window log collector (log-collector-script-windows) and maintainer everything there. Please suggest @nithu0115

[nithu0115]

@arun-amzn, could you please create new directory for windows instead of updating existing one to make it less cumbersome ?

Should I go with creating a sub-directory for windows in log-collector-script and keep the script and readme over there. I think it better to have SSM document supports both OS. Or new directory for window log collector (log-collector-script-windows) and maintainer everything there. Please suggest @nithu0115

@arun-amzn, to provide better code readability and maintainability, please create 2 new sub-directories under log-collector-script - for-linux, for-windows and move the existing/new scripts, SSM documents, Readme into the respective directories.

--log-collector-script
       |--for-linux
       |--for-windows

[mogren]

@arun-amzn Thanks for adding the snap support.

Just a question though, why adding so many empty lines between every sentence in the READMEs? The looked fine before, all the empty lines do not improve the readability.

[arun-amzn]

@arun-amzn Thanks for adding the snap support.

Just a question though, why adding so many empty lines between every sentence in the READMEs? The looked fine before, all the empty lines do not improve the readability.

I think my editor were causing the issue. I have updated the READMEs now.

[mogren]

I think this line has been a bit mixed up compared to the the previous switch statement. I assume we want both kubelet and kube-proxy logs here as well, and the systemctl output? Or is kube-proxy started some other way?

      timeout 75 snap logs kubelet-eks -n all > "${COLLECT_DIR}"/kubelet/kubelet.log
      timeout 75 snap logs kube-proxy -n all > "${COLLECT_DIR}"/kubelet/kubeproxy.log

      for entry in kubelet-eks kube-proxy; do
        systemctl cat "${entry}" > "${COLLECT_DIR}"/kubelet/"${entry}"_service.txt 2>&1
      done

[arunbhagyanath]

kube-proxy is configured as daemon-set . So, there wouldn't be any logs from systemd or snap. i think we need to remove those from systemd as well. What you say ?

systemd)
      timeout 75 journalctl --unit=kubelet --since "${DAYS_10}" > "${COLLECT_DIR}"/kubelet/kubelet.log
      timeout 75 journalctl --unit=kubeproxy --since "${DAYS_10}" > "${COLLECT_DIR}"/kubelet/kubeproxy.log

      for entry in kubelet kube-proxy; do
        systemctl cat "${entry}" > "${COLLECT_DIR}"/kubelet/"${entry}"_service.txt 2>&1
      done

[mogren]

Yes, that sounds right. Also that you can't use "${entry}" in the output file path for the snap get command.

[arun-amzn]

Update the kubelet service details for snap and systemd. Also, changed the way we collect the kubeconfig when kubectl is not available.

[mogren]

Thanks a lot @arun-amzn for iterating on this PR! LGTM, but I'll ask @nithu0115 to take a look as well, since he wrote the initial version of this script.

[nithu0115]

This should be .ps1 ?

[nithu0115]

Overall, this looks good! But, few things, please see comments inline.

[nithu0115]

I don't see ignore_introspection, ignore_metrics functions in the script. Can you update the readme section to match help from the script.

[arun-amzn]

I have updated the readme for the same.

[mogren]

@arun-amzn Thanks for all your work. Just needs a rebase because of the Docker-in-Docker fix adding -o to pgrep that got merged in #389.

[arunbhagyanath]

@arun-amzn Thanks for all your work. Just needs a rebase because of the Docker-in-Docker fix adding -o to pgrep that got merged in #389.

@mogren Rebased.

[nithu0115]

LGTM! Thanks for your efforts on this. One last minor thing @arun-amzn, can you squash all these changes to single commit ?