Skip to content

Commit cac9f2a

Browse files
ewoutpewoutp
committed
Record event on not being allowed to change storage class
1 parent ec7eebe commit cac9f2a

File tree

4 files changed

+69
-41
lines changed

4 files changed

+69
-41
lines changed

‎pkg/deployment/reconcile/plan_builder.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func (d *Reconciler) CreatePlan() error {
5454
apiObject := d.context.GetAPIObject()
5555
spec := d.context.GetSpec()
5656
status, lastVersion := d.context.GetStatus()
57-
newPlan, changed := createPlan(d.log, apiObject, status.Plan, spec, status, pods, d.context.GetTLSKeyfile, d.context.GetTLSCA, d.context.GetPvc)
57+
newPlan, changed := createPlan(d.log, apiObject, status.Plan, spec, status, pods, d.context.GetTLSKeyfile, d.context.GetTLSCA, d.context.GetPvc, d.context.CreateEvent)
5858

5959
// If not change, we're done
6060
if !changed {
@@ -76,12 +76,13 @@ func (d *Reconciler) CreatePlan() error {
7676
// createPlan considers the given specification & status and creates a plan to get the status in line with the specification.
7777
// If a plan already exists, the given plan is returned with false.
7878
// Otherwise the new plan is returned with a boolean true.
79-
func createPlan(log zerolog.Logger, apiObject metav1.Object,
79+
func createPlan(log zerolog.Logger, apiObject k8sutil.APIObject,
8080
currentPlan api.Plan, spec api.DeploymentSpec,
8181
status api.DeploymentStatus, pods []v1.Pod,
8282
getTLSKeyfile func(group api.ServerGroup, member api.MemberStatus) (string, error),
8383
getTLSCA func(string) (string, string, bool, error),
84-
getPVC func(pvcName string) (*v1.PersistentVolumeClaim, error)) (api.Plan, bool) {
84+
getPVC func(pvcName string) (*v1.PersistentVolumeClaim, error),
85+
createEvent func(evt *v1.Event)) (api.Plan, bool) {
8586
if len(currentPlan) > 0 {
8687
// Plan already exists, complete that first
8788
return currentPlan, false
@@ -183,7 +184,7 @@ func createPlan(log zerolog.Logger, apiObject metav1.Object,
183184

184185
// Check for changes storage classes or requirements
185186
if len(plan) == 0 {
186-
plan = createRotateServerStoragePlan(log, spec, status, getPVC)
187+
plan = createRotateServerStoragePlan(log, apiObject, spec, status, getPVC, createEvent)
187188
}
188189

189190
// Check for the need to rotate TLS CA certificate and all members

‎pkg/deployment/reconcile/plan_builder_storage.go‎

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@ import (
2828

2929
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha"
3030
"github.com/arangodb/kube-arangodb/pkg/util"
31+
"github.com/arangodb/kube-arangodb/pkg/util/k8sutil"
3132
)
3233

3334
// createRotateServerStoragePlan creates plan to rotate a server and its volume because of a
3435
// different storage class or a difference in storage resource requirements.
35-
func createRotateServerStoragePlan(log zerolog.Logger, spec api.DeploymentSpec, status api.DeploymentStatus,
36-
getPVC func(pvcName string) (*v1.PersistentVolumeClaim, error)) api.Plan {
36+
func createRotateServerStoragePlan(log zerolog.Logger, apiObject k8sutil.APIObject, spec api.DeploymentSpec, status api.DeploymentStatus,
37+
getPVC func(pvcName string) (*v1.PersistentVolumeClaim, error),
38+
createEvent func(evt *v1.Event)) api.Plan {
3739
if spec.GetMode() == api.DeploymentModeSingle {
3840
// Storage cannot be changed in single server deployments
3941
return nil
@@ -53,10 +55,6 @@ func createRotateServerStoragePlan(log zerolog.Logger, spec api.DeploymentSpec,
5355
// Plan is irrelevant without PVC
5456
continue
5557
}
56-
if group == api.ServerGroupSyncWorkers {
57-
// SyncWorkers have no externally created TLS keyfile
58-
continue
59-
}
6058
groupSpec := spec.GetServerGroupSpec(group)
6159
storageClassName := groupSpec.GetStorageClassName()
6260
if storageClassName == "" {
@@ -78,30 +76,36 @@ func createRotateServerStoragePlan(log zerolog.Logger, spec api.DeploymentSpec,
7876
replacementNeeded = true
7977
}
8078
if replacementNeeded {
81-
if group != api.ServerGroupAgents {
82-
plan = append(plan,
83-
// Scale up, so we're sure that a new member is available
84-
api.NewAction(api.ActionTypeAddMember, group, ""),
85-
api.NewAction(api.ActionTypeWaitForMemberUp, group, api.MemberIDPreviousAction),
86-
)
87-
}
88-
if group == api.ServerGroupDBServers {
89-
plan = append(plan,
90-
// Cleanout
91-
api.NewAction(api.ActionTypeCleanOutMember, group, m.ID),
92-
)
93-
}
94-
plan = append(plan,
95-
// Shutdown & remove the server
96-
api.NewAction(api.ActionTypeShutdownMember, group, m.ID),
97-
api.NewAction(api.ActionTypeRemoveMember, group, m.ID),
98-
)
99-
if group == api.ServerGroupAgents {
79+
if group != api.ServerGroupAgents && group != api.ServerGroupDBServers {
80+
// Only agents & dbservers are allowed to change their storage class.
81+
createEvent(k8sutil.NewCannotChangeStorageClassEvent(apiObject, m.ID, group.AsRole(), "Not supported"))
82+
continue
83+
} else {
84+
if group != api.ServerGroupAgents {
85+
plan = append(plan,
86+
// Scale up, so we're sure that a new member is available
87+
api.NewAction(api.ActionTypeAddMember, group, ""),
88+
api.NewAction(api.ActionTypeWaitForMemberUp, group, api.MemberIDPreviousAction),
89+
)
90+
}
91+
if group == api.ServerGroupDBServers {
92+
plan = append(plan,
93+
// Cleanout
94+
api.NewAction(api.ActionTypeCleanOutMember, group, m.ID),
95+
)
96+
}
10097
plan = append(plan,
101-
// Scale up, so we're adding the removed agent (note: with the old ID)
102-
api.NewAction(api.ActionTypeAddMember, group, m.ID),
103-
api.NewAction(api.ActionTypeWaitForMemberUp, group, m.ID),
98+
// Shutdown & remove the server
99+
api.NewAction(api.ActionTypeShutdownMember, group, m.ID),
100+
api.NewAction(api.ActionTypeRemoveMember, group, m.ID),
104101
)
102+
if group == api.ServerGroupAgents {
103+
plan = append(plan,
104+
// Scale up, so we're adding the removed agent (note: with the old ID)
105+
api.NewAction(api.ActionTypeAddMember, group, m.ID),
106+
api.NewAction(api.ActionTypeWaitForMemberUp, group, m.ID),
107+
)
108+
}
105109
}
106110
}
107111
}

‎pkg/deployment/reconcile/plan_builder_test.go‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/rs/zerolog"
3030
"github.com/stretchr/testify/assert"
3131
"github.com/stretchr/testify/require"
32+
"k8s.io/api/core/v1"
3233
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3334

3435
api "github.com/arangodb/kube-arangodb/pkg/apis/deployment/v1alpha"
@@ -43,6 +44,10 @@ func TestCreatePlanSingleScale(t *testing.T) {
4344
getTLSCA := func(string) (string, string, bool, error) {
4445
return "", "", false, maskAny(fmt.Errorf("Not implemented"))
4546
}
47+
getPVC := func(pvcName string) (*v1.PersistentVolumeClaim, error) {
48+
return nil, maskAny(fmt.Errorf("Not implemented"))
49+
}
50+
createEvent := func(evt *v1.Event) {}
4651
log := zerolog.Nop()
4752
spec := api.DeploymentSpec{
4853
Mode: api.NewMode(api.DeploymentModeSingle),
@@ -58,7 +63,7 @@ func TestCreatePlanSingleScale(t *testing.T) {
5863

5964
// Test with empty status
6065
var status api.DeploymentStatus
61-
newPlan, changed := createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
66+
newPlan, changed := createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
6267
assert.True(t, changed)
6368
assert.Len(t, newPlan, 0) // Single mode does not scale
6469

@@ -69,7 +74,7 @@ func TestCreatePlanSingleScale(t *testing.T) {
6974
PodName: "something",
7075
},
7176
}
72-
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
77+
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
7378
assert.True(t, changed)
7479
assert.Len(t, newPlan, 0) // Single mode does not scale
7580

@@ -84,7 +89,7 @@ func TestCreatePlanSingleScale(t *testing.T) {
8489
PodName: "something1",
8590
},
8691
}
87-
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
92+
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
8893
assert.True(t, changed)
8994
assert.Len(t, newPlan, 0) // Single mode does not scale
9095
}
@@ -97,6 +102,10 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) {
97102
getTLSCA := func(string) (string, string, bool, error) {
98103
return "", "", false, maskAny(fmt.Errorf("Not implemented"))
99104
}
105+
getPVC := func(pvcName string) (*v1.PersistentVolumeClaim, error) {
106+
return nil, maskAny(fmt.Errorf("Not implemented"))
107+
}
108+
createEvent := func(evt *v1.Event) {}
100109
log := zerolog.Nop()
101110
spec := api.DeploymentSpec{
102111
Mode: api.NewMode(api.DeploymentModeActiveFailover),
@@ -113,7 +122,7 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) {
113122

114123
// Test with empty status
115124
var status api.DeploymentStatus
116-
newPlan, changed := createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
125+
newPlan, changed := createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
117126
assert.True(t, changed)
118127
require.Len(t, newPlan, 2)
119128
assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type)
@@ -126,7 +135,7 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) {
126135
PodName: "something",
127136
},
128137
}
129-
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
138+
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
130139
assert.True(t, changed)
131140
require.Len(t, newPlan, 1)
132141
assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type)
@@ -151,7 +160,7 @@ func TestCreatePlanActiveFailoverScale(t *testing.T) {
151160
PodName: "something4",
152161
},
153162
}
154-
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
163+
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
155164
assert.True(t, changed)
156165
require.Len(t, newPlan, 2) // Note: Downscaling is only down 1 at a time
157166
assert.Equal(t, api.ActionTypeShutdownMember, newPlan[0].Type)
@@ -168,6 +177,10 @@ func TestCreatePlanClusterScale(t *testing.T) {
168177
getTLSCA := func(string) (string, string, bool, error) {
169178
return "", "", false, maskAny(fmt.Errorf("Not implemented"))
170179
}
180+
getPVC := func(pvcName string) (*v1.PersistentVolumeClaim, error) {
181+
return nil, maskAny(fmt.Errorf("Not implemented"))
182+
}
183+
createEvent := func(evt *v1.Event) {}
171184
log := zerolog.Nop()
172185
spec := api.DeploymentSpec{
173186
Mode: api.NewMode(api.DeploymentModeCluster),
@@ -183,7 +196,7 @@ func TestCreatePlanClusterScale(t *testing.T) {
183196

184197
// Test with empty status
185198
var status api.DeploymentStatus
186-
newPlan, changed := createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
199+
newPlan, changed := createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
187200
assert.True(t, changed)
188201
require.Len(t, newPlan, 6) // Adding 3 dbservers & 3 coordinators (note: agents do not scale now)
189202
assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type)
@@ -216,7 +229,7 @@ func TestCreatePlanClusterScale(t *testing.T) {
216229
PodName: "coordinator1",
217230
},
218231
}
219-
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
232+
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
220233
assert.True(t, changed)
221234
require.Len(t, newPlan, 3)
222235
assert.Equal(t, api.ActionTypeAddMember, newPlan[0].Type)
@@ -253,7 +266,7 @@ func TestCreatePlanClusterScale(t *testing.T) {
253266
}
254267
spec.DBServers.Count = util.NewInt(1)
255268
spec.Coordinators.Count = util.NewInt(1)
256-
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA)
269+
newPlan, changed = createPlan(log, depl, nil, spec, status, nil, getTLSKeyfile, getTLSCA, getPVC, createEvent)
257270
assert.True(t, changed)
258271
require.Len(t, newPlan, 5) // Note: Downscaling is done 1 at a time
259272
assert.Equal(t, api.ActionTypeCleanOutMember, newPlan[0].Type)

‎pkg/util/k8sutil/events.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,16 @@ func NewPlanAbortedEvent(apiObject APIObject, itemType, memberID, role string) *
165165
return event
166166
}
167167

168+
// NewCannotChangeStorageClassEvent creates an event indicating that an item would need to use a different StorageClass,
169+
// but this is not possible for the given reason.
170+
func NewCannotChangeStorageClassEvent(apiObject APIObject, memberID, role, subReason string) *v1.Event {
171+
event := newDeploymentEvent(apiObject)
172+
event.Type = v1.EventTypeNormal
173+
event.Reason = fmt.Sprintf("%s Member StorageClass Cannot Change", strings.Title(role))
174+
event.Message = fmt.Sprintf("Member %s with role %s should use a different StorageClass, but is cannot because: %s", memberID, role, subReason)
175+
return event
176+
}
177+
168178
// NewErrorEvent creates an even of type error.
169179
func NewErrorEvent(reason string, err error, apiObject APIObject) *v1.Event {
170180
event := newDeploymentEvent(apiObject)

0 commit comments

Comments
 (0)