Skip to content

Commit a1bd0b8

Browse files
clintonkclintonk
authored
Storage class selector override annotation
1 parent 61510ae commit a1bd0b8

File tree

14 files changed

+612
-136
lines changed

14 files changed

+612
-136
lines changed

‎core/orchestrator_core.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4950,6 +4950,50 @@ func (o *TridentOrchestrator) AddStorageClass(
49504950
return sc.ConstructExternal(ctx), nil
49514951
}
49524952

4953+
func (o *TridentOrchestrator) UpdateStorageClass(
4954+
ctx context.Context, scConfig *storageclass.Config,
4955+
) (scExternal *storageclass.External, err error) {
4956+
ctx = GenerateRequestContextForLayer(ctx, LogLayerCore)
4957+
4958+
if o.bootstrapError != nil {
4959+
return nil, o.bootstrapError
4960+
}
4961+
4962+
defer recordTiming("storageclass_update", &err)()
4963+
4964+
o.mutex.Lock()
4965+
defer o.mutex.Unlock()
4966+
defer o.updateMetrics()
4967+
4968+
newSC := storageclass.New(scConfig)
4969+
scName := newSC.GetName()
4970+
oldSC, ok := o.storageClasses[scName]
4971+
if !ok {
4972+
return nil, errors.NotFoundError("storage class %s not found", scName)
4973+
}
4974+
4975+
if err = o.storeClient.UpdateStorageClass(ctx, newSC); err != nil {
4976+
return nil, err
4977+
}
4978+
4979+
// Remove storage class from backend map
4980+
for _, storagePool := range oldSC.GetStoragePoolsForProtocol(ctx, config.ProtocolAny, config.ReadWriteOnce) {
4981+
storagePool.RemoveStorageClass(scName)
4982+
}
4983+
4984+
// Add storage class to backend map
4985+
added := 0
4986+
for _, backend := range o.backends {
4987+
added += newSC.CheckAndAddBackend(ctx, backend)
4988+
}
4989+
Logc(ctx).WithField("storageClass", scName).Infof("Storage class satisfied by %d storage pools.", added)
4990+
4991+
// Update internal cache
4992+
o.storageClasses[scName] = newSC
4993+
4994+
return newSC.ConstructExternal(ctx), nil
4995+
}
4996+
49534997
func (o *TridentOrchestrator) GetStorageClass(
49544998
ctx context.Context, scName string,
49554999
) (scExternal *storageclass.External, err error) {

‎core/orchestrator_core_test.go‎

Lines changed: 242 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2747,6 +2747,11 @@ func TestOrchestratorNotReady(t *testing.T) {
27472747
t.Errorf("Expected AddStorageClass to return an error.")
27482748
}
27492749

2750+
storageClass, err = orchestrator.UpdateStorageClass(ctx(), nil)
2751+
if storageClass != nil || !errors.IsNotReadyError(err) {
2752+
t.Errorf("Expected UpdateStorageClass to return an error.")
2753+
}
2754+
27502755
storageClass, err = orchestrator.GetStorageClass(ctx(), "")
27512756
if storageClass != nil || !errors.IsNotReadyError(err) {
27522757
t.Errorf("Expected GetStorageClass to return an error.")
@@ -4175,6 +4180,243 @@ func TestGetProtocol(t *testing.T) {
41754180
}
41764181
}
41774182

4183+
func TestAddStorageClass(t *testing.T) {
4184+
orchestrator := getOrchestrator(t, false)
4185+
defer cleanup(t, orchestrator)
4186+
4187+
storageClassConfig := &storageclass.Config{
4188+
Name: "test-storage-class",
4189+
Attributes: map[string]sa.Request{
4190+
sa.IOPS: sa.NewIntRequest(1000),
4191+
},
4192+
}
4193+
4194+
// First add should work
4195+
storageClassExt, err := orchestrator.AddStorageClass(ctx(), storageClassConfig)
4196+
if err != nil {
4197+
t.Errorf("Failed to add storage class: %v", err)
4198+
}
4199+
4200+
if _, ok := orchestrator.storageClasses[storageClassExt.GetName()]; !ok {
4201+
t.Errorf("Storage class %s not found in orchestrator", storageClassExt.GetName())
4202+
}
4203+
4204+
// Second add should fail
4205+
storageClassExt, err = orchestrator.AddStorageClass(ctx(), storageClassConfig)
4206+
4207+
assert.Nil(t, storageClassExt, "Value should be nil")
4208+
assert.Error(t, err, "Add should have failed")
4209+
}
4210+
4211+
func TestAddStorageClass_PersistentStoreError(t *testing.T) {
4212+
mockCtrl := gomock.NewController(t)
4213+
defer mockCtrl.Finish()
4214+
4215+
mockStoreClient := mockpersistentstore.NewMockStoreClient(mockCtrl)
4216+
orchestrator, err := NewTridentOrchestrator(mockStoreClient)
4217+
orchestrator.bootstrapped = true
4218+
orchestrator.bootstrapError = nil
4219+
4220+
mockStoreClient.EXPECT().AddBackend(gomock.Any(), gomock.Any()).Return(nil)
4221+
mockStoreClient.EXPECT().AddStorageClass(gomock.Any(), gomock.Any()).Return(errors.New("failed"))
4222+
4223+
// Add backend with two pools
4224+
mockPools := tu.GetFakePools()
4225+
pools := map[string]*fake.StoragePool{
4226+
"fast-small": mockPools[tu.FastSmall],
4227+
"slow-snapshots": mockPools[tu.SlowSnapshots],
4228+
}
4229+
cfg, err := fakedriver.NewFakeStorageDriverConfigJSON("backendWithTwoPools", config.File, pools, make([]fake.Volume, 0))
4230+
if err != nil {
4231+
t.Fatalf("Unable to generate cfg JSON: %v", err)
4232+
}
4233+
_, err = orchestrator.AddBackend(ctx(), cfg, "")
4234+
if err != nil {
4235+
t.Fatalf("Unable to add backend: %v", err)
4236+
}
4237+
4238+
// Add initial storage class
4239+
initialStorageClassConfig := &storageclass.Config{
4240+
Name: "initial-storage-class",
4241+
Attributes: map[string]sa.Request{
4242+
sa.IOPS: sa.NewIntRequest(2000),
4243+
},
4244+
}
4245+
_, err = orchestrator.AddStorageClass(ctx(), initialStorageClassConfig)
4246+
assert.Error(t, err, "AddStorageClass should have failed")
4247+
}
4248+
4249+
func TestUpdateStorageClass(t *testing.T) {
4250+
mockCtrl := gomock.NewController(t)
4251+
defer mockCtrl.Finish()
4252+
4253+
mockStoreClient := mockpersistentstore.NewMockStoreClient(mockCtrl)
4254+
orchestrator, err := NewTridentOrchestrator(mockStoreClient)
4255+
orchestrator.bootstrapError = nil
4256+
4257+
// Test 1: Update existing storage class
4258+
scConfig := &storageclass.Config{Name: "testSC"}
4259+
sc := storageclass.New(scConfig)
4260+
orchestrator.storageClasses[sc.GetName()] = sc
4261+
4262+
mockStoreClient.EXPECT().UpdateStorageClass(gomock.Any(), sc).Return(nil).Times(1)
4263+
_, err = orchestrator.UpdateStorageClass(ctx(), scConfig)
4264+
assert.NoError(t, err, "should not return an error when updating an existing storage class")
4265+
4266+
// Test 2: Update non-existing storage class
4267+
scConfig = &storageclass.Config{Name: "nonExistentSC"}
4268+
_, err = orchestrator.UpdateStorageClass(ctx(), scConfig)
4269+
assert.Error(t, err, "should return an error when updating a non-existing storage class")
4270+
4271+
// Test 3: Error updating storage class in store
4272+
scConfig = &storageclass.Config{Name: "testSC"}
4273+
sc = storageclass.New(scConfig)
4274+
orchestrator.storageClasses[sc.GetName()] = sc
4275+
4276+
mockStoreClient.EXPECT().UpdateStorageClass(gomock.Any(), sc).Return(fmt.Errorf("store error")).Times(1)
4277+
_, err = orchestrator.UpdateStorageClass(ctx(), scConfig)
4278+
assert.Error(t, err, "should return an error when store client fails to update storage class")
4279+
}
4280+
4281+
func TestUpdateStorageClassWithBackends(t *testing.T) {
4282+
// Ensure backend-to-storageclass mapping is updated
4283+
4284+
mockCtrl := gomock.NewController(t)
4285+
defer mockCtrl.Finish()
4286+
4287+
mockStoreClient := mockpersistentstore.NewMockStoreClient(mockCtrl)
4288+
orchestrator, err := NewTridentOrchestrator(mockStoreClient)
4289+
orchestrator.bootstrapped = true
4290+
orchestrator.bootstrapError = nil
4291+
4292+
mockStoreClient.EXPECT().AddBackend(gomock.Any(), gomock.Any()).Return(nil)
4293+
mockStoreClient.EXPECT().AddStorageClass(gomock.Any(), gomock.Any()).Return(nil)
4294+
mockStoreClient.EXPECT().UpdateStorageClass(gomock.Any(), gomock.Any()).Return(nil)
4295+
4296+
// Add backend with two pools
4297+
mockPools := tu.GetFakePools()
4298+
pools := map[string]*fake.StoragePool{
4299+
"fast-small": mockPools[tu.FastSmall],
4300+
"slow-snapshots": mockPools[tu.SlowSnapshots],
4301+
}
4302+
cfg, err := fakedriver.NewFakeStorageDriverConfigJSON("backendWithTwoPools", config.File, pools, make([]fake.Volume, 0))
4303+
if err != nil {
4304+
t.Fatalf("Unable to generate cfg JSON: %v", err)
4305+
}
4306+
_, err = orchestrator.AddBackend(ctx(), cfg, "")
4307+
if err != nil {
4308+
t.Fatalf("Unable to add backend: %v", err)
4309+
}
4310+
4311+
// Add initial storage class
4312+
initialStorageClassConfig := &storageclass.Config{
4313+
Name: "initial-storage-class",
4314+
Attributes: map[string]sa.Request{
4315+
sa.IOPS: sa.NewIntRequest(2000),
4316+
},
4317+
}
4318+
_, err = orchestrator.AddStorageClass(ctx(), initialStorageClassConfig)
4319+
if err != nil {
4320+
t.Fatalf("Unable to add initial storage class: %v", err)
4321+
}
4322+
4323+
// Validate initial storage class matches pool1
4324+
mockStoreClient.EXPECT().GetStorageClass(gomock.Any(), gomock.Any()).Return(
4325+
orchestrator.storageClasses["initial-storage-class"].ConstructPersistent(), nil)
4326+
4327+
validateStorageClass(t, orchestrator, initialStorageClassConfig.Name, []*tu.PoolMatch{
4328+
{Backend: "backendWithTwoPools", Pool: tu.FastSmall},
4329+
})
4330+
4331+
// Update storage class to match pool2
4332+
updatedStorageClassConfig := &storageclass.Config{
4333+
Name: "initial-storage-class",
4334+
Attributes: map[string]sa.Request{
4335+
sa.IOPS: sa.NewIntRequest(40),
4336+
},
4337+
}
4338+
_, err = orchestrator.UpdateStorageClass(ctx(), updatedStorageClassConfig)
4339+
if err != nil {
4340+
t.Fatalf("Unable to update storage class: %v", err)
4341+
}
4342+
4343+
// Validate updated storage class matches pool2
4344+
mockStoreClient.EXPECT().GetStorageClass(gomock.Any(), gomock.Any()).Return(
4345+
orchestrator.storageClasses["initial-storage-class"].ConstructPersistent(), nil)
4346+
4347+
validateStorageClass(t, orchestrator, updatedStorageClassConfig.Name, []*tu.PoolMatch{
4348+
{Backend: "backendWithTwoPools", Pool: tu.SlowSnapshots},
4349+
})
4350+
}
4351+
4352+
func TestUpdateStorageClassWithBackends_PersistentStoreError(t *testing.T) {
4353+
// Ensure backend-to-storageclass mapping is not updated if persistent store update fails
4354+
mockCtrl := gomock.NewController(t)
4355+
defer mockCtrl.Finish()
4356+
4357+
mockStoreClient := mockpersistentstore.NewMockStoreClient(mockCtrl)
4358+
orchestrator, err := NewTridentOrchestrator(mockStoreClient)
4359+
orchestrator.bootstrapped = true
4360+
orchestrator.bootstrapError = nil
4361+
4362+
mockStoreClient.EXPECT().AddBackend(gomock.Any(), gomock.Any()).Return(nil)
4363+
mockStoreClient.EXPECT().AddStorageClass(gomock.Any(), gomock.Any()).Return(nil)
4364+
mockStoreClient.EXPECT().UpdateStorageClass(gomock.Any(), gomock.Any()).Return(errors.New("failed"))
4365+
4366+
// Add backend with two pools
4367+
mockPools := tu.GetFakePools()
4368+
pools := map[string]*fake.StoragePool{
4369+
"fast-small": mockPools[tu.FastSmall],
4370+
"slow-snapshots": mockPools[tu.SlowSnapshots],
4371+
}
4372+
cfg, err := fakedriver.NewFakeStorageDriverConfigJSON("backendWithTwoPools", config.File, pools, make([]fake.Volume, 0))
4373+
if err != nil {
4374+
t.Fatalf("Unable to generate cfg JSON: %v", err)
4375+
}
4376+
_, err = orchestrator.AddBackend(ctx(), cfg, "")
4377+
if err != nil {
4378+
t.Fatalf("Unable to add backend: %v", err)
4379+
}
4380+
4381+
// Add initial storage class
4382+
initialStorageClassConfig := &storageclass.Config{
4383+
Name: "initial-storage-class",
4384+
Attributes: map[string]sa.Request{
4385+
sa.IOPS: sa.NewIntRequest(2000),
4386+
},
4387+
}
4388+
_, err = orchestrator.AddStorageClass(ctx(), initialStorageClassConfig)
4389+
if err != nil {
4390+
t.Fatalf("Unable to add initial storage class: %v", err)
4391+
}
4392+
4393+
// Validate initial storage class matches pool1
4394+
mockStoreClient.EXPECT().GetStorageClass(gomock.Any(), gomock.Any()).Return(
4395+
orchestrator.storageClasses["initial-storage-class"].ConstructPersistent(), nil)
4396+
4397+
validateStorageClass(t, orchestrator, initialStorageClassConfig.Name, []*tu.PoolMatch{
4398+
{Backend: "backendWithTwoPools", Pool: tu.FastSmall},
4399+
})
4400+
4401+
// Update storage class to match pool2
4402+
updatedStorageClassConfig := &storageclass.Config{
4403+
Name: "initial-storage-class",
4404+
Attributes: map[string]sa.Request{
4405+
sa.IOPS: sa.NewIntRequest(40),
4406+
},
4407+
}
4408+
_, err = orchestrator.UpdateStorageClass(ctx(), updatedStorageClassConfig)
4409+
assert.Error(t, err, "UpdateStorageClass should have failed")
4410+
4411+
// Validate storage class still matches pool1
4412+
mockStoreClient.EXPECT().GetStorageClass(gomock.Any(), gomock.Any()).Return(
4413+
orchestrator.storageClasses["initial-storage-class"].ConstructPersistent(), nil)
4414+
4415+
validateStorageClass(t, orchestrator, updatedStorageClassConfig.Name, []*tu.PoolMatch{
4416+
{Backend: "backendWithTwoPools", Pool: tu.FastSmall},
4417+
})
4418+
}
4419+
41784420
func TestGetBackend(t *testing.T) {
41794421
// Boilerplate mocking code
41804422
mockCtrl := gomock.NewController(t)

‎core/types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ type Orchestrator interface {
7171
DeleteSnapshot(ctx context.Context, volumeName, snapshotName string) error
7272

7373
AddStorageClass(ctx context.Context, scConfig *storageclass.Config) (*storageclass.External, error)
74+
UpdateStorageClass(ctx context.Context, scConfig *storageclass.Config) (*storageclass.External, error)
7475
DeleteStorageClass(ctx context.Context, scName string) error
7576
GetStorageClass(ctx context.Context, scName string) (*storageclass.External, error)
7677
ListStorageClasses(ctx context.Context) ([]*storageclass.External, error)

‎frontend/csi/controller_helpers/kubernetes/config.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ const (
6161
AnnReadOnlyClone = annPrefix + "/readOnlyClone"
6262
AnnLUKSEncryption = annPrefix + "/luksEncryption" // import only
6363
AnnSkipRecoveryQueue = annPrefix + "/skipRecoveryQueue"
64+
AnnSelector = annPrefix + "/selector"
6465
)
6566

6667
var features = map[controllerhelpers.Feature]*versionutils.Version{

0 commit comments

Comments
 (0)