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

[cinder-csi-plugin] update CSI version #2478

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

jichenjc
Copy link
Contributor

@jichenjc jichenjc commented Nov 23, 2023

What this PR does / why we need it:

update CSI cinder version

note that csi-node-driver-registrar:v2.9.2 is not ready yet

# docker pull registry.k8s.io/sig-storage/csi-node-driver-registrar:v2.9.2
Error response from daemon: manifest for registry.k8s.io/sig-storage/csi-node-driver-registrar:v2.9.2 not found: manifest unknown: Failed to fetch "v2.9.2"

Which issue this PR fixes(if applicable):
fixes #

Special notes for reviewers:

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 23, 2023
@k8s-ci-robot k8s-ci-robot requested review from dulek and zetaab November 23, 2023 07:56
@jichenjc jichenjc changed the title update CSI version [cinder-csi-plugin] update CSI version Nov 23, 2023
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 23, 2023
@jichenjc jichenjc force-pushed the update_csi_version_2 branch from 142a5b8 to 4da7e6a Compare November 23, 2023 23:41
@zetaab
Copy link
Member

zetaab commented Nov 24, 2023

perhaps we should wait until all docker images are working from registry?

@jichenjc
Copy link
Contributor Author

ok, will give a try

2.9.1=>2.9.2 just a CVE update, not some functional , anyway , wait for a few days and let's see :)

@jichenjc
Copy link
Contributor Author

waiting for kubernetes-csi/node-driver-registrar#359

@jichenjc jichenjc force-pushed the update_csi_version_2 branch 2 times, most recently from 07e5caa to 56b35f6 Compare December 12, 2023 03:36
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2023
@jichenjc jichenjc force-pushed the update_csi_version_2 branch from 56b35f6 to d64cd51 Compare December 12, 2023 05:04
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2023
@jichenjc
Copy link
Contributor Author

@zetaab updated, please help take a look, thanks~

@kayrus
Copy link
Contributor

kayrus commented Dec 12, 2023

@jichenjc can you rebase on latest master? It looks like this PR includes full merge with the latest changes and it's hard to read the actual changes. git pull --rebase https://github.com/kubernetes/cloud-provider-openstack master should work without conflicts.

@jichenjc
Copy link
Contributor Author

@kayrus I can see only 4 files changes with 14 lines :)
I did rebase but https://github.com/kubernetes/cloud-provider-openstack/pull/2478/files seems doesn't have that much change?

@kayrus
Copy link
Contributor

kayrus commented Dec 12, 2023

@jichenjc hm, probably github somehow cached the diff. Now it looks like it should. Do you need my lgtm?

@kayrus
Copy link
Contributor

kayrus commented Dec 12, 2023

do we need to update the spec versions here as well?

$ grep -ri specVersion pkg/
pkg/csi/manila/driver.go:       specVersion   = "1.8.0"
pkg/csi/manila/driver.go:       klog.Info("CSI spec version: ", specVersion)
pkg/csi/cinder/driver.go:       specVersion = "1.8.0"
pkg/csi/cinder/driver.go:       klog.Info("CSI Spec version: ", specVersion)
$ grep -r 1.8.0 docs
docs/cinder-csi-plugin/using-cinder-csi-plugin.md:This plugin is compatible with CSI v1.8.0

I'm not sure if this is somehow relevant...

@jichenjc
Copy link
Contributor Author

I'm not sure if this is somehow relevant...

need check more on the spec itself ,e.g whether we really comply with the latest spec version
so I Think it's a different topic and can be a TODO item later @kayrus

@kayrus
Copy link
Contributor

kayrus commented Dec 12, 2023

@jichenjc let's skip the spec topic for a while, since this variable is only informational.

@jichenjc
Copy link
Contributor Author

@kayrus ok, please help lgtm this if ok to you, thanks

@kayrus
Copy link
Contributor

kayrus commented Dec 13, 2023

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 13, 2023
@jichenjc
Copy link
Contributor Author

@zetaab help to approve this ?

@dulek
Copy link
Contributor

dulek commented Dec 14, 2023

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dulek

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 Dec 14, 2023
Copy link
Member

@zetaab zetaab left a comment

Choose a reason for hiding this comment

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

/lgtm

@jichenjc
Copy link
Contributor Author

um... why the merge is not auto-complete? curious

@zetaab
Copy link
Member

zetaab commented Dec 28, 2023

/retest

@zetaab zetaab merged commit ee06cc1 into kubernetes:master Dec 28, 2023
5 of 6 checks passed
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. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants