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

adding log-collector-script in EKS AL2 AMI #967

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

ravisinha0506
Copy link
Contributor

Issue #, if available:

Description of changes:
Adding log-collector-script to the AL2 AMI for easier log collection in non-classic partition.

Testing

  • Created an ami successfully after this change
  • Can see that the file got uploaded.
sh-4.2$ ls -ltr /etc/eks/log-collector-script/
total 20
-rw-r--r-- 1 root root 18779 Jul 15 22:36 eks-log-collector.sh

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

Comment on lines +138 to +139
"source": "{{template_dir}}/log-collector-script/linux/",
"destination": "/tmp/worker/log-collector-script/"
Copy link
Member

Choose a reason for hiding this comment

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

It might be less confusing to just keep linux in the path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure how customers use the existing path. Hence, I didn't update the existing path to ensure its backward compatible.

Copy link
Member

Choose a reason for hiding this comment

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

But isn't /tmp/worker/log-collector-script/ a new path that you're adding, or is it referenced somewhere else?

Comment on lines +373 to +374
sudo mkdir -p /etc/eks/log-collector-script/
sudo cp $TEMPLATE_DIR/log-collector-script/eks-log-collector.sh /etc/eks/log-collector-script/
Copy link
Member

Choose a reason for hiding this comment

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

Why is this and the above needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous step collects files from a different directory. We need this change to copy the log-collector-script from a directory which is in parallel of target path of previous step.

Copy link
Member

Choose a reason for hiding this comment

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

I see. The other location is the tmp directory.

Comment on lines +138 to +139
"source": "{{template_dir}}/log-collector-script/linux/",
"destination": "/tmp/worker/log-collector-script/"
Copy link
Member

Choose a reason for hiding this comment

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

But isn't /tmp/worker/log-collector-script/ a new path that you're adding, or is it referenced somewhere else?

Comment on lines +373 to +374
sudo mkdir -p /etc/eks/log-collector-script/
sudo cp $TEMPLATE_DIR/log-collector-script/eks-log-collector.sh /etc/eks/log-collector-script/
Copy link
Member

Choose a reason for hiding this comment

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

I see. The other location is the tmp directory.

@ravisinha0506 ravisinha0506 merged commit 584f9a5 into master Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants