Skip to content

Commit

Permalink
status_cluster: add argument to skip checkDependency (#192)
Browse files Browse the repository at this point in the history
Signed-off-by: haorenfsa <[email protected]>
  • Loading branch information
haorenfsa authored Sep 24, 2024
1 parent cef14bc commit 20f7176
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 26 deletions.
2 changes: 1 addition & 1 deletion pkg/controllers/deploy_ctrl.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func (c *DeployControllerBizImpl) IsUpdating(ctx context.Context, mc v1beta1.Mil
if mc.Spec.IsStopping() {
return false, nil
}
err := c.statusSyncer.UpdateStatusForNewGeneration(ctx, &mc)
err := c.statusSyncer.UpdateStatusForNewGeneration(ctx, &mc, false)
if err != nil {
return false, errors.Wrap(err, "update status for new generation")
}
Expand Down
12 changes: 6 additions & 6 deletions pkg/controllers/deploy_ctrl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,20 +247,20 @@ func TestDeployControllerBizImpl_IsUpdating(t *testing.T) {
})

t.Run("update status failed", func(t *testing.T) {
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc).Return(errMock)
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc, false).Return(errMock)
_, err := bizImpl.IsUpdating(ctx, mc)
assert.Error(t, err)
})

t.Run("milvus condition not exist, sugguests updating", func(t *testing.T) {
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc).Return(nil)
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc, false).Return(nil)
ret, err := bizImpl.IsUpdating(ctx, mc)
assert.NoError(t, err)
assert.True(t, ret)
})

t.Run("milvus condition shows updating", func(t *testing.T) {
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc).Return(nil)
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc, false).Return(nil)
mc.Status.Conditions = []v1beta1.MilvusCondition{
{
Type: v1beta1.MilvusUpdated,
Expand All @@ -280,14 +280,14 @@ func TestDeployControllerBizImpl_IsUpdating(t *testing.T) {
}

t.Run("get old deploy failed", func(t *testing.T) {
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc).Return(nil)
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc, false).Return(nil)
mockUtil.EXPECT().GetOldDeploy(ctx, mc, component).Return(nil, errMock)
_, err := bizImpl.IsUpdating(ctx, mc)
assert.Error(t, err)
})

t.Run("is new rollout, so updating", func(t *testing.T) {
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc).Return(nil)
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc, false).Return(nil)
deploy := appsv1.Deployment{}

mockUtil.EXPECT().GetOldDeploy(ctx, mc, component).Return(&deploy, nil)
Expand All @@ -300,7 +300,7 @@ func TestDeployControllerBizImpl_IsUpdating(t *testing.T) {
})

t.Run("not updating", func(t *testing.T) {
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc).Return(nil)
mockStatusSyncer.EXPECT().UpdateStatusForNewGeneration(ctx, &mc, false).Return(nil)
deploy := appsv1.Deployment{}

mockUtil.EXPECT().GetOldDeploy(ctx, mc, component).Return(&deploy, nil)
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/milvus_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ func (r *MilvusReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
return ctrl.Result{}, err
}

if err := r.statusSyncer.UpdateStatusForNewGeneration(ctx, milvus); err != nil {
// not check dependency, to avoid blocking too long
if err := r.statusSyncer.UpdateStatusForNewGeneration(ctx, milvus, false); err != nil {
return ctrl.Result{}, err
}
// metrics
Expand Down
27 changes: 9 additions & 18 deletions pkg/controllers/status_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type EtcdEndPointHealth struct {
// MilvusStatusSyncerInterface abstracts MilvusStatusSyncer
type MilvusStatusSyncerInterface interface {
RunIfNot()
UpdateStatusForNewGeneration(ctx context.Context, mc *v1beta1.Milvus) error
UpdateStatusForNewGeneration(ctx context.Context, mc *v1beta1.Milvus, checkDependency bool) error
}

type MilvusStatusSyncer struct {
Expand Down Expand Up @@ -218,7 +218,7 @@ func (r *MilvusStatusSyncer) UpdateStatusRoutine(ctx context.Context, mc *v1beta
// so we call default again to ensure
mc.Default()

err := r.UpdateStatusForNewGeneration(ctx, mc)
err := r.UpdateStatusForNewGeneration(ctx, mc, true)
return errors.Wrapf(err, "UpdateStatus for milvus[%s/%s]", mc.Namespace, mc.Name)
}

Expand Down Expand Up @@ -252,29 +252,20 @@ func (r *MilvusStatusSyncer) checkDependencyConditions(ctx context.Context, mc *
return nil
}

func (r *MilvusStatusSyncer) UpdateStatusForNewGeneration(ctx context.Context, mc *v1beta1.Milvus) error {
// UpdateStatusForNewGeneration updates the status of the Milvus CR. if given checkDependency = true, it will check the dependency conditions
func (r *MilvusStatusSyncer) UpdateStatusForNewGeneration(ctx context.Context, mc *v1beta1.Milvus, checkDependency bool) error {
beginStatus := mc.Status.DeepCopy()
mc.Status.ObservedGeneration = mc.Generation

if !Debug {
if Debug {
checkDependency = false
}

if checkDependency {
err := r.checkDependencyConditions(ctx, mc)
if err != nil {
return errors.Wrap(err, "check dependency conditions failed")
}
} else {
// debug mode, set dependency conditions to true
UpdateCondition(&mc.Status, v1beta1.MilvusCondition{
Type: v1beta1.EtcdReady,
Status: corev1.ConditionTrue,
})
UpdateCondition(&mc.Status, v1beta1.MilvusCondition{
Type: v1beta1.StorageReady,
Status: corev1.ConditionTrue,
})
UpdateCondition(&mc.Status, v1beta1.MilvusCondition{
Type: v1beta1.MsgStreamReady,
Status: corev1.ConditionTrue,
})
}

err := r.UpdateIngressStatus(ctx, mc)
Expand Down

0 comments on commit 20f7176

Please sign in to comment.