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

Update aws-sdk to v1.29.11 to get IMDSv2 support #463

Merged
merged 1 commit into from
Mar 21, 2020

Conversation

msau42
Copy link
Contributor

@msau42 msau42 commented Mar 12, 2020

Is this a bug fix or adding new feature?
New feature

What is this PR about? / Why do we need it?
Update the SDK to get support for IMDSv2.

There potentially is a latency regression for IMDSv1: aws/aws-sdk-go#2972, but supposedly it has been fixed: aws/aws-sdk-go#3066.

In my testing I have not noticed any regression that would cause driver errors.

What testing is done?
Ran k8s external-storage e2es on a cluster with IMDSv1 (and no v2), and separately on a cluster with IMDSv2 (only) enabled.

Change-Id: I7a05fd6931c4d8596b24cd72c9162af63c5cbcbf
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 12, 2020
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1036

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 77.43%

Totals Coverage Status
Change from base Build 1031: 0.0%
Covered Lines: 1386
Relevant Lines: 1790

💛 - Coveralls

@leakingtapan
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 12, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leakingtapan, msau42

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 the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 12, 2020
@leakingtapan
Copy link
Contributor

/retest

1 similar comment
@leakingtapan
Copy link
Contributor

/retest

@leakingtapan
Copy link
Contributor

Done setting up docker in docker.
Activated service account credentials for: [pr-kubekins@kubernetes-jenkins-pull.iam.gserviceaccount.com]
+ make test-e2e-single-az
go get github.com/aws/aws-k8s-tester/e2e/tester/cmd/k8s-e2e-tester@master
go: k8s.io/kubernetes@v1.17.3 requires
	k8s.io/repo-infra@v0.0.1-alpha.1 requires
	github.com/golangci/golangci-lint@v1.18.0 requires
	github.com/sourcegraph/go-diff@v0.5.1 requires
	sourcegraph.com/sqs/pbtypes@v0.0.0-20180604144634-d3ebe8f20ae4: unrecognized import path "sourcegraph.com/sqs/pbtypes" (parse https://sourcegraph.com/sqs/pbtypes?go-get=1: no go-import meta tags (meta tag localhost:3080/sqs/pbtypes did not match import path sourcegraph.com/sqs/pbtypes))

@msau42
Copy link
Contributor Author

msau42 commented Mar 16, 2020

@leakingtapan my pr did not modify github.com/aws/aws-k8s-tester/e2e/tester/cmd/k8s-e2e-tester dependency. Should we fix it to a specific version instead of picking up master?

@leakingtapan
Copy link
Contributor

leakingtapan commented Mar 18, 2020

Same error happening in the other driver: kubernetes-sigs/aws-fsx-csi-driver#132

@leakingtapan
Copy link
Contributor

Should be fixed by: #468

@leakingtapan
Copy link
Contributor

/retest

@k8s-ci-robot k8s-ci-robot merged commit 1f76f9b into kubernetes-sigs:master Mar 21, 2020
@msau42 msau42 deleted the update-sdk-master branch April 23, 2020 23:12
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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants