-
Notifications
You must be signed in to change notification settings - Fork 83
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
tail blobfuse-proxy logs at the start of blobfuse-proxy daemonset #400
Conversation
Hi @boddumanohar. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/ok-to-test
7d4993e
to
6f1c13d
Compare
6f1c13d
to
51b9e7f
Compare
echo "sleeping for 19 years" | ||
sleep 600000000s | ||
echo "sleeping for 10 secs" | ||
sleep 10s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that means it can only get logs in the exact time of after 10s, is there better way to get all logs after all tests completed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is getting all the logs. In the above case, the sleep is to wait for the blobfuse-proxy service to come up. Although it happens much quicker than that.
should filter out
|
9d06f10
to
d240383
Compare
@andyzhangx The .deb file present in the master is a little old. So I updated it with the latest one. Now we can observe we are not leaking secrets in the logs. also, the current method we are using to distribute the .deb file is a little tedious. Because if someone made changes to the blobfuse-proxy then they have to build the new .deb themselves and update the links in |
@@ -44,7 +44,7 @@ spec: | |||
apt-get update | |||
apt-get install -y blobfuse=1.3.6 | |||
# download blobfuse-proxy .deb package | |||
wget https://github.com/kubernetes-sigs/blob-csi-driver/raw/master/deploy/blobfuse-proxy/v0.1.0/blobfuse-proxy-v0.1.0.deb -O /tmp/blobfuse-proxy-v0.1.0.deb | |||
wget https://github.com/boddumanohar/blob-csi-driver/raw/issues/392/deploy/blobfuse-proxy/v0.1.0/blobfuse-proxy-v0.1.0.deb -O /tmp/blobfuse-proxy-v0.1.0.deb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of creating a separate PR to to change: boddumanohar
--> kubernetes-sigs
. Can I update it right now? So that when the PR is merged, it will point to the correct file itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@boddumanohar could add a verification script in |
d240383
to
397256f
Compare
@andyzhangx do you want me to propose the changes discussed above in this PR itself or a new PR? |
@boddumanohar new PR is ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, boddumanohar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
tail blobfuse-proxy logs at the start of blobfuse-proxy daemonset
/kind cleanup
What this PR does / why we need it: #392
Which issue(s) this PR fixes:
Fixes #
Requirements:
Special notes for your reviewer:
Release note: