From ec11ff68f2a6680602ac23094fa3cd9fd89984a5 Mon Sep 17 00:00:00 2001 From: Drew Sirenko <68304519+AndrewSirenko@users.noreply.github.com> Date: Wed, 4 Sep 2024 10:41:26 -0400 Subject: [PATCH] Add LegacyXFS parameter to support Linux <= v5.4 --- .../templates/_node-windows.tpl | 3 ++ charts/aws-ebs-csi-driver/templates/_node.tpl | 3 ++ charts/aws-ebs-csi-driver/values.yaml | 4 ++ docs/options.md | 35 +++++++------- pkg/driver/node.go | 3 ++ pkg/driver/node_test.go | 48 +++++++++++++++++++ pkg/driver/options.go | 4 ++ pkg/driver/options_test.go | 6 +++ 8 files changed, 89 insertions(+), 17 deletions(-) diff --git a/charts/aws-ebs-csi-driver/templates/_node-windows.tpl b/charts/aws-ebs-csi-driver/templates/_node-windows.tpl index 76ab90fd16..ee758cb7f9 100644 --- a/charts/aws-ebs-csi-driver/templates/_node-windows.tpl +++ b/charts/aws-ebs-csi-driver/templates/_node-windows.tpl @@ -71,6 +71,9 @@ spec: {{- with .Values.node.volumeAttachLimit }} - --volume-attach-limit={{ . }} {{- end }} + {{- if .Values.node.legacyXFS }} + - --legacy-xfs=true + {{- end}} {{- with .Values.node.loggingFormat }} - --logging-format={{ . }} {{- end }} diff --git a/charts/aws-ebs-csi-driver/templates/_node.tpl b/charts/aws-ebs-csi-driver/templates/_node.tpl index c0c4814332..a91b1b5eb0 100644 --- a/charts/aws-ebs-csi-driver/templates/_node.tpl +++ b/charts/aws-ebs-csi-driver/templates/_node.tpl @@ -74,6 +74,9 @@ spec: {{- with .Values.node.volumeAttachLimit }} - --volume-attach-limit={{ . }} {{- end }} + {{- if .Values.node.legacyXFS }} + - --legacy-xfs=true + {{- end}} {{- with .Values.node.loggingFormat }} - --logging-format={{ . }} {{- end }} diff --git a/charts/aws-ebs-csi-driver/values.yaml b/charts/aws-ebs-csi-driver/values.yaml index 75a445742e..dcde3d7a2e 100644 --- a/charts/aws-ebs-csi-driver/values.yaml +++ b/charts/aws-ebs-csi-driver/values.yaml @@ -388,6 +388,10 @@ node: # Enable the linux daemonset creation enableLinux: true enableWindows: false + # Warning: This option will be deprecated in a future version of EBS CSI Driver. + # Formats XFS volumes with bigtime=0,inobtcount=0,reflink=0, for mounting onto nodes with linux kernel ≤ v5.4. + # Note that XFS volumes formatted with this option will only have timestamp records until 2038. + legacyXFS: false # The number of attachment slots to reserve for system use (and not to be used for CSI volumes) # When this parameter is not specified (or set to -1), the EBS CSI Driver will attempt to determine the number of reserved slots via heuristic # Cannot be specified at the same time as `node.volumeAttachLimit` diff --git a/docs/options.md b/docs/options.md index 05951f2ab5..d0e0db7cd8 100644 --- a/docs/options.md +++ b/docs/options.md @@ -2,20 +2,21 @@ There are a couple of driver options that can be passed as arguments when starting the driver container. -| Option argument | value sample | default | Description | -|-----------------------------|---------------------------------------------------|-----------------------------------------------------|---------------------| -| endpoint | tcp://127.0.0.1:10000/ | unix:///var/lib/csi/sockets/pluginproxy/csi.sock | The socket on which the driver will listen for CSI RPCs| -| http-endpoint | :8080 | | The TCP network address where the HTTP server for metrics will listen (example: `:8080`). The default is empty string, which means the server is disabled.| -| metrics-cert-file | /metrics.crt | | The path to a certificate to use for serving the metrics server over HTTPS. If the certificate is signed by a certificate authority, this file should be the concatenation of the server's certificate, any intermediates, and the CA's certificate. If this is non-empty, `--http-endpoint` and `--metrics-key-file` MUST also be non-empty.| -| metrics-key-file | /metrics.key | | The path to a key to use for serving the metrics server over HTTPS. If this is non-empty, `--http-endpoint` and `--metrics-cert-file` MUST also be non-empty.| -| volume-attach-limit | 1,2,3 ... | -1 | Value for the maximum number of volumes attachable per node. If specified, the limit applies to all nodes. If not specified, the value is approximated from the instance type| -| extra-tags | key1=value1,key2=value2 | | Tags attached to each dynamically provisioned resource| -| k8s-tag-cluster-id | aws-cluster-id-1 | | ID of the Kubernetes cluster used for tagging provisioned EBS volumes| -| aws-sdk-debug-log | true | false | If set to true, the driver will enable the aws sdk debug log level| -| logging-format | json | text | Sets the log format. Permitted formats: text, json| -| user-agent-extra | csi-ebs | helm | Extra string appended to user agent| -| enable-otel-tracing | true | false | If set to true, the driver will enable opentelemetry tracing. Might need [additional env variables](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration) to export the traces to the right collector| -| batching | true | true | If set to true, the driver will enable batching of API calls. This is especially helpful for improving performance in workloads that are sensitive to EC2 rate limits at the cost of a small increase to worst-case latency| -| modify-volume-request-handler-timeout | 10s | 2s | Timeout for the window in which volume modification calls must be received in order for them to coalesce into a single volume modification call to AWS. If changing this, be aware that the ebs-csi-controller's csi-resizer and volumemodifier containers both have timeouts on the calls they make, if this value exceeds those timeouts it will cause them to always fail and fall into a retry loop, so adjust those values accordingly. -| warn-on-invalid-tag | true | false | To warn on invalid tags, instead of returning an error| -|reserved-volume-attachments | 2 | -1 | Number of volume attachments reserved for system use. Not used when --volume-attach-limit is specified. When -1, the amount of reserved attachments is loaded from instance metadata that captured state at node boot and may include not only system disks but also CSI volumes.| +| Option argument | value sample | default | Description | +|---------------------------|-------------------------|--------------------------------------------------|---------------------| +| endpoint | tcp://127.0.0.1:10000/ | unix:///var/lib/csi/sockets/pluginproxy/csi.sock | The socket on which the driver will listen for CSI RPCs| +| http-endpoint | :8080 | | The TCP network address where the HTTP server for metrics will listen (example: `:8080`). The default is empty string, which means the server is disabled.| +| metrics-cert-file | /metrics.crt | | The path to a certificate to use for serving the metrics server over HTTPS. If the certificate is signed by a certificate authority, this file should be the concatenation of the server's certificate, any intermediates, and the CA's certificate. If this is non-empty, `--http-endpoint` and `--metrics-key-file` MUST also be non-empty.| +| metrics-key-file | /metrics.key | | The path to a key to use for serving the metrics server over HTTPS. If this is non-empty, `--http-endpoint` and `--metrics-cert-file` MUST also be non-empty.| +| volume-attach-limit | 1,2,3 ... | -1 | Value for the maximum number of volumes attachable per node. If specified, the limit applies to all nodes. If not specified, the value is approximated from the instance type| +| extra-tags | key1=value1,key2=value2 | | Tags attached to each dynamically provisioned resource| +| k8s-tag-cluster-id | aws-cluster-id-1 | | ID of the Kubernetes cluster used for tagging provisioned EBS volumes| +| aws-sdk-debug-log | true | false | If set to true, the driver will enable the aws sdk debug log level| +| logging-format | json | text | Sets the log format. Permitted formats: text, json| +| user-agent-extra | csi-ebs | helm | Extra string appended to user agent| +| enable-otel-tracing | true | false | If set to true, the driver will enable opentelemetry tracing. Might need [additional env variables](https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#general-sdk-configuration) to export the traces to the right collector| +| batching | true | true | If set to true, the driver will enable batching of API calls. This is especially helpful for improving performance in workloads that are sensitive to EC2 rate limits at the cost of a small increase to worst-case latency| +| modify-volume-request-handler-timeout | 10s | 2s | Timeout for the window in which volume modification calls must be received in order for them to coalesce into a single volume modification call to AWS. If changing this, be aware that the ebs-csi-controller's csi-resizer and volumemodifier containers both have timeouts on the calls they make, if this value exceeds those timeouts it will cause them to always fail and fall into a retry loop, so adjust those values accordingly. +| warn-on-invalid-tag | true | false | To warn on invalid tags, instead of returning an error| +| reserved-volume-attachments | 2 | -1 | Number of volume attachments reserved for system use. Not used when --volume-attach-limit is specified. When -1, the amount of reserved attachments is loaded from instance metadata that captured state at node boot and may include not only system disks but also CSI volumes.| +| legacy-xfs | true | false | Warning: This option will be deprecated in a future version of EBS CSI Driver. Formats XFS volumes with `bigtime=0,inobtcount=0,reflink=0`, so that they can be mounted onto nodes with linux kernel ≤ v5.4. Note that XFS volumes formatted with this option will only have timestamp records until 2038.| diff --git a/pkg/driver/node.go b/pkg/driver/node.go index ca63441810..0bb7398aeb 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -274,6 +274,9 @@ func (d *NodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol if len(ext4ClusterSize) > 0 { formatOptions = append(formatOptions, "-C", ext4ClusterSize) } + if fsType == FSTypeXfs && d.options.LegacyXFSProgs { + formatOptions = append(formatOptions, "-m", "bigtime=0,inobtcount=0,reflink=0") + } err = d.mounter.FormatAndMountSensitiveWithFormatOptions(source, target, fsType, mountOptions, nil, formatOptions) if err != nil { msg := fmt.Sprintf("could not format %q and mount it at %q: %v", source, target, err) diff --git a/pkg/driver/node_test.go b/pkg/driver/node_test.go index d7536e3376..05eca704fc 100644 --- a/pkg/driver/node_test.go +++ b/pkg/driver/node_test.go @@ -84,6 +84,7 @@ func TestNodeStageVolume(t *testing.T) { req *csi.NodeStageVolumeRequest mounterMock func(ctrl *gomock.Controller) *mounter.MockMounter metadataMock func(ctrl *gomock.Controller) *metadata.MockMetadataService + options *Options expectedErr error inflight bool }{ @@ -966,6 +967,45 @@ func TestNodeStageVolume(t *testing.T) { }, expectedErr: nil, }, + { + name: "format_options_xfs_legacy", + req: &csi.NodeStageVolumeRequest{ + VolumeId: "vol-test", + StagingTargetPath: "/staging/path", + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{ + FsType: "xfs", + }, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + VolumeContext: map[string]string{ + InodeSizeKey: "512", + }, + PublishContext: map[string]string{ + DevicePathKey: "/dev/xvdba", + }, + }, + mounterMock: func(ctrl *gomock.Controller) *mounter.MockMounter { + m := mounter.NewMockMounter(ctrl) + m.EXPECT().FindDevicePath(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("/dev/xvdba", nil) + m.EXPECT().PathExists(gomock.Eq("/staging/path")).Return(true, nil) + m.EXPECT().GetDeviceNameFromMount(gomock.Eq("/staging/path")).Return("", 1, nil) + m.EXPECT().FormatAndMountSensitiveWithFormatOptions(gomock.Eq("/dev/xvdba"), gomock.Eq("/staging/path"), gomock.Eq("xfs"), gomock.Any(), gomock.Any(), gomock.Eq([]string{"-i", "size=512", "-m", "bigtime=0,inobtcount=0,reflink=0"})).Return(nil) + m.EXPECT().NeedResize(gomock.Eq("/dev/xvdba"), gomock.Eq("/staging/path")).Return(false, nil) + return m + }, + metadataMock: func(ctrl *gomock.Controller) *metadata.MockMetadataService { + m := metadata.NewMockMetadataService(ctrl) + m.EXPECT().GetRegion().Return("us-west-2") + return m + }, + options: &Options{LegacyXFSProgs: true}, + expectedErr: nil, + }, } for _, tc := range testCases { @@ -983,9 +1023,17 @@ func TestNodeStageVolume(t *testing.T) { metadata = tc.metadataMock(ctrl) } + var options *Options + if tc.options != nil { + options = tc.options + } else { + options = &Options{} // Initialize struct to avoid nil pointer dereference + } + driver := &NodeService{ metadata: metadata, mounter: mounter, + options: options, inFlight: internal.NewInFlight(), } diff --git a/pkg/driver/options.go b/pkg/driver/options.go index 5dc13c4b43..256f566b7d 100644 --- a/pkg/driver/options.go +++ b/pkg/driver/options.go @@ -87,6 +87,9 @@ type Options struct { ReservedVolumeAttachments int // ALPHA: WindowsHostProcess indicates whether the driver is running in a Windows privileged container WindowsHostProcess bool + // LegacyXFSProgs enables the use of `` when formatting a volume with XFS. This is necessary if the driver is used + // to mount XFS volumes on Linux Nodes with a kernel version ≥ 5.4 due to __. + LegacyXFSProgs bool } func (o *Options) AddFlags(f *flag.FlagSet) { @@ -115,6 +118,7 @@ func (o *Options) AddFlags(f *flag.FlagSet) { f.Int64Var(&o.VolumeAttachLimit, "volume-attach-limit", -1, "Value for the maximum number of volumes attachable per node. If specified, the limit applies to all nodes and overrides --reserved-volume-attachments. If not specified, the value is approximated from the instance type.") f.IntVar(&o.ReservedVolumeAttachments, "reserved-volume-attachments", -1, "Number of volume attachments reserved for system use. Not used when --volume-attach-limit is specified. The total amount of volume attachments for a node is computed as: - - . When -1, the amount of reserved attachments is loaded from instance metadata that captured state at node boot and may include not only system disks but also CSI volumes.") f.BoolVar(&o.WindowsHostProcess, "windows-host-process", false, "ALPHA: Indicates whether the driver is running in a Windows privileged container") + f.BoolVar(&o.LegacyXFSProgs, "legacy-xfs", false, "Warning: This option will be deprecated in a future version of EBS CSI Driver. Formats XFS volumes with `bigtime=0,inobtcount=0,reflink=0`, so that they can be mounted onto nodes with linux kernel ≤ v5.4. Note that XFS volumes formatted with this option will only have timestamp records until 2038.") } } diff --git a/pkg/driver/options_test.go b/pkg/driver/options_test.go index 21b4fe4a75..38944b7c9f 100644 --- a/pkg/driver/options_test.go +++ b/pkg/driver/options_test.go @@ -70,6 +70,9 @@ func TestAddFlags(t *testing.T) { if err := f.Set("reserved-volume-attachments", "5"); err != nil { t.Errorf("error setting reserved-volume-attachments: %v", err) } + if err := f.Set("legacy-xfs", "true"); err != nil { + t.Errorf("error setting legacy-xfs: %v", err) + } if o.Endpoint != "custom-endpoint" { t.Errorf("unexpected Endpoint: got %s, want custom-endpoint", o.Endpoint) @@ -107,6 +110,9 @@ func TestAddFlags(t *testing.T) { if o.ReservedVolumeAttachments != 5 { t.Errorf("unexpected ReservedVolumeAttachments: got %d, want 5", o.ReservedVolumeAttachments) } + if !o.LegacyXFSProgs { + t.Errorf("unexpected LegacyXFSProgs: got false, want true") + } } func TestValidateAttachmentLimits(t *testing.T) {