-
Notifications
You must be signed in to change notification settings - Fork 880
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
fix: missing array type in the CRD rollout's spec volumes #1737
Conversation
78009a2
to
cd0030d
Compare
Codecov Report
@@ Coverage Diff @@
## master #1737 +/- ##
=======================================
Coverage 82.02% 82.02%
=======================================
Files 116 116
Lines 16096 16096
=======================================
Hits 13203 13203
Misses 2218 2218
Partials 675 675 Continue to review full report at Codecov.
|
@huikang thanks for fixing this. Do you think we could modify an e2e test to have a volume so that we catch this in the future? |
Sure, let me look into this and get back to you later. |
@jessesuen , checking that the volumes field is an array seems a feature starting from k8s release 1.23.1 (which could put more strict check on the crd fields). For k8s version v1.21.1, v1.22.1, the original
Since the existing git CI of this repo defaults to v1.22.1, one solution would be to upgrade the version to v1.23.1 argo-rollouts/.github/workflows/e2e.yaml Lines 25 to 32 in c291d2d
and then add What do you think? |
Signed-off-by: Hui Kang <[email protected]> Signed-off-by: Hui Kang <[email protected]>
Signed-off-by: Hui Kang <[email protected]>
0d7833f
to
b6b23b6
Compare
Signed-off-by: Hui Kang <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add e2e test as a separate PR which conditionally skips depending on the k8s version it is running against.
Signed-off-by: Hui Kang [email protected]
fix #1721
Checklist:
"fix(controller): Updates such and such. Fixes #1234"
.