Skip to content

Commit

Permalink
[ISSUE-635] Support WBT disabling (#638)
Browse files Browse the repository at this point in the history
Signed-off-by: safronovD <danil_safronov@dell.com>
  • Loading branch information
safronovD authored Dec 16, 2021
1 parent 2605732 commit 5294130
Show file tree
Hide file tree
Showing 15 changed files with 1,176 additions and 7 deletions.
23 changes: 23 additions & 0 deletions cmd/node/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"google.golang.org/grpc"
"google.golang.org/grpc/health/grpc_health_v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
Expand All @@ -52,6 +53,7 @@ import (
"github.com/dell/csi-baremetal/pkg/events"
"github.com/dell/csi-baremetal/pkg/metrics"
"github.com/dell/csi-baremetal/pkg/node"
"github.com/dell/csi-baremetal/pkg/node/wbt"
grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promhttp"
Expand Down Expand Up @@ -140,6 +142,11 @@ func main() {
logger.Fatalf("fail to prepare event recorder: %v", err)
}

wbtWatcher, err := prepareWbtWatcher(k8SClient, eventRecorder, *nodeName, logger)
if err != nil {
logger.Fatalf("fail to prepare wbt watcher: %v", err)
}

// Wait till all events are sent/handled
defer eventRecorder.Wait()

Expand Down Expand Up @@ -191,6 +198,9 @@ func main() {
// wait for readiness
waitForVolumeManagerReadiness(csiNodeService, logger)

// start to updating Wbt Config
wbtWatcher.StartWatch(csiNodeService)

logger.Info("Starting handle CSI calls ...")
if err := csiUDSServer.RunServer(); err != nil && err != grpc.ErrServerStopped {
logger.Fatalf("fail to serve: %v", err)
Expand Down Expand Up @@ -329,3 +339,16 @@ func prepareEventRecorder(nodeName string, logger *logrus.Logger) (*events.Recor
}
return eventRecorder, nil
}

func prepareWbtWatcher(client k8sClient.Client, eventsRecorder *events.Recorder, nodeName string, logger *logrus.Logger) (*wbt.ConfWatcher, error) {
k8sNode := &corev1.Node{}
err := client.Get(context.Background(), k8sClient.ObjectKey{Name: nodeName}, k8sNode)
if err != nil {
return nil, err
}

nodeKernel := k8sNode.Status.NodeInfo.KernelVersion
ll := logger.WithField("componentName", "WbtWatcher")

return wbt.NewConfWatcher(client, eventsRecorder, ll, nodeKernel), nil
}
74 changes: 74 additions & 0 deletions docs/custom-wbt-settings.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Proposal: Support option to change WBT settings

Last updated: 07.12.2021


## Abstract

CSI should have an opportunity to automate managing of WBT (writeback throttling) value for specific SCs.

## Background
WBT feature was introduced in 4.10 kernel and have a [bug in some versions](https://access.redhat.com/solutions/4904681).
Storage Systems, which use Baremetal CSI, may have performance issues due to incorrect WBT settings.
Recommended workaround in disabling this feature.
So it is specific for each device (sdb, sdc, ...), CSI should have an opportunity to control it on Volume or Pod creation stage.

## Proposal

#### Operator part

Add ConfigMap to manage WBT settings in csi-baremetal-deployment helm chart.
CSI Node will check it every 60 seconds and update existing one.

```
data:
config.yaml: |-
enable: true
# Value to set (uint32), 0 means disabling
wbt_lat_usec_value: 0
acceptable_volume_options:
# Block volumes don't take any impact from WBT
modes:
- FS
# It is risky to change WBT settings for LVG Volumes
storage_types:
- HDD
- SSD
- NVME
# The list of acceptable kernel versions
# Is some nodes are not in, user should add values manually via editting CM
acceptable_kernels.yaml: |-
node_kernel_versions:
- 4.18.0-193.65.2.el8_2.x86_64
```

#### Node Part

- Node Stage ->
1. Check WBT configuration
2. Scan current value (`cat /sys/block/<drive>/queue/wbt_lat_usec`)
3. Set passed one, if it's not equal (`echo <value> > /sys/block/<drive>/queue/wbt_lat_usec`)
4. Add `wbt-changed=yes` annotation on Volume
5. Send `WBTValueSetFailed` Event if error

- Node UnStage ->
1. Check `wbt-changed=yes` annotation on Volume
2. Restore default (`echo "-1" > /sys/block/<drive>/queue/wbt_lat_usec`) (It was performed even if unmount/removeDir errors)
3. Delete annotation
4. Send `WBTValueRestoreFailed` Event if error

## Rationale

We could trigger WBT disabling via PVC annotation as for fake-attach feature.

## Compatibility

WBT is supported only for 4.10 kernel version and above. Link - https://cateee.net/lkddb/web-lkddb/BLK_WBT.html

## Open issues (if applicable)

ID | Name | Descriptions | Status | Comments
---| -----| -------------| ------ | --------
ISSUE-1 | Should we enable WBT disabling by default for specific kernel version? | We could describe the kernel version map, where it is necessary | | The kernel version map is not clarified
ISSUE-2 | Is it correct to switch WBT settings on Stage/UnStage? | Other candidates - CreateVolume/DeleteVolume, Publish/UnPublish | RESOLVED | In Create/DeleteVolume case settings won't be persisted on node reboot. When volume is shared across the pods you will have to manipulate WBT multiple times on PublishVolume
16 changes: 16 additions & 0 deletions pkg/eventing/eventing.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,4 +193,20 @@ var (
severity: ErrorType,
symptomCode: NoneSymptomCode,
}

WBTValueSetFailed = &EventDescription{
reason: "WBTValueSetFailed",
severity: ErrorType,
symptomCode: NoneSymptomCode,
}
WBTValueRestoreFailed = &EventDescription{
reason: "WBTValueRestoreFailed",
severity: WarningType,
symptomCode: NoneSymptomCode,
}
WBTConfigMapUpdateFailed = &EventDescription{
reason: "WBTConfigMapUpdateFailed",
severity: WarningType,
symptomCode: NoneSymptomCode,
}
)
2 changes: 1 addition & 1 deletion pkg/mocks/executors.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (g *GoMockExecutor) RunCmdWithAttempts(cmd interface{}, attempts int, timeo

// RunCmd simulates execution of a command with OnCommand where user can set what the method should return
func (g *GoMockExecutor) RunCmd(cmd interface{}, opts ...command.Options) (string, string, error) {
args := g.Mock.Called(cmd.(string))
args := g.Mock.Called(cmd)
return args.String(0), args.String(1), args.Error(2)
}

Expand Down
38 changes: 38 additions & 0 deletions pkg/mocks/linuxutils/wbt.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
Copyright © 2020 Dell Inc. or its subsidiaries. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package linuxutils

import (
"github.com/stretchr/testify/mock"
)

// MockWrapWbt is a mock implementation of WrapWbt
type MockWrapWbt struct {
mock.Mock
}

// SetValue is a mock implementations
func (m *MockWrapWbt) SetValue(device string, value uint32) error {
args := m.Mock.Called(device, value)
return args.Error(0)
}

// RestoreDefault is a mock implementations
func (m *MockWrapWbt) RestoreDefault(device string) error {
args := m.Mock.Called(device)
return args.Error(0)
}
4 changes: 2 additions & 2 deletions pkg/node/common_test_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ var (

testLogger = getTestLogger()
testCtx = context.Background()
disk1 = api.Drive{UUID: uuid.New().String(), SerialNumber: "hdd1", Size: 1024 * 1024 * 1024 * 500, NodeId: nodeID}
disk2 = api.Drive{UUID: uuid.New().String(), SerialNumber: "hdd2", Size: 1024 * 1024 * 1024 * 200, NodeId: nodeID}
disk1 = api.Drive{UUID: uuid.New().String(), SerialNumber: "hdd1", Size: 1024 * 1024 * 1024 * 500, NodeId: nodeID, Path: "/dev/sda"}
disk2 = api.Drive{UUID: uuid.New().String(), SerialNumber: "hdd2", Size: 1024 * 1024 * 1024 * 200, NodeId: nodeID, Path: "/dev/sda"}
testAC1Name = fmt.Sprintf("%s-%s", nodeID, strings.ToLower(disk1.UUID))
testAC1 = accrd.AvailableCapacity{
TypeMeta: k8smetav1.TypeMeta{Kind: "AvailableCapacity", APIVersion: apiV1.APIV1Version},
Expand Down
22 changes: 22 additions & 0 deletions pkg/node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ const (

fakeAttachVolumeAnnotation = "fake-attach"
fakeAttachVolumeKey = "yes"

wbtChangedVolumeAnnotation = "wbt-changed"
wbtChangedVolumeKey = "yes"
)

// CSINodeService is the implementation of NodeServer interface from GO CSI specification.
Expand Down Expand Up @@ -238,6 +241,16 @@ func (s *CSINodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStage
"Fake-attach cleared for volume with ID %s", volumeID)
}

if newStatus == apiV1.VolumeReady && s.VolumeManager.checkWbtChangingEnable(ctx, volumeCR) {
if err := s.VolumeManager.setWbtValue(volumeCR); err != nil {
ll.Errorf("Unable to set custom WBT value for volume %s: %v", volumeCR.Name, err)
s.VolumeManager.recorder.Eventf(volumeCR, eventing.WBTValueSetFailed,
"Unable to set custom WBT value for volume %s", volumeCR.Name)
} else {
volumeCR.Annotations[wbtChangedVolumeAnnotation] = wbtChangedVolumeKey
}
}

if currStatus != apiV1.VolumeReady {
volumeCR.Spec.CSIStatus = newStatus
if err := s.k8sClient.UpdateCR(ctx, volumeCR); err != nil {
Expand Down Expand Up @@ -318,6 +331,15 @@ func (s *CSINodeService) NodeUnstageVolume(ctx context.Context, req *csi.NodeUns
}
}

if val, ok := volumeCR.Annotations[wbtChangedVolumeAnnotation]; ok && val == wbtChangedVolumeKey {
delete(volumeCR.Annotations, wbtChangedVolumeAnnotation)
if err := s.VolumeManager.restoreWbtValue(volumeCR); err != nil {
ll.Errorf("Unable to restore WBT value for volume %s: %v", volumeCR.Name, err)
s.VolumeManager.recorder.Eventf(volumeCR, eventing.WBTValueSetFailed,
"Unable to restore WBT value for volume %s", volumeCR.Name)
}
}

ctxWithID := context.WithValue(context.Background(), base.RequestUUID, req.GetVolumeId())
if updateErr := s.k8sClient.UpdateCR(ctxWithID, volumeCR); updateErr != nil {
ll.Errorf("Unable to update volume CR: %v", updateErr)
Expand Down
Loading

0 comments on commit 5294130

Please sign in to comment.