-
Notifications
You must be signed in to change notification settings - Fork 22
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.
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
Support Kubernetes 1.27 #525
Conversation
controller-runtime v0.15.0 was released. |
Signed-off-by: Masayuki Ishii <[email protected]>
bf29d3d
to
a5299cb
Compare
Signed-off-by: Masayuki Ishii <[email protected]>
Signed-off-by: Masayuki Ishii <[email protected]>
Signed-off-by: Masayuki Ishii <[email protected]>
Signed-off-by: Masayuki Ishii <[email protected]>
Signed-off-by: Masayuki Ishii <[email protected]>
Signed-off-by: Masayuki Ishii <[email protected]>
Signed-off-by: Masayuki Ishii <[email protected]>
Signed-off-by: Masayuki Ishii <[email protected]>
@@ -975,7 +974,7 @@ func (r *MySQLClusterReconciler) reconcileV1StatefulSet(ctx context.Context, req | |||
log.Info("volumeClaimTemplates has changed, delete StatefulSet and try to recreate it", "statefulSetName", cluster.PrefixedName()) | |||
|
|||
// When DeletePropagationOrphan is used to delete, it waits because it is not deleted immediately. | |||
if err := wait.PollImmediate(time.Millisecond*500, time.Second*5, func() (bool, error) { | |||
if err := wait.PollUntilContextTimeout(ctx, time.Millisecond*500, time.Second*5, true, func(ctx context.Context) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For newly added but not used context.Context argument, this function accepts with ctx
but the function at line 2046 accepts with _
.
If you care about it, please fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ctx
will be used for the Get()
immediately after...
By the way, I will change argument name form _
to ctx
at line 2046.
Because I found that some existing functions accept ctx
even though it was not used.
https://github.com/cybozu-go/moco/blob/v0.16.1/api/v1beta2/mysqlcluster_webhook.go#L37
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ctx will be used for the Get() immediately after...
oops...
Signed-off-by: Masayuki Ishii <[email protected]>
@@ -29,27 +30,27 @@ type backupPolicyAdmission struct { | |||
|
|||
var _ webhook.CustomValidator = &backupPolicyAdmission{} | |||
|
|||
func (a *backupPolicyAdmission) ValidateCreate(ctx context.Context, obj runtime.Object) error { | |||
func (a *backupPolicyAdmission) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ValidateCreate()
, ValidateUpdate()
and ValidateDelete()
need to retern (warnings Warnings, err error)
@@ -146,13 +146,6 @@ func subMain(ns, addr string, port int) error { | |||
return err | |||
} | |||
|
|||
if config.pprofAddr != "" && config.pprofAddr != "0" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The manager supports pprof server by default.
So we can remove the original pprof server added in #500.
Watches(&source.Kind{Type: certificateObj}, certHandler). | ||
Watches(&source.Kind{Type: &corev1.ConfigMap{}}, configMapHandler). | ||
Watches(&source.Kind{Type: &mocov1beta2.BackupPolicy{}}, backupPolicyHandler). | ||
Watches(certificateObj, certHandler). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The signature of Watches()
has changed.
@@ -2044,7 +2043,7 @@ func setControllerReferenceWithCronJob(cluster *mocov1beta2.MySQLCluster, cronJo | |||
|
|||
// SetupWithManager sets up the controller with the Manager. | |||
func (r *MySQLClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { | |||
certHandler := handler.EnqueueRequestsFromMapFunc(func(a client.Object) []reconcile.Request { | |||
certHandler := handler.EnqueueRequestsFromMapFunc(func(_ context.Context, a client.Object) []reconcile.Request { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MapFunc
requires context.Context
.
@@ -975,7 +974,7 @@ func (r *MySQLClusterReconciler) reconcileV1StatefulSet(ctx context.Context, req | |||
log.Info("volumeClaimTemplates has changed, delete StatefulSet and try to recreate it", "statefulSetName", cluster.PrefixedName()) | |||
|
|||
// When DeletePropagationOrphan is used to delete, it waits because it is not deleted immediately. | |||
if err := wait.PollImmediate(time.Millisecond*500, time.Second*5, func() (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PollImmediate
is deprecated.
refs: kubernetes/kubernetes#107826 and kubernetes/kubernetes#117143
@@ -975,7 +974,7 @@ func (r *MySQLClusterReconciler) reconcileV1StatefulSet(ctx context.Context, req | |||
log.Info("volumeClaimTemplates has changed, delete StatefulSet and try to recreate it", "statefulSetName", cluster.PrefixedName()) | |||
|
|||
// When DeletePropagationOrphan is used to delete, it waits because it is not deleted immediately. | |||
if err := wait.PollImmediate(time.Millisecond*500, time.Second*5, func() (bool, error) { | |||
if err := wait.PollUntilContextTimeout(ctx, time.Millisecond*500, time.Second*5, true, func(ctx context.Context) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ctx
will be used for the Get()
immediately after...
By the way, I will change argument name form _
to ctx
at line 2046.
Because I found that some existing functions accept ctx
even though it was not used.
https://github.com/cybozu-go/moco/blob/v0.16.1/api/v1beta2/mysqlcluster_webhook.go#L37
close: #531
This PR updates k8s libraries and some tools.
The controller-runtime v0.15.0 has breaking changes. So I fix some codes.
Please see PR's comments for the reason of changes.
https://github.com/kubernetes-sigs/controller-runtime/releases/tag/v0.15.0
Signed-off-by: Masayuki Ishii [email protected]