From 6df58fc7b3a262d9c3a93d756ed8850b88818fe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Wanzenb=C3=B6ck?= Date: Wed, 24 Jan 2024 14:44:24 +0100 Subject: [PATCH 1/3] do not create a diskless resource without matching diskless flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The diskless flag is used to indicate the type of diskless resource LINSTOR should create. This is determined by the available layers in the resource. If there is no DRBD or NVMe layer, diskless attachment does not make sense, so we error out in that case. Signed-off-by: Moritz Wanzenböck --- CHANGELOG.md | 4 ++++ pkg/client/linstor.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cdbe36..6b465d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Do not try to create diskless resource if there is no compatible diskless layer (DRBD or NVMe) available + ## [1.3.0] - 2023-11-15 ### Added diff --git a/pkg/client/linstor.go b/pkg/client/linstor.go index 6a1de8b..1d7daf0 100644 --- a/pkg/client/linstor.go +++ b/pkg/client/linstor.go @@ -517,6 +517,10 @@ func (s *Linstor) Attach(ctx context.Context, volId, node string, rwxBlock bool) // are already covered in the allowed topology bits. s.log.WithError(err).Info("fall back to manual diskless creation after make-available refused") + if disklessFlag == "" { + return fmt.Errorf("resource does not support diskless attachment") + } + rCreate := lapi.ResourceCreate{Resource: lapi.Resource{ Name: volId, NodeName: node, From ab7b3db74c0707278caaf18ec9e03ab877776b98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Wanzenb=C3=B6ck?= Date: Wed, 24 Jan 2024 15:49:39 +0100 Subject: [PATCH 2/3] do not attach volume with no replicas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Moritz Wanzenböck --- CHANGELOG.md | 3 ++- pkg/client/linstor.go | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b465d7..89d2241 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Do not try to create diskless resource if there is no compatible diskless layer (DRBD or NVMe) available +- Do not try to create diskless resource if there is no compatible diskless layer (DRBD or NVMe) available. +- Do not allow attaching a volume that has no existing replica. ## [1.3.0] - 2023-11-15 diff --git a/pkg/client/linstor.go b/pkg/client/linstor.go index 1d7daf0..a91929c 100644 --- a/pkg/client/linstor.go +++ b/pkg/client/linstor.go @@ -469,10 +469,14 @@ func (s *Linstor) Attach(ctx context.Context, volId, node string, rwxBlock bool) }).Info("attaching volume") ress, err := s.client.Resources.GetAll(ctx, volId) - if nil404(err) != nil { + if err != nil { return err } + if len(ress) == 0 { + return fmt.Errorf("failed to attach resource with no deployed replica") + } + var existingRes *lapi.Resource disklessFlag := "" From 10a2f0a278051c7420dbef886a0194ab76305ef2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20Wanzenb=C3=B6ck?= Date: Wed, 24 Jan 2024 16:10:09 +0100 Subject: [PATCH 3/3] set default acces policy for resources without replication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit if a user creates a resource without replication, it does not make sense to access it from anywhere but the local node. So we set the default access policy accordingly. Signed-off-by: Moritz Wanzenböck --- CHANGELOG.md | 4 ++++ go.mod | 1 + go.sum | 2 ++ pkg/volume/parameter.go | 22 +++++++++++++++------- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89d2241..74f274b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Default access policy for resources without replication layer is now "local only". + ### Fixed - Do not try to create diskless resource if there is no compatible diskless layer (DRBD or NVMe) available. diff --git a/go.mod b/go.mod index 9196032..da50424 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/pborman/uuid v1.2.1 github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.8.4 + golang.org/x/exp v0.0.0-20240119083558-1b970713d09a golang.org/x/sys v0.16.0 golang.org/x/time v0.5.0 google.golang.org/grpc v1.60.1 diff --git a/go.sum b/go.sum index 0071377..4982629 100644 --- a/go.sum +++ b/go.sum @@ -122,6 +122,8 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.18.0 h1:PGVlW0xEltQnzFZ55hkuX5+KLyrMYhHld1YHO4AKcdc= golang.org/x/crypto v0.18.0/go.mod h1:R0j02AL6hcrfOiy9T4ZYp/rcWeMxM3L6QYxlOuEG1mg= +golang.org/x/exp v0.0.0-20240119083558-1b970713d09a h1:Q8/wZp0KX97QFTc2ywcOE0YRjZPVIx+MXInMzdvQqcA= +golang.org/x/exp v0.0.0-20240119083558-1b970713d09a/go.mod h1:idGWGoKP1toJGkd5/ig9ZLuPcZBC3ewk7SzmH0uou08= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= diff --git a/pkg/volume/parameter.go b/pkg/volume/parameter.go index a7933a5..9b68ab8 100644 --- a/pkg/volume/parameter.go +++ b/pkg/volume/parameter.go @@ -11,6 +11,7 @@ import ( "github.com/LINBIT/golinstor/devicelayerkind" "github.com/pborman/uuid" log "github.com/sirupsen/logrus" + "golang.org/x/exp/slices" "github.com/piraeusdatastore/linstor-csi/pkg/linstor" "github.com/piraeusdatastore/linstor-csi/pkg/topology" @@ -108,13 +109,12 @@ var DefaultRemoteAccessPolicy = RemoteAccessPolicyAnywhere func NewParameters(params map[string]string, topologyPrefix string) (Parameters, error) { // set zero values p := Parameters{ - LayerList: []devicelayerkind.DeviceLayerKind{devicelayerkind.Drbd, devicelayerkind.Storage}, - PlacementCount: 1, - DisklessStoragePool: DefaultDisklessStoragePoolName, - Encryption: false, - PlacementPolicy: topology.AutoPlaceTopology, - AllowRemoteVolumeAccess: DefaultRemoteAccessPolicy, - Properties: make(map[string]string), + LayerList: []devicelayerkind.DeviceLayerKind{devicelayerkind.Drbd, devicelayerkind.Storage}, + PlacementCount: 1, + DisklessStoragePool: DefaultDisklessStoragePoolName, + Encryption: false, + PlacementPolicy: topology.AutoPlaceTopology, + Properties: make(map[string]string), } for k, v := range params { @@ -260,6 +260,14 @@ func NewParameters(params map[string]string, topologyPrefix string) (Parameters, p.ResourceGroup = "sc-" + uuid.NewSHA1(namespace, encoded).String() } + if p.AllowRemoteVolumeAccess == nil { + if slices.Contains(p.LayerList, devicelayerkind.Drbd) || slices.Contains(p.LayerList, devicelayerkind.Nvme) { + p.AllowRemoteVolumeAccess = DefaultRemoteAccessPolicy + } else { + p.AllowRemoteVolumeAccess = RemoteAccessPolicyLocalOnly + } + } + // User has manually configured deployments, ignore autoplacing options. if len(p.NodeList)+len(p.ClientList) != 0 { p.PlacementCount = 0