New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Updated Conditon in Status #546

Merged

masa213f merged 1 commit into cybozu-go:main from YZ775:add-statefulset-condition

Aug 24, 2023

Contributor

YZ775 commented Jul 26, 2023 •

edited

Loading

The health status in MySQLCluster.Status.Condition will be true even if all StatefulSet is not ready when updating MySQLCluster.
So if we use this status as a health check in ArgoCD, they proceed next wave in spite of the update of MySQLCluster is not finished.

As a solution to this problem, I added ConditionUpdated Condition in MySQLCluster.Status.Condition.
This condition will become true when an update of Statefulset is finished.

This PR contains following improvement

Change codes to use metav1.Condition instead of MySQLClusterCondition
Fix unstable envtest

Member

ymmt2005 commented Jul 26, 2023

@YZ775
I guess UpToDate makes more sense than Updated.

Contributor Author

YZ775 commented Jul 26, 2023

@YZ775 I guess UpToDate makes more sense than Updated.

Thank you. I also think UpToDate is better.
I've renamed the field.

YZ775 requested a review from zoetrope

July 26, 2023 04:22

zoetrope reviewed

View reviewed changes

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

YZ775 commented

View reviewed changes

clustering/process.go Show resolved Hide resolved

zoetrope reviewed

View reviewed changes

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

zoetrope reviewed

View reviewed changes

e2e/upgrade_test.go Outdated Show resolved Hide resolved

YZ775 force-pushed the add-statefulset-condition branch 3 times, most recently from a21f064 to 60e7997 Compare

July 26, 2023 09:01

masa213f reviewed

View reviewed changes

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

e2e/upgrade_test.go Outdated Show resolved Hide resolved

Member

zoetrope commented Jul 27, 2023

Please add documentation for this feature to reconcile.md.

masa213f requested changes

View reviewed changes

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller_test.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller_test.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller_test.go Show resolved Hide resolved

e2e/upgrade_test.go Outdated Show resolved Hide resolved

YZ775 force-pushed the add-statefulset-condition branch from 1e924b7 to cb5e6f7 Compare

August 4, 2023 00:48

YZ775 requested review from masa213f and zoetrope

August 4, 2023 01:14

masa213f requested changes

View reviewed changes

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

clustering/process.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller_test.go Show resolved Hide resolved

docs/crd_mysqlcluster_v1beta1.md Outdated Show resolved Hide resolved

docs/reconcile.md Show resolved Hide resolved

controllers/mysqlcluster_controller_test.go Outdated Show resolved Hide resolved

e2e/lifecycle_test.go Show resolved Hide resolved

masa213f self-requested a review

August 16, 2023 08:39

masa213f requested changes

View reviewed changes

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

docs/reconcile.md Outdated Show resolved Hide resolved

docs/reconcile.md Outdated Show resolved Hide resolved

docs/links.csv Outdated Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

e2e/lifecycle_test.go Show resolved Hide resolved

e2e/upgrade_test.go Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

masa213f self-requested a review

August 21, 2023 01:03

masa213f requested changes

View reviewed changes

controllers/mysqlcluster_controller.go Outdated

+              	log := crlog.FromContext(ctx)
+              	orig := cluster.DeepCopy()
+              	if cluster.Status.ReconcileInfo.Generation != cluster.Generation {

Contributor

masa213f Aug 21, 2023

This if statement is unnecessary.
Currently, we use DeepEqual() to determine whether or not an update is required.

e2e/lifecycle_test.go Show resolved Hide resolved

controllers/mysqlcluster_controller.go Outdated Show resolved Hide resolved

YZ775 requested a review from masa213f

August 23, 2023 07:14


          add StatefulSet condition and change MySQLClusterCondition to metav1.…

b7d6985

…Condition

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

add envtest

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update gomod

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update envtest and add e2e

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update apidoc

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

rename

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

add ObservedGeneration and add partition handling, and update e2e test

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

remove unnecessary ObservedGeneration and move updateStatus() after reconcileV1StatefulSet() , and update e2e test

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

add document about UpToDate

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update against review

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

wip

change status to StatefulSetReady and add ReconcileSuccess, and fix unstable test

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

remove unnecessary condition check

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

remove redundant assignments in updateStatusByStatefulSet

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

move updateReconcileStatus in reconcileV1

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

fix controller test

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update documents

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update lifecycle_test.go to check StatefulSetReady condition

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

fix auto generated document

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

join update status funcion

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

revert Eventually in envtest

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

fix multiple Update() call

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update docs

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

fix controller

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update condition when sts not found

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update test

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

update lifecycle_test.go

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

fix condition

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

fix generation check and err handling

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

do not show err log when deletion of cluster

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

add StatefulSet condition and change MySQLClusterCondition to metav1.Condition

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>

YZ775 force-pushed the add-statefulset-condition branch from 482edb0 to b7d6985 Compare

August 24, 2023 02:43

masa213f approved these changes

View reviewed changes

Contributor

masa213f left a comment

LGTM. Thank you!

zoetrope approved these changes

View reviewed changes

Member

zoetrope left a comment

LGTM

masa213f merged commit 6b953a6 into cybozu-go:main

15 checks passed

masa213f mentioned this pull request

Add a status indicating whether or not the MySQLCluster is being updated. #539

Closed

4 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet