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

add options to enable aws sdk debug log and add more logs when driver… #830

Merged

Conversation

AndyXiangLi
Copy link
Contributor

… start up

Is this a bug fix or adding new feature?
Fixes #216
What is this PR about? / Why do we need it?
Add a new driver level option -aws-sdk-debug-log, if set to true, aws go sdk will add debug log to the std output.
Sample for debug log

2021/04/15 00:41:50 DEBUG: Send Request ec2/DescribeVolumes failed, attempt 0/8, error RequestError: send request failed
caused by: Post "https://ec2.us-west-2.amazonaws.com/": dial tcp 54.240.253.45:443: i/o timeout
2021/04/15 00:41:50 DEBUG: Request ec2/DescribeVolumes Details:

Also adjust and update the klog level across driver.
What testing is done?
unit test
e2e test

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 15, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AndyXiangLi

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 15, 2021
@coveralls
Copy link

coveralls commented Apr 15, 2021

Pull Request Test Coverage Report for Build 1840

  • 27 of 36 (75.0%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 82.042%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/driver/driver.go 5 6 83.33%
pkg/driver/node.go 8 9 88.89%
pkg/driver/node_linux.go 0 1 0.0%
pkg/cloud/cloud.go 8 14 57.14%
Files with Coverage Reduction New Missed Lines %
pkg/cloud/cloud.go 1 82.15%
Totals Coverage Status
Change from base Build 1823: 0.2%
Covered Lines: 1832
Relevant Lines: 2233

💛 - Coveralls

@AndyXiangLi
Copy link
Contributor Author

/test pull-aws-ebs-csi-driver-unit

)

// MockCloud is a mock of Cloud interface.
// MockEC2 is a mock of EC2 interface
Copy link
Contributor

Choose a reason for hiding this comment

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

how come it's mocking ec2 interface now? the ec2 mock should go to mock_ec2.go (looking at script https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/hack/update-gomock#L21)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting.. Can you try on your end? I just ran make bin/mockgen under project root path and get this EC2 mock added into the mock_cloud.go

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried with latest mockgen, it only changed my go.sum : /

~/g/s/g/k/aws-ebs-csi-driver (master|✔) $ rm bin/mockgen 
~/g/s/g/k/aws-ebs-csi-driver (master|✔) $ make mockgen
go get github.com/golang/mock/mockgen@latest
go: found github.com/golang/mock/mockgen in github.com/golang/mock v1.5.0
go: downloading golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e
go: downloading golang.org/x/mod v0.3.0
./hack/update-gomock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

figured that out.. I didn't specify the interface name when I ran the command.. updated now

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 16, 2021
@AndyXiangLi
Copy link
Contributor Author

/test pull-aws-ebs-csi-driver-migration-test-latest

@AndyXiangLi
Copy link
Contributor Author

@ayberk migration test is consistently failing the first time, any thoughts?

@wongma7
Copy link
Contributor

wongma7 commented Apr 16, 2021

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 16, 2021
@k8s-ci-robot k8s-ci-robot merged commit 25edc97 into kubernetes-sigs:master Apr 16, 2021
@ki0 ki0 mentioned this pull request Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Deep health check in driver probe
4 participants