Skip to content

Commit

Permalink
fix recreate sts check for spec.volumeClaimTemplates (#719)
Browse files Browse the repository at this point in the history 
* fix recreate sts when check volumeClaimTemplates

statefulset should be recreated if any of the volumeClaimTemplates got changed

* add changelog

* fix changelog display issue
  • Loading branch information
Haleygo authored Aug 10, 2023
1 parent 2a03bde commit 9ea59e2
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 74 deletions.
27 changes: 12 additions & 15 deletions controllers/factory/k8stools/expansion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ func cleanUpFinalize(ctx context.Context, rclient client.Client, instance client
// One of possible solutions - save current sts to the object annotation and remove it later if needed.
// Other solution, to check orphaned objects by selector.
// Lets leave it as this for now and handle later.
func wasCreatedSTS(ctx context.Context, rclient client.Client, pvcName string, newSTS, existingSTS *appsv1.StatefulSet) (bool, error) {

func wasCreatedSTS(ctx context.Context, rclient client.Client, newSTS, existingSTS *appsv1.StatefulSet) (bool, error) {
handleRemove := func() error {
// removes finalizer from exist sts, it allows to delete it
if err := cleanUpFinalize(ctx, rclient, existingSTS); err != nil {
Expand Down Expand Up @@ -67,7 +66,7 @@ func wasCreatedSTS(ctx context.Context, rclient client.Client, pvcName string, n
// try to restore previous one and throw error
existingSTS.ResourceVersion = ""
if err2 := rclient.Create(ctx, existingSTS); err2 != nil {
return fmt.Errorf("cannot restore previous sts: %s configruation after remove original error: %s: restore error %w", existingSTS.Name, err, err2)
return fmt.Errorf("cannot restore previous sts: %s configuration after remove original error: %s: restore error %w", existingSTS.Name, err, err2)
}
return fmt.Errorf("cannot create new sts: %s instead of replaced, some manual action is required, err: %w", newSTS.Name, err)
}
Expand All @@ -83,7 +82,7 @@ func wasCreatedSTS(ctx context.Context, rclient client.Client, pvcName string, n
}
return nil
}
needRecreateOnStorageChange := func() bool {
needRecreateOnStorageChange := func(pvcName string) bool {
actualPVC := getPVCFromSTS(pvcName, existingSTS)
newPVC := getPVCFromSTS(pvcName, newSTS)
// fast path
Expand Down Expand Up @@ -114,19 +113,18 @@ func wasCreatedSTS(ctx context.Context, rclient client.Client, pvcName string, n

return false
}
needRecreateOnSpecChange := func() bool {
// vct changed - added or removed.
if len(newSTS.Spec.VolumeClaimTemplates) != len(existingSTS.Spec.VolumeClaimTemplates) {
log.Info("VolumeClaimTemplate for statefulset was changed, recreating it", "sts", newSTS.Name)
return true
}
return false
}

if needRecreateOnSpecChange() || needRecreateOnStorageChange() {
// if vct got added, removed or changed, recreate the sts
if len(newSTS.Spec.VolumeClaimTemplates) != len(existingSTS.Spec.VolumeClaimTemplates) {
log.Info("VolumeClaimTemplates for statefulset was changed, recreating it", "sts", newSTS.Name)
return true, handleRemove()
}

for _, newVCT := range newSTS.Spec.VolumeClaimTemplates {
if needRecreateOnStorageChange(newVCT.Name) {
log.Info("VolumeClaimTemplate for statefulset was changed, recreating it", "sts", newSTS.Name, "VolumeClaimTemplates", newVCT.Name)
return true, handleRemove()
}
}
return false, nil
}

Expand Down Expand Up @@ -163,7 +161,6 @@ func growSTSPVC(ctx context.Context, rclient client.Client, sts *appsv1.Stateful

// isStorageClassExpandable check is it possible to update size of given pvc
func isStorageClassExpandable(ctx context.Context, rclient client.Client, pvc *corev1.PersistentVolumeClaim) (bool, error) {

// do not perform any checks if user set annotation explicitly.
if pvc.Annotations[victoriametricsv1beta1.PVCExpandableLabel] == "true" {
return true, nil
Expand Down
69 changes: 47 additions & 22 deletions controllers/factory/k8stools/expansion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
func Test_reCreateSTS(t *testing.T) {
type args struct {
ctx context.Context
pvcName string
newSTS *appsv1.StatefulSet
existingSTS *appsv1.StatefulSet
}
Expand Down Expand Up @@ -57,15 +56,6 @@ func Test_reCreateSTS(t *testing.T) {
},
}},
},
pvcName: "new-claim",
},
predefinedObjects: []runtime.Object{
&appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: "vmselect",
Namespace: "default",
},
},
},
validate: func(sts *appsv1.StatefulSet) error {
if len(sts.Spec.VolumeClaimTemplates) != 1 {
Expand Down Expand Up @@ -110,22 +100,57 @@ func Test_reCreateSTS(t *testing.T) {
},
}},
},
pvcName: "new-claim",
},
predefinedObjects: []runtime.Object{
&appsv1.StatefulSet{
validate: func(sts *appsv1.StatefulSet) error {
if len(sts.Spec.VolumeClaimTemplates) != 1 {
return fmt.Errorf("unexpected configuration for volumeclaim at sts: %v, want at least one, got: %v", sts.Name, sts.Spec.VolumeClaimTemplates)
}
sz := sts.Spec.VolumeClaimTemplates[0].Spec.Resources.Requests.Storage().String()
if sz != "15Gi" {
return fmt.Errorf("unexpected sts size, got: %v, want: %v", sz, "15Gi")
}
return nil
},
},
{
name: "change claim storageClass name",
args: args{
ctx: context.TODO(),
existingSTS: &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: "vmselect",
Namespace: "default",
},
Spec: appsv1.StatefulSetSpec{VolumeClaimTemplates: []v1.PersistentVolumeClaim{
{
ObjectMeta: metav1.ObjectMeta{Name: "new-claim"},
Spec: v1.PersistentVolumeClaimSpec{Resources: v1.ResourceRequirements{
Requests: map[v1.ResourceName]resource.Quantity{
v1.ResourceStorage: resource.MustParse("10Gi"),
Spec: v1.PersistentVolumeClaimSpec{
Resources: v1.ResourceRequirements{
Requests: map[v1.ResourceName]resource.Quantity{
v1.ResourceStorage: resource.MustParse("10Gi"),
},
},
}},
StorageClassName: pointer.String("old-sc"),
},
},
}},
},
newSTS: &appsv1.StatefulSet{
ObjectMeta: metav1.ObjectMeta{
Name: "vmselect",
Namespace: "default",
},
Spec: appsv1.StatefulSetSpec{VolumeClaimTemplates: []v1.PersistentVolumeClaim{
{
ObjectMeta: metav1.ObjectMeta{Name: "new-claim"},
Spec: v1.PersistentVolumeClaimSpec{
Resources: v1.ResourceRequirements{
Requests: map[v1.ResourceName]resource.Quantity{
v1.ResourceStorage: resource.MustParse("10Gi"),
},
},
StorageClassName: pointer.String("new-sc"),
},
},
}},
},
Expand All @@ -134,18 +159,18 @@ func Test_reCreateSTS(t *testing.T) {
if len(sts.Spec.VolumeClaimTemplates) != 1 {
return fmt.Errorf("unexpected configuration for volumeclaim at sts: %v, want at least one, got: %v", sts.Name, sts.Spec.VolumeClaimTemplates)
}
sz := sts.Spec.VolumeClaimTemplates[0].Spec.Resources.Requests.Storage().String()
if sz != "15Gi" {
return fmt.Errorf("unexpected sts size, got: %v, want: %v", sz, "15Gi")
name := *sts.Spec.VolumeClaimTemplates[0].Spec.StorageClassName
if name != "new-sc" {
return fmt.Errorf("unexpected sts storageClass name, got: %v, want: %v", name, "new-sc")
}
return nil
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cl := GetTestClientWithObjects(tt.predefinedObjects)
_, err := wasCreatedSTS(tt.args.ctx, cl, tt.args.pvcName, tt.args.newSTS, tt.args.existingSTS)
cl := GetTestClientWithObjects([]runtime.Object{tt.args.existingSTS})
_, err := wasCreatedSTS(tt.args.ctx, cl, tt.args.newSTS, tt.args.existingSTS)
if (err != nil) != tt.wantErr {
t.Errorf("wasCreatedSTS() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
5 changes: 1 addition & 4 deletions controllers/factory/k8stools/sts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ type STSOptions struct {

// HandleSTSUpdate performs create and update operations for given statefulSet with STSOptions
func HandleSTSUpdate(ctx context.Context, rclient client.Client, cr STSOptions, newSts *appsv1.StatefulSet, c *config.BaseOperatorConf) error {

var currentSts appsv1.StatefulSet
if err := rclient.Get(ctx, types.NamespacedName{Name: newSts.Name, Namespace: newSts.Namespace}, &currentSts); err != nil {
if errors.IsNotFound(err) {
Expand All @@ -57,7 +56,7 @@ func HandleSTSUpdate(ctx context.Context, rclient client.Client, cr STSOptions,
newSts.Spec.Template.Annotations = MergeAnnotations(currentSts.Spec.Template.Annotations, newSts.Spec.Template.Annotations)
newSts.Finalizers = victoriametricsv1beta1.MergeFinalizers(&currentSts, victoriametricsv1beta1.FinalizerName)

isRecreated, err := wasCreatedSTS(ctx, rclient, cr.VolumeName(), newSts, &currentSts)
isRecreated, err := wasCreatedSTS(ctx, rclient, newSts, &currentSts)
if err != nil {
return err
}
Expand Down Expand Up @@ -243,12 +242,10 @@ func performRollingUpdateOnSts(ctx context.Context, wasRecreated bool, rclient c
}

return nil

}

// PodIsFailedWithReason reports if pod failed and the reason of fail
func PodIsFailedWithReason(pod corev1.Pod) (bool, string) {

var reasons []string
for _, containerCond := range pod.Status.ContainerStatuses {
if containerCond.Ready {
Expand Down
Loading

0 comments on commit 9ea59e2

Please sign in to comment.