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 CSINodeInfo #101

Merged
merged 3 commits into from
Nov 27, 2018
Merged

Conversation

jsafrane
Copy link
Contributor

Kubernetes 1.13 introduced CSINodeInfo.Spec + Status. Fix the code and vendor kubernetes 1.13.0-beta2.

Fixes: #99

@xing-yang, I fixed your PR to vendor the right code from the right places (I think :-).

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 26, 2018
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 26, 2018
@xing-yang
Copy link
Contributor

Thanks @jsafrane ! I'll give this PR a try.

@xing-yang
Copy link
Contributor

I tested this PR with and without enabling CSINodeInfo, but I still got the error:

external-attacher logs:

I1126 20:03:33.457293 1 csi_handler.go:524] Can't get CSINodeInfo 127.0.0.1: csinodeinfos.csi.storage.k8s.io "127.0.0.1" not found
I1126 20:03:33.457309 1 csi_handler.go:388] Saving attach error to "csi-cc697e353e2d1b67f03d4c6e37d7f7c04480c3126d12b036e1e29c3517bd0512"
I1126 20:03:33.459175 1 csi_handler.go:398] Saved attach error to "csi-cc697e353e2d1b67f03d4c6e37d7f7c04480c3126d12b036e1e29c3517bd0512"
I1126 20:03:33.459194 1 csi_handler.go:103] Error processing "csi-cc697e353e2d1b67f03d4c6e37d7f7c04480c3126d12b036e1e29c3517bd0512": failed to attach: node "127.0.0.1" has no NodeID annotation

driver-registrar logs:

I1126 20:00:30.487440 1 connection.go:136] GRPC call: /csi.v1.Node/NodeGetInfo
I1126 20:00:30.487454 1 connection.go:137] GRPC request:
I1126 20:00:30.495795 1 connection.go:139] GRPC response: node_id:"ubuntu,iqn:iqn.1993-08.org.debian:01:c6961059376e"
I1126 20:00:30.496273 1 connection.go:140] GRPC error:
I1126 20:00:30.496526 1 node_register.go:63] CSI driver node ID: "ubuntu,iqn:iqn.1993-08.org.debian:01:c6961059376e"
I1126 20:00:30.497795 1 node_register.go:86] Starting Registration Server at: /registration/csi-opensdsplugin-reg.sock
I1126 20:00:30.499127 1 node_register.go:93] Registration Server started at: /registration/csi-opensdsplugin-reg.sock

@davidz627
Copy link
Contributor

@xing-yang are you using the new plugin registration dir?
https://github.com/kubernetes-sigs/gcp-compute-persistent-disk-csi-driver/blob/master/deploy/kubernetes/base/node.yaml#L64
path: /var/lib/kubelet/plugins_registry/
as opposed to the old: path: /var/lib/kubelet/plugins

@xing-yang
Copy link
Contributor

Yes. Actually I realized I have been testing using K8S v1.13.0-beta.1 so my tests are invalid. I need to re-test using beta.2.

@xing-yang
Copy link
Contributor

Tested with CSINodeInfo enabled using K8S v1.13.0-beta.2. It works!
Will test without CSINodeInfo next.

@xing-yang
Copy link
Contributor

Tested without CSINodeInfo using K8S v1.13.0-beta.2. It works as well!

@xing-yang
Copy link
Contributor

LGTM

xing-yang added a commit to sodafoundation/nbp that referenced this pull request Nov 27, 2018
This PR updates csi-attacher image built from this fix: kubernetes-csi/external-attacher#101.
This fix allows attacher to work with and without CSINodeInfo feature gate.
@saad-ali
Copy link
Member

/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 Nov 27, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsafrane, saad-ali

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 merged commit af1e619 into kubernetes-csi:master Nov 27, 2018
name = "github.com/json-iterator/go"
version = "1.1.4"

// TODO: use [[constraint]] when kubernetes-csi/kubernetes-csi-migration-library uses 1.13 code
Copy link
Contributor

Choose a reason for hiding this comment

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

@saad-ali
Copy link
Member

Let's cherry pick this to release-1.0 branch

@jsafrane
Copy link
Contributor Author

Picked in #103

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

Successfully merging this pull request may close these issues.

CSINodeInfo should not be required
5 participants