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.

Sign up for GitHub

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

Jump to bottom

Update dependency go modules for k8s v1.27.0-rc.0 and v1.27.0 #901

Merged
merged 5 commits into from
Apr 24, 2023
Merged

Update dependency go modules for k8s v1.27.0-rc.0 and v1.27.0 #901

merged 5 commits into from
Apr 24, 2023

Conversation

sunnylovestiramisu
Copy link
Contributor

@sunnylovestiramisu sunnylovestiramisu commented Mar 28, 2023 •
edited
Loading

Ran kubernetes-csi/csi-release-tools go-get-kubernetes.sh -p 1.27.0-rc.0
and
ran kubernetes-csi/csi-release-tools go-get-kubernetes.sh -p 1.27.0

Update go version to 1.20

@k8s-ci-robot k8s-ci-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. 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 28, 2023
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Mar 28, 2023
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 4, 2023
@sunnylovestiramisu sunnylovestiramisu force-pushed the module-update-master branch 2 times, most recently from 827e502 to b1fef17 Compare April 10, 2023 18:36
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2023
@sunnylovestiramisu sunnylovestiramisu force-pushed the module-update-master branch 3 times, most recently from b105fed to a129b37 Compare April 10, 2023 22:25
@sunnylovestiramisu
Copy link
Contributor Author

/test pull-kubernetes-csi-external-provisioner-unit

@sunnylovestiramisu sunnylovestiramisu force-pushed the module-update-master branch 2 times, most recently from 32cc9b9 to 3655e2d Compare April 11, 2023 00:29
@sunnylovestiramisu
Copy link
Contributor Author

This change seems related because it is setting feature flag for tests:

kubernetes/component-base@7443432

@sunnylovestiramisu
Copy link
Contributor Author

The test failed with:

Test Panicked
    In [AfterEach] at: /home/prow/go/pkg/csiprow.MTnBXrL9hy/go-1.19/src/runtime/panic.go:260
    runtime error: invalid memory address or nil pointer dereference
    Full Stack Trace
      k8s.io/kubernetes/test/e2e/storage/testsuites.(*volumePerformanceTestSuite).DefineTests.func2()
        test/e2e/storage/testsuites/volumeperf.go:133 +0x55

And then there is a change lately of the test setup: kubernetes/kubernetes@2f6c4f5

Not sure why this line is throwing nil pointer exception. @pohly Do you have any insight into this?

@msau42
Copy link
Collaborator

msau42 commented Apr 14, 2023 •
edited
Loading

Note that the e2e jobs will download Kubernetes at different release versions and build the e2e binaries. So PR above does not seem relevant

HEAD is now at b46a3f88 Release commit for Kubernetes v1.26.0
Thu Apr 13 23:14:07 UTC 2023 go1.20.3 /home/prow/go/src/k8s.io/kubernetes$ git clean -fdx
Thu Apr 13 23:14:08 UTC 2023 go1.19 $ make WHAT=test/e2e/e2e.test -C/home/prow/go/src/k8s.io/kubernetes

Looking at the test code, l is not initialized until the It clause: https://github.com/kubernetes/kubernetes/blob/v1.26.0/test/e2e/storage/testsuites/volumeperf.go#L147

So I believe if the test case exits in the BeforeEach, it could be possible that the AfterEach will run without l being initialized.

Now the question I cannot answer though is why updating k8s dependencies would trigger the error in the e2e tests which are not built from this repo.

@sunnylovestiramisu sunnylovestiramisu mentioned this pull request Apr 14, 2023
Merged
msau42
msau42 reviewed Apr 14, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
@msau42 msau42 mentioned this pull request Apr 14, 2023
Open
@sunnylovestiramisu
Squashed 'release-tools/' changes from 78c0fb714..6613c3980
37bfa0f 
6613c3980 Merge pull request kubernetes-csi#223 from sunnylovestiramisu/update
0e7ae993d Update k8s image repo url
77e47cce8 Merge pull request kubernetes-csi#222 from xinydev/fix-dep-version
155854b09 Fix dep version mismatch
8f839056a Merge pull request kubernetes-csi#221 from sunnylovestiramisu/go-update
1d3f94dd5 Update go version to 1.20 to match k/k v1.27
e322ce5e5 Merge pull request kubernetes-csi#220 from andyzhangx/fix-golint-error
b74a51209 test: fix golint error
aa61bfd0c Merge pull request kubernetes-csi#218 from xing-yang/update_csi_driver
7563d1963 Update CSI_PROW_DRIVER_VERSION to v1.11.0
a2171bef0 Merge pull request kubernetes-csi#216 from msau42/process
cb9878261 Merge pull request kubernetes-csi#217 from msau42/owners
a11216e47 add new reviewers and remove inactive reviewers
dd9867540 Add step for checking builds
b66c08249 Merge pull request kubernetes-csi#214 from pohly/junit-fixes
b9b6763bd filter-junit.go: fix loss of testcases when parsing Ginkgo v2 JUnit
d4277839f filter-junit.go: preserve system error log
38e11468f prow.sh: publish individual JUnit files as separate artifacts

git-subtree-dir: release-tools
git-subtree-split: 6613c3980d1e418bebb7bc49d64c977cfff85671
@sunnylovestiramisu
Merge commit '37bfa0fef405a9804acc43ba6512bb7d3ab85363' into module-u…
2b31965 
…pdate-master
msau42
msau42 reviewed Apr 14, 2023
View reviewed changes
go.mod Outdated
k8s.io/component-base v0.26.1
k8s.io/component-helpers v0.26.1
k8s.io/csi-translation-lib v0.26.3
k8s.io/api v0.27.0-rc.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we update to 0.27.0 now that it's available?

Copy link
Contributor Author

@sunnylovestiramisu sunnylovestiramisu Apr 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Actually let me just add a new commit to the same pr for that.

@sunnylovestiramisu sunnylovestiramisu changed the title Update dependency go modules for k8s v1.27.0-rc.0 Update dependency go modules for k8s v1.27.0-rc.0 and v1.27.0 Apr 17, 2023
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Apr 17, 2023
@gnufied
Copy link
Contributor

gnufied commented Apr 18, 2023

/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 Apr 18, 2023
xing-yang
xing-yang reviewed Apr 24, 2023
View reviewed changes
go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 24, 2023
@xing-yang
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 Apr 24, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gnufied, sunnylovestiramisu, xing-yang

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 Apr 24, 2023
@k8s-ci-robot k8s-ci-robot merged commit 17a538e into kubernetes-csi:master Apr 24, 2023
@mpatlasov mpatlasov mentioned this pull request May 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msau42 msau42 msau42 left review comments

@xing-yang xing-yang xing-yang left review comments

@andyzhangx andyzhangx Awaiting requested review from andyzhangx

@jingxu97 jingxu97 Awaiting requested review from jingxu97

Assignees

@gnufied gnufied

@xing-yang xing-yang

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 Denotes a PR that will be considered when it comes time to generate release notes. 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.
5 participants
@sunnylovestiramisu @msau42 @gnufied @xing-yang @k8s-ci-robot