Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Update vendor to bring in updated CSI translation library #133

Merged
merged 1 commit into from
Mar 28, 2019
Merged

Update vendor to bring in updated CSI translation library #133

merged 1 commit into from
Mar 28, 2019

Conversation

leakingtapan
Copy link

@leakingtapan leakingtapan commented Mar 26, 2019 •
edited
Loading

This is needed as part of CSI migration. Updated k8s.io/csi-translation-lib to kubernetes-1.14.0 since the latest translation library is depending on k8s v1.14 released staging repo k8s.io/api for LabelZoneFailureDomain.

Not sure why there are some behavior change along with the dependency update, so that I have to update a test case to make test happy.

/cc @ddebroy @davidz627 @msau42

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 26, 2019
@ddebroy
Copy link
Contributor

ddebroy commented Mar 27, 2019 •
edited
Loading

Looks good but how about removing the various constraints tied to k8s versions in the gopkg.toml in a manner similar to https://github.com/kubernetes-csi/external-provisioner/commits/master/Gopkg.toml

msau42
msau42 reviewed Mar 27, 2019
View reviewed changes
pkg/controller/csi_handler_test.go
@@ -624,7 +624,7 @@ func TestCSIHandler(t *testing.T) {
},
expectedCSICalls: []csiCall{
{"attach", "projects/UNSPECIFIED/zones/testZone/disks/testpd", testNodeID,
map[string]string{"partition": "0"}, noSecrets, readWrite, success, notDetached, noMetadata, 0},
map[string]string{"partition": ""}, noSecrets, readWrite, success, notDetached, noMetadata, 0},
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is due to kubernetes/kubernetes#74932

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be better to separate out this change into a separate commit in this PR (with the main commit focussing on the vendoring update).

Copy link
Author

@leakingtapan leakingtapan Mar 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@msau42 so this is expected then
@ddebroy yeah.. just having a separate commit for vendor is a change that breaks build. Maybe its okay in this case?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes this is expected

jsafrane added a commit to jsafrane/external-attacher that referenced this pull request Mar 28, 2019
@jsafrane
Bump Kubernetes 1.14
6503b3a 
From kubernetes-csi#133
@jsafrane jsafrane mentioned this pull request Mar 28, 2019
Merged
@msau42
Copy link
Collaborator

msau42 commented Mar 28, 2019

/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 28, 2019
@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 28, 2019
@k8s-ci-robot k8s-ci-robot merged commit d4ee00c into kubernetes-csi:master Mar 28, 2019
@leakingtapan leakingtapan deleted the vendor-update branch March 29, 2019 00:43
@pohly pohly mentioned this pull request Feb 3, 2021
Merged
pohly added a commit to pohly/external-attacher that referenced this pull request Feb 3, 2021
@pohly
Squashed 'release-tools/' changes from 1d60e779..7bc70e52
29fe6c8 
7bc70e52 Merge pull request kubernetes-csi#129 from pohly/squash-documentation
e0b02e72 README.md: document usage of --squash
316cb957 Merge pull request kubernetes-csi#132 from yiyang5055/bugfix/boilerplate
26e2ab10 fix: default boilerplate path
1add8c18 Merge pull request kubernetes-csi#133 from pohly/kubernetes-1.20-tag
3e811d6c prow.sh: fix "on-master" prow jobs

git-subtree-dir: release-tools
git-subtree-split: 7bc70e5264a5ce5f47780bdbc6c7b7f4e79243fa
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@msau42 msau42 msau42 left review comments

@davidz627 davidz627 Awaiting requested review from davidz627

@ddebroy ddebroy Awaiting requested review from ddebroy

Assignees

@msau42 msau42

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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@leakingtapan @ddebroy @msau42 @k8s-ci-robot