-
Notifications
You must be signed in to change notification settings - Fork 100
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
SC Gardener aws-ebs-csi-driver image update #1080
SC Gardener aws-ebs-csi-driver image update #1080
Conversation
@scs-gardener-team-gitlab Thank you for your contribution. |
Thank you @scs-gardener-team-gitlab for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below. |
Additional Links:
|
The PR does not really enable the XFS option though does it ? From a user's perspective, how can he enable the new Helm chart option if it's not exposed via the shoot spec (e.g. controlplaneConfig) |
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.
See comment above
lgtm. Let's merge this PR first though. |
Hi @kon-angelo and @hebelsan , Are we ready to proceed with merging this PR? |
if value, ok := cluster.Shoot.Annotations[aws.legacyXFS]; ok { | ||
csiDriverNodeValues["driver"] = map[string]interface{}{ | ||
"legacyXFS": value, | ||
} | ||
} | ||
|
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.
With the code as it is, you would overwrite the value of VolumeAttachLimit
in the driver
key of the map. You should only create the map if it's not there.
@@ -48,6 +48,9 @@ spec: | |||
{{- if .Values.driver.volumeAttachLimit }} | |||
- --volume-attach-limit={{ .Values.driver.volumeAttachLimit }} | |||
{{- end }} | |||
{{- if .Values.driver.legacyXFS }} | |||
- --legacyXFS={{ .Values.driver.legacyXFS }} |
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.
Also this flag is not even correct. https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/7fd319a43ee6fe339517051226c506304d3f5f94/pkg/driver/options.go#L120 is the proper format.
eb405d7
to
79baeb7
Compare
How to categorize this PR?
/area storage
/area control-plane
/area delivery
/kind bug
/platform aws
What this PR does / why we need it:
SC Gardener landscapes require the aws csi-driver to run in legacyXFS mode. To do this the parameter needs to be configurable inside of the values.yaml file as well as added to the daemonset if aforementioned parameter is set to
True
.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Test manually. Paused the corresponding managed resource object, edited the csi-driver pod and its image for one of our cluster to use the
1.35.0
version. Then, edited the pod spec to include the extra legacyXFS patameter. The pod is running fine with no issues in legacyXFS mode.Release note: