Merge branch 'master' into feature-issue-651-immediate-volume-binding…

…-mode

.github/workflows/e2e.yml

@@ -159,17 +159,22 @@ jobs:
     - name: Make test
       run: make test-short-ci CSI_VERSION=$CSI_VERSION OPERATOR_VERSION=$CSI_OPERATOR_VERSION CHARTS_DIR=${{env.CSI_BAREMETAL_OPERATOR_DIR}}/charts
 
-    - name: Upload report to artifacts
-      uses: actions/upload-artifact@v2.2.1
+    - name: Publish Unit Test Results
+      uses: EnricoMi/publish-unit-test-result-action@v1
       with:
-        name: report.xml
-        path: test/e2e/report.xml
-
-    - name: Upload log to artifacts
+        files: test/e2e/reports/report.xml
+
+    - name: Archive artifacts
+      uses: papeloto/action-zip@v1
+      with:
+        files: test/e2e/reports/
+        dest: reports.zip
+
+    - name: Upload report to artifacts
       uses: actions/upload-artifact@v2.2.1
       with:
-        name: log.txt
-        path: log.txt
+        name: e2e_artifacts
+        path: reports.zip
 
   result_job:
     needs: e2e

cmd/node/main.go

@@ -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"
@@ -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"
@@ -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()
 
@@ -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)
@@ -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
+}

docs/custom-wbt-settings.md

@@ -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

pkg/eventing/eventing.go

@@ -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,
+	}
 )

pkg/mocks/executors.go

@@ -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)
 }
 

pkg/mocks/linuxutils/wbt.go

@@ -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)
+}

pkg/node/common_test_data.go

@@ -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},

pkg/node/node.go

@@ -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.
@@ -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 {
@@ -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)