-
Notifications
You must be signed in to change notification settings - Fork 39k
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
e2e manifests #69868
e2e manifests #69868
Changes from 1 commit
1effe0c
dcbcef1
38efff5
a56c518
248df4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ spec: | |
serviceAccountName: csi-attacher | ||
containers: | ||
- name: csi-attacher | ||
image: quay.io/k8scsi/csi-attacher:v0.4.0 | ||
image: quay.io/k8scsi/csi-attacher:v0.4.1 | ||
args: | ||
- --v=5 | ||
- --csi-address=$(ADDRESS) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you want to simplify the hostpath specs too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but let's do that in a separate PR and then also coordinate the update with the original .yaml file in kubernetes-csi/docs. I've filed kubernetes-csi/docs#68 for this. |
||
|
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.
Hm I wonder if the alpha pd driver is going to have issues with the latest sidecars.
Also, unfortunately we haven't published public images of the beta gce pd driver yet. So I think it's best to just leave PD driver image versions the way it was before, and we will update the versions when we're ready.
Also I think we need to add the [Serial] tag to pd so that tests won't be run it parallel.
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.
+1
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.
The test has passed, so it looks like the alpha driver is compatible with the latest sidecar drivers. Do you want to revert that part nonetheless?
Please lets add the [Serial] tag as part of PR #68025 /cc @mkimuram
It has been working so far and this PR doesn't make it worse. From a practical perspective,
I don't see a good way to add it to the current test because of the way how it loops over the different drivers, and that part will be changed anyway:
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.
If its passing then it's fine to leave it. It's not a configuration that's officially been tested/supported by us, but I will update it later once we get the latest driver image published.
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.
@msau42 so is this PR ready for merging now?