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

STOR-1593: Rebase external-snapshotter to v8.0.1 for OCP 4.17 #156

Merged
merged 111 commits into from
Aug 7, 2024

Conversation

dfajmon
Copy link

@dfajmon dfajmon commented Jun 25, 2024

Diff to upstream 8.0.1:
kubernetes-csi/external-snapshotter@v8.0.1...dfajmon:rebase-8.0.1

Changes between 7.0.1 (OCP 4.16) and 8.0.1 (OCP 4.17):

Full changelogs:
https://github.com/kubernetes-csi/external-snapshotter/releases/tag/v8.0.1

@openshift/storage

xing-yang and others added 30 commits February 9, 2024 12:53
…econversion

Add nil check for SourceVolumeMode in the validation webhook
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.61.0 to 1.61.1.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.61.0...v1.61.1)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Currently the RestoreSize is missing when
the corresponding VSC are created for the
the volumesnapshotgroup, This PR adds the missing
RestoreSize for VSC.

Signed-off-by: Madhu Rajanna <[email protected]>
…/go_modules/google.golang.org/grpc-1.61.1

Bump google.golang.org/grpc from 1.61.0 to 1.61.1
Add snapshot size to VSC created for VGS
updating code to use correct method
names for logging.

Signed-off-by: Madhu Rajanna <[email protected]>
use GroupSnapshotDeleteError as event type for
event incase if there is any failure in delete
operation.

Signed-off-by: Madhu Rajanna <[email protected]>
Fix logging in the volumegroupsnapshot methods
The `IsDefaultAnnotation()` function has been extended to check for the
correct default annotation, taking the Kind of the object into
consideration.
Fix detecting default annotation on VolumeGroupSnapshotClass
dc4d0ae Merge pull request kubernetes-csi#249 from jsafrane/use-go-version
e681b17 Use .go-version to get Kubernetes go version

git-subtree-dir: release-tools
git-subtree-split: dc4d0ae
…go-update

Update release tools with go version fix
Bumps [github.com/prometheus/client_model](https://github.com/prometheus/client_model) from 0.5.0 to 0.6.0.
- [Release notes](https://github.com/prometheus/client_model/releases)
- [Commits](prometheus/client_model@v0.5.0...v0.6.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_model
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…/go_modules/github.com/prometheus/client_model-0.6.0

Bump github.com/prometheus/client_model from 0.5.0 to 0.6.0
Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.18.0 to 1.19.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/v1.19.0/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.18.0...v1.19.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…-rpc-spam

Discard unnecessary VolumeSnapshotContent updates to prevent rapid RPC calls
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.61.1 to 1.62.1.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.61.1...v1.62.1)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…/go_modules/google.golang.org/grpc-1.62.1

Bump google.golang.org/grpc from 1.61.1 to 1.62.1
…/go_modules/github.com/prometheus/client_golang-1.19.0

Bump github.com/prometheus/client_golang from 1.18.0 to 1.19.0
…igin-master

CVE-2024-24786: bump google.golang.org/protobuf to v1.33.0
Bumps [github.com/prometheus/common](https://github.com/prometheus/common) from 0.48.0 to 0.51.1.
- [Release notes](https://github.com/prometheus/common/releases)
- [Commits](prometheus/common@v0.48.0...v0.51.1)

---
updated-dependencies:
- dependency-name: github.com/prometheus/common
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Add snapshot secret reference to group snapshot controller.
…/go_modules/github.com/prometheus/common-0.51.1

Bump github.com/prometheus/common from 0.48.0 to 0.51.1
Update controller deployment to latest version
@openshift-ci-robot
Copy link

openshift-ci-robot commented Jul 8, 2024

@dfajmon: This pull request references STOR-1593 which is a valid jira issue.

In response to this:

Diff to upstream 8.0.1:
kubernetes-csi/external-snapshotter@v8.0.1...dfajmon:rebase-8.0.1

Changes between 7.0.1 (OCP 4.16) and 8.0.1 (OCP 4.17):

Full changelogs:
https://github.com/kubernetes-csi/external-snapshotter/releases/tag/v8.0.1

@openshift/storage

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2024
@dfajmon
Copy link
Author

dfajmon commented Jul 11, 2024

/retest

1 similar comment
@dfajmon
Copy link
Author

dfajmon commented Jul 12, 2024

/retest

@duanwei33
Copy link

Need to take a look on the snapshot failed cases due to:

{ fail [k8s.io/[email protected]/test/e2e/storage/framework/snapshot_resource.go:75]: admission webhook "volumesnapshotclasses.snapshot.storage.k8s.io" denied the request: expect resource to be {snapshot.storage.k8s.io v1 volumesnapshotclasses}, but found {snapshot.storage.k8s.io v1 volumesnapshots}

@duanwei33
Copy link

The error comes from csi-snapshot-webhook, not sure why it admitting volumesnapshotclasses when creating the volumesnapshot.

I0716 00:31:13.424783 1 webhook.go:131] handling request: {"kind":"AdmissionReview","apiVersion":"admission.k8s.io/v1","request":{"uid":"e4f2d61a-1a44-44a6-8503-ffe0c754964d","kind":{"group":"snapshot.storage.k8s.io","version":"v1","kind":"VolumeSnapshot"},"resource":{"group":"snapshot.storage.k8s.io","version":"v1","resource":"volumesnapshots"},"requestKind":{"group":"snapshot.storage.k8s.io","version":"v1","kind":"VolumeSnapshot"},"requestResource":{"group":"snapshot.storage.k8s.io","version":"v1","resource":"volumesnapshots"},"name":"mysnapshot","namespace":"wduan-snapshot","operation":"CREATE","userInfo":{"username":"system:admin","groups":["system:masters","system:authenticated"]},"object":{"apiVersion":"snapshot.storage.k8s.io/v1","kind":"VolumeSnapshot","metadata":{"creationTimestamp":"2024-07-16T00:31:13Z","generation":1,"managedFields":[{"apiVersion":"snapshot.storage.k8s.io/v1","fieldsType":"FieldsV1","fieldsV1":{"f:spec":{".":{},"f:source":{".":{},"f:persistentVolumeClaimName":{}},"f:volumeSnapshotClassName":{}}},"manager":"kubectl-create","operation":"Update","time":"2024-07-16T00:31:13Z"}],"name":"mysnapshot","namespace":"wduan-snapshot","uid":"d2851159-b30e-450a-ab51-7f74301334f7"},"spec":{"source":{"persistentVolumeClaimName":"mypvc-ori"},"volumeSnapshotClassName":"csi-aws-vsc"}},"oldObject":null,"dryRun":false,"options":{"kind":"CreateOptions","apiVersion":"meta.k8s.io/v1","fieldManager":"kubectl-create","fieldValidation":"Ignore"}}}
I0716 00:31:13.424888 1 snapshot.go:52] admitting volumesnapshotclasses
E0716 00:31:13.424902 1 snapshot.go:84] expect resource to be {snapshot.storage.k8s.io v1 volumesnapshotclasses}, but found {snapshot.storage.k8s.io v1 volumesnapshots}
I0716 00:31:13.424936 1 webhook.go:177] sending response: &AdmissionReview{Request:nil,Response:&AdmissionResponse{UID:e4f2d61a-1a44-44a6-8503-ffe0c754964d,Allowed:false,Result:&v1.Status{ListMeta:ListMeta{SelfLink:,ResourceVersion:,Continue:,RemainingItemCount:nil,},Status:,Message:expect resource to be {snapshot.storage.k8s.io v1 volumesnapshotclasses}, but found {snapshot.storage.k8s.io v1 volumesnapshots},Reason:,Details:nil,Code:0,},Patch:nil,PatchType:nil,AuditAnnotations:map[string]string{},Warnings:[],},}

@jsafrane
Copy link

The release note say:

Urgent Upgrade Notes

(No, really, you MUST read this before you upgrade)

The validating logic for VolumeSnapshots, VolumeSnapshotContents, VolumeGroupSnapshots, and VolumeGroupSnapshotContents has been replaced by CEL validation rules. The validating webhook is now only being used for VolumeSnapshotClasses and VolumeGroupSnapshotClasses to ensure that there's at most one class per CSI Driver. The validation webhook is deprecated and will be removed in the next release. (kubernetes-csi#1091, @leonardoce)

So, we need to rework the cluster-csi-snapshot-controller-operator a bit:

  1. We need to update snapshot CRD definitions there with the new CRDs from external-snapshotter.
  2. We need to update the webhook config there to validate only VolumeSnapshotClasses and VolumeGroupSnapshotClasses.

@jsafrane
Copy link

/retest

@jsafrane
Copy link

/retest
expecting the new snapshot-operator to be there...

@dfajmon
Copy link
Author

dfajmon commented Jul 31, 2024

/retest

@jsafrane
Copy link

/lgtm
/approve
/label px-approved
/label docs-approved

@openshift-ci openshift-ci bot added px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Jul 31, 2024
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 31, 2024
Copy link

openshift-ci bot commented Jul 31, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dfajmon, jsafrane

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 31, 2024
@jsafrane
Copy link

@duanwei33 upstream has reworked the webhook - some validation is done by the API server directly, without the webhook. In general, users should see no difference.

@duanwei33
Copy link

Snapshot cases look good in CI.
We might need to update some validation test in QE's Polarion, we can do that later.

/label qe-approved

@openshift-ci openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Aug 7, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 7, 2024

@dfajmon: This pull request references STOR-1593 which is a valid jira issue.

In response to this:

Diff to upstream 8.0.1:
kubernetes-csi/external-snapshotter@v8.0.1...dfajmon:rebase-8.0.1

Changes between 7.0.1 (OCP 4.16) and 8.0.1 (OCP 4.17):

Full changelogs:
https://github.com/kubernetes-csi/external-snapshotter/releases/tag/v8.0.1

@openshift/storage

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Copy link

openshift-ci bot commented Aug 7, 2024

@dfajmon: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit aa558ca into openshift:master Aug 7, 2024
9 checks passed
@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: csi-snapshot-validation-webhook
This PR has been included in build ose-csi-snapshot-validation-webhook-container-v4.18.0-202408071445.p0.gaa558ca.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-csi-external-snapshotter
This PR has been included in build ose-csi-external-snapshotter-container-v4.18.0-202408071445.p0.gaa558ca.assembly.stream.el9.
All builds following this will include this PR.

@openshift-bot
Copy link

[ART PR BUILD NOTIFIER]

Distgit: ose-csi-snapshot-controller
This PR has been included in build ose-csi-snapshot-controller-container-v4.18.0-202408071445.p0.gaa558ca.assembly.stream.el9.
All builds following this will include this PR.

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. docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR qe-approved Signifies that QE has signed off on this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ProbeForever has 1 second delay