Skip to content

Commit c982576

Browse files
pure-jliaopure-jliao
committed
[PWX-27765] Fix stc object and migration condition update issues during migration
1 parent 9c4dfe4 commit c982576

File tree

6 files changed

+118
-47
lines changed

6 files changed

+118
-47
lines changed

‎drivers/storage/portworx/portworx_test.go‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2471,6 +2471,63 @@ func TestUpdateClusterStatusWithPortworxDisabled(t *testing.T) {
24712471
require.Equal(t, corev1.ClusterConditionStatusOnline, condition.Status)
24722472
}
24732473

2474+
func TestUpdateClusterStatusMarkMigrationCompleted(t *testing.T) {
2475+
cluster := &corev1.StorageCluster{
2476+
ObjectMeta: metav1.ObjectMeta{
2477+
Name: "px-cluster",
2478+
Namespace: "kube-test",
2479+
Annotations: map[string]string{
2480+
constants.AnnotationDisableStorage: "True",
2481+
},
2482+
},
2483+
Status: corev1.StorageClusterStatus{
2484+
Phase: string(corev1.ClusterStateInit),
2485+
Conditions: []corev1.ClusterCondition{{
2486+
Source: pxutil.PortworxComponentName,
2487+
Type: corev1.ClusterConditionTypeMigration,
2488+
Status: corev1.ClusterConditionStatusInProgress,
2489+
}},
2490+
},
2491+
}
2492+
2493+
ds := &appsv1.DaemonSet{
2494+
ObjectMeta: metav1.ObjectMeta{
2495+
Name: constants.PortworxDaemonSetName,
2496+
Namespace: "kube-test",
2497+
},
2498+
}
2499+
k8sClient := testutil.FakeK8sClient(ds)
2500+
coreops.SetInstance(coreops.New(fakek8sclient.NewSimpleClientset()))
2501+
driver := portworx{}
2502+
err := driver.Init(k8sClient, runtime.NewScheme(), record.NewFakeRecorder(100))
2503+
require.NoError(t, err)
2504+
2505+
// Migration is still in progress, PX is online
2506+
err = driver.UpdateStorageClusterStatus(cluster)
2507+
require.NoError(t, err)
2508+
require.Equal(t, string(corev1.ClusterStateInit), cluster.Status.Phase)
2509+
condition := util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
2510+
require.NotNil(t, condition)
2511+
require.Equal(t, corev1.ClusterConditionStatusInProgress, condition.Status)
2512+
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeRuntimeState)
2513+
require.NotNil(t, condition)
2514+
require.Equal(t, corev1.ClusterConditionStatusOnline, condition.Status)
2515+
2516+
// Daemonset deleted
2517+
err = k8sClient.Delete(context.TODO(), ds)
2518+
require.NoError(t, err)
2519+
2520+
err = driver.UpdateStorageClusterStatus(cluster)
2521+
require.NoError(t, err)
2522+
require.Equal(t, string(corev1.ClusterStateRunning), cluster.Status.Phase)
2523+
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
2524+
require.NotNil(t, condition)
2525+
require.Equal(t, corev1.ClusterConditionStatusCompleted, condition.Status)
2526+
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeRuntimeState)
2527+
require.NotNil(t, condition)
2528+
require.Equal(t, corev1.ClusterConditionStatusOnline, condition.Status)
2529+
}
2530+
24742531
func TestUpdateDeprecatedClusterStatus(t *testing.T) {
24752532
mockCtrl := gomock.NewController(t)
24762533
defer mockCtrl.Finish()
@@ -2644,11 +2701,11 @@ func TestUpdateClusterStatus(t *testing.T) {
26442701
InspectCurrent(gomock.Any(), &api.SdkClusterInspectCurrentRequest{}).
26452702
Return(expectedClusterResp, nil).
26462703
Times(2)
2647-
// Migration in progress
2704+
// Migration not completed
26482705
util.UpdateStorageClusterCondition(cluster, &corev1.ClusterCondition{
26492706
Source: pxutil.PortworxComponentName,
26502707
Type: corev1.ClusterConditionTypeMigration,
2651-
Status: corev1.ClusterConditionStatusInProgress,
2708+
Status: corev1.ClusterConditionStatusPending,
26522709
})
26532710
err = driver.UpdateStorageClusterStatus(cluster)
26542711
require.NoError(t, err)
@@ -2658,7 +2715,7 @@ func TestUpdateClusterStatus(t *testing.T) {
26582715
require.Len(t, cluster.Status.Conditions, 2)
26592716
condition := util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
26602717
require.NotNil(t, condition)
2661-
require.Equal(t, corev1.ClusterConditionStatusInProgress, condition.Status)
2718+
require.Equal(t, corev1.ClusterConditionStatusPending, condition.Status)
26622719
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeRuntimeState)
26632720
require.NotNil(t, condition)
26642721
require.Equal(t, corev1.ClusterConditionStatusOffline, condition.Status)

‎drivers/storage/portworx/status.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strings"
99

1010
"github.com/sirupsen/logrus"
11+
appsv1 "k8s.io/api/apps/v1"
1112
v1 "k8s.io/api/core/v1"
1213
"k8s.io/apimachinery/pkg/api/errors"
1314
"k8s.io/apimachinery/pkg/api/resource"
@@ -41,6 +42,7 @@ func (p *portworx) UpdateStorageClusterStatus(
4142
}
4243

4344
convertDeprecatedClusterStatus(cluster)
45+
p.updatePortworxMigrationStatus(cluster)
4446
err := p.updatePortworxRuntimeStatus(cluster)
4547
newState := getStorageClusterState(cluster)
4648
cluster.Status.Phase = string(newState)
@@ -171,6 +173,28 @@ func (p *portworx) updatePortworxRuntimeStatus(
171173
return p.updateStorageNodes(cluster)
172174
}
173175

176+
func (p *portworx) updatePortworxMigrationStatus(
177+
cluster *corev1.StorageCluster,
178+
) {
179+
// Mark migration as completed when portworx daemonset got deleted
180+
// TODO: check other component condition here
181+
migrationCondition := util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
182+
if migrationCondition != nil && migrationCondition.Status == corev1.ClusterConditionStatusInProgress {
183+
ds := &appsv1.DaemonSet{}
184+
err := p.k8sClient.Get(context.TODO(), types.NamespacedName{
185+
Name: constants.PortworxDaemonSetName,
186+
Namespace: cluster.Namespace,
187+
}, ds)
188+
if errors.IsNotFound(err) {
189+
util.UpdateStorageClusterCondition(cluster, &corev1.ClusterCondition{
190+
Source: pxutil.PortworxComponentName,
191+
Type: corev1.ClusterConditionTypeMigration,
192+
Status: corev1.ClusterConditionStatusCompleted,
193+
})
194+
}
195+
}
196+
}
197+
174198
// getPortworxConditions returns a map of portworx conditions with type as keys
175199
func getPortworxConditions(
176200
cluster *corev1.StorageCluster,

‎pkg/constants/migration.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,7 @@ const (
2828
PhaseAwaitingApproval = "AwaitingMigrationApproval"
2929
// PhaseMigrationInProgress status when migration is in progress
3030
PhaseMigrationInProgress = "MigrationInProgress"
31+
32+
// PortworxDaemonSetName name of portworx DaemonSet
33+
PortworxDaemonSetName = "portworx"
3134
)

‎pkg/migration/backup.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"sigs.k8s.io/yaml"
1717

1818
"github.com/libopenstorage/operator/drivers/storage/portworx/component"
19+
"github.com/libopenstorage/operator/pkg/constants"
1920
k8sutil "github.com/libopenstorage/operator/pkg/util/k8s"
2021
)
2122

@@ -156,7 +157,7 @@ func (h *Handler) addObject(
156157
}
157158

158159
func (h *Handler) getDaemonSetComponent(namespace string, objs *[]client.Object) error {
159-
if err := h.addObject(portworxDaemonSetName, namespace, &appsv1.DaemonSet{}, objs); err != nil {
160+
if err := h.addObject(constants.PortworxDaemonSetName, namespace, &appsv1.DaemonSet{}, objs); err != nil {
160161
return err
161162
}
162163

‎pkg/migration/migration.go‎

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -174,19 +174,31 @@ func (h *Handler) processMigration(
174174
// correctly as we go through different phases of migration we can use that instead of this
175175
// internal annotation.
176176
// Block the component migration until the portworx pod migration is finished.
177-
cluster.Annotations[constants.AnnotationPauseComponentMigration] = "true"
178-
if err := h.client.Update(context.TODO(), cluster, &client.UpdateOptions{}); err != nil {
177+
liveCluster := &corev1.StorageCluster{}
178+
if err := h.client.Get(context.TODO(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, liveCluster); err != nil {
179179
return err
180180
}
181+
liveCluster.Annotations[constants.AnnotationPauseComponentMigration] = "true"
182+
if err := h.client.Update(context.TODO(), liveCluster, &client.UpdateOptions{}); err != nil {
183+
return err
184+
}
185+
181186
// Unblock operator reconcile loop to start managing the storagecluster
182-
util.UpdateStorageClusterCondition(cluster, &corev1.ClusterCondition{
183-
Source: pxutil.PortworxComponentName,
184-
Type: corev1.ClusterConditionTypeMigration,
185-
Status: corev1.ClusterConditionStatusInProgress,
186-
})
187-
if err := k8sutil.UpdateStorageClusterStatus(h.client, cluster); err != nil {
187+
liveCluster = &corev1.StorageCluster{}
188+
if err := h.client.Get(context.TODO(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, liveCluster); err != nil {
188189
return err
189190
}
191+
condition := util.GetStorageClusterCondition(liveCluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
192+
if condition != nil && condition.Status == corev1.ClusterConditionStatusPending {
193+
util.UpdateStorageClusterCondition(liveCluster, &corev1.ClusterCondition{
194+
Source: pxutil.PortworxComponentName,
195+
Type: corev1.ClusterConditionTypeMigration,
196+
Status: corev1.ClusterConditionStatusInProgress,
197+
})
198+
if err := k8sutil.UpdateStorageClusterStatus(h.client, liveCluster); err != nil {
199+
return err
200+
}
201+
}
190202

191203
if err := h.updateDaemonsetToRunOnPendingNodes(ds); err != nil {
192204
return err
@@ -207,33 +219,19 @@ func (h *Handler) processMigration(
207219
return err
208220
}
209221

210-
updatedCluster := &corev1.StorageCluster{}
211-
if err := h.client.Get(context.TODO(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, updatedCluster); err != nil {
222+
liveCluster = &corev1.StorageCluster{}
223+
if err := h.client.Get(context.TODO(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, liveCluster); err != nil {
212224
return err
213225
}
214226
// Notify operator to start installing the new components
215227
logrus.Infof("Starting operator managed components")
216-
delete(updatedCluster.Annotations, constants.AnnotationPauseComponentMigration)
217-
if err := h.client.Update(context.TODO(), updatedCluster, &client.UpdateOptions{}); err != nil {
228+
delete(liveCluster.Annotations, constants.AnnotationPauseComponentMigration)
229+
if err := h.client.Update(context.TODO(), liveCluster, &client.UpdateOptions{}); err != nil {
218230
return err
219231
}
220232

221233
// TODO: Wait for all components to be up, before marking the migration as completed
222234

223-
// Mark migration as completed in the cluster condition list
224-
updatedCluster = &corev1.StorageCluster{}
225-
if err := h.client.Get(context.TODO(), types.NamespacedName{Name: cluster.Name, Namespace: cluster.Namespace}, updatedCluster); err != nil {
226-
return err
227-
}
228-
util.UpdateStorageClusterCondition(updatedCluster, &corev1.ClusterCondition{
229-
Source: pxutil.PortworxComponentName,
230-
Type: corev1.ClusterConditionTypeMigration,
231-
Status: corev1.ClusterConditionStatusCompleted,
232-
})
233-
if err := k8sutil.UpdateStorageClusterStatus(h.client, updatedCluster); err != nil {
234-
return err
235-
}
236-
237235
// TODO: once daemonset is deleted, if we restart operator all code after this line
238236
// will not be re-executed, so we should delete daemonset after everything is finished.
239237
logrus.Infof("Deleting portworx DaemonSet")
@@ -609,7 +607,7 @@ func (h *Handler) getPortworxDaemonSet(ds *appsv1.DaemonSet) (*appsv1.DaemonSet,
609607
err := h.client.Get(
610608
context.TODO(),
611609
types.NamespacedName{
612-
Name: portworxDaemonSetName,
610+
Name: constants.PortworxDaemonSetName,
613611
Namespace: ds.Namespace,
614612
},
615613
pxDaemonSet,
@@ -624,12 +622,12 @@ func (h *Handler) findPortworxDaemonSet() (*appsv1.DaemonSet, error) {
624622
}
625623

626624
for _, ds := range dsList.Items {
627-
if ds.Name == portworxDaemonSetName {
625+
if ds.Name == constants.PortworxDaemonSetName {
628626
return ds.DeepCopy(), nil
629627
}
630628
}
631629

632-
return nil, errors.NewNotFound(appsv1.Resource("DaemonSet"), portworxDaemonSetName)
630+
return nil, errors.NewNotFound(appsv1.Resource("DaemonSet"), constants.PortworxDaemonSetName)
633631
}
634632

635633
func (h *Handler) deletePortworxDaemonSet(ds *appsv1.DaemonSet) error {

‎pkg/migration/migration_test.go‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2156,8 +2156,6 @@ func TestSuccessfulMigration(t *testing.T) {
21562156
require.NoError(t, err)
21572157
_, annotationExists := cluster.Annotations[constants.AnnotationPauseComponentMigration]
21582158
require.False(t, annotationExists)
2159-
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
2160-
require.Equal(t, corev1.ClusterConditionStatusCompleted, condition.Status)
21612159
}
21622160

21632161
func TestFailedMigrationRecoveredWithSkip(t *testing.T) {
@@ -2952,8 +2950,6 @@ func TestOldComponentsAreDeleted(t *testing.T) {
29522950
require.NoError(t, err)
29532951
_, annotationExists := cluster.Annotations[constants.AnnotationPauseComponentMigration]
29542952
require.False(t, annotationExists)
2955-
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
2956-
require.Equal(t, corev1.ClusterConditionStatusCompleted, condition.Status)
29572953
}
29582954

29592955
func TestStorageClusterWithExtraListOfVolumes(t *testing.T) {
@@ -4535,8 +4531,6 @@ func TestSuccessfulMigrationOnDelete(t *testing.T) {
45354531
require.NoError(t, err)
45364532
_, annotationExists := cluster.Annotations[constants.AnnotationPauseComponentMigration]
45374533
require.False(t, annotationExists)
4538-
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
4539-
require.Equal(t, corev1.ClusterConditionStatusCompleted, condition.Status)
45404534
}
45414535

45424536
func TestSuccessfulMigrationRollingUpdateToOnDelete(t *testing.T) {
@@ -4771,8 +4765,6 @@ func TestSuccessfulMigrationRollingUpdateToOnDelete(t *testing.T) {
47714765
require.NoError(t, err)
47724766
_, annotationExists := cluster.Annotations[constants.AnnotationPauseComponentMigration]
47734767
require.False(t, annotationExists)
4774-
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
4775-
require.Equal(t, corev1.ClusterConditionStatusCompleted, condition.Status)
47764768
}
47774769

47784770
func TestSuccessfulMigrationOnDeleteToRollingUpdate(t *testing.T) {
@@ -5016,8 +5008,6 @@ func TestSuccessfulMigrationOnDeleteToRollingUpdate(t *testing.T) {
50165008
require.NoError(t, err)
50175009
_, annotationExists := cluster.Annotations[constants.AnnotationPauseComponentMigration]
50185010
require.False(t, annotationExists)
5019-
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
5020-
require.Equal(t, corev1.ClusterConditionStatusCompleted, condition.Status)
50215011
}
50225012

50235013
func TestStorageClusterCreatedWithInvalidClusterName(t *testing.T) {
@@ -5341,12 +5331,10 @@ func TestTelemetryMigrationWithPX2_12(t *testing.T) {
53415331
cluster = &corev1.StorageCluster{}
53425332
err = testutil.Get(k8sClient, cluster, clusterName, ds.Namespace)
53435333
require.NoError(t, err)
5344-
condition = util.GetStorageClusterCondition(cluster, pxutil.PortworxComponentName, corev1.ClusterConditionTypeMigration)
5345-
if condition.Status != corev1.ClusterConditionStatusCompleted {
5334+
_, annotationExists := cluster.Annotations[constants.AnnotationPauseComponentMigration]
5335+
if annotationExists {
53465336
return false, nil
53475337
}
5348-
_, annotationExists := cluster.Annotations[constants.AnnotationPauseComponentMigration]
5349-
require.False(t, annotationExists)
53505338
return true, nil
53515339
})
53525340
require.NoError(t, err)

0 commit comments

Comments
 (0)