KEP-2436 Leader Migration: to beta #103533

jiahuif · 2021-07-07T05:22:50Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR graduates kubernetes/enhancements#2436 , Leader Migration for Controller Managers, from alpha to beta, by making following changes:

Change the feature flag from "alpha, disabled by default" to "beta, enabled by default"
Copy the v1alpha resource definitions to v1beta, and add corresponding registration & tests.

Special notes for your reviewer:

Leader Migration can graduate to beta because:

Both cloud-provider-gcp and cloud-provider-aws have tested this feature [1]
Missing feature has been implemented during 1.22 cycle KEP-2436: wildcard and default value enhancements#2731 , Leader Migration: wildcard and default value #102711 .

For vendor/ approver: This PR adds a new module k8s.io/controller-manager/config/v1beta1 as a result of API graduation. The module needs to be vendor'ed as a part of k8s.io/controller-manager.

For feature approver: This PR adds a feature gate defined in staging/src/k8s.io/controller-manager/pkg/features/kube_features.go to pkg/features/kube_features.go so that pkg/features/kube_features.go contains all feature gates of k8s.io/controller-manager.

Does this PR introduce a user-facing change?

Leader Migration for controller managers graduated to beta.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/issues/2436

[1] SIG Cloud Provider Meeting Agenda, June 23

jiahuif · 2021-07-07T17:08:49Z

/assign @jpbetz

jpbetz · 2021-07-07T17:12:57Z

Do we need conversions or defautls for v1beta1 like exist for v1alpha1?

jiahuif · 2021-07-07T17:14:59Z

Conversions from/to v1beta1 are generated. Defaults are applied to internal version only.

jpbetz · 2021-07-07T17:24:10Z

/lgtm

cheftako · 2021-07-07T17:49:02Z

staging/src/k8s.io/controller-manager/pkg/features/kube_features.go

@@ -49,5 +50,5 @@ func SetupCurrentKubernetesSpecificFeatureGates(featuregates featuregate.Mutable
 // To add a new feature, define a key for it at k8s.io/api/pkg/features and add it here.
 var cloudPublicFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	IPv6DualStack:                    {Default: true, PreRelease: featuregate.Beta},
-	ControllerManagerLeaderMigration: {Default: false, PreRelease: featuregate.Alpha},
+	ControllerManagerLeaderMigration: {Default: true, PreRelease: featuregate.Beta},


I believe this should also exist in pkg/features/kube_features.go. (ala IPv6DualStack)

We had this discussion when working on alpha. The conclusion was that ControllerManagerLeaderMigration is a feature flag local to staging/controller-manger thus does not need a place in pkg/features/kube_features.go. I do not see a reason to inverse such conclusion during alpha-to-beta graduation. Could you illiterate a bit more why we need it in pkg/features/kube_features.go now?

I don't believe we did have a discussion on this in alpha. You did in fact make this assertion during alpha (#99417 (comment)) however I did not agree then and still do not. However as an alpha feature I was unwilling to block based on that alone. For beta I do consider this to be blocking. pkg/feature/kube_features is the master list and if its missing from there it will be missing from places like https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/.

Thanks for pointing out the doc. Updating now.

cheftako · 2021-07-07T18:00:45Z

staging/src/k8s.io/controller-manager/config/v1beta1/types.go

+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+


Are we getting rid of GenericControllerManagerConfiguration (https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/controller-manager/config/v1alpha1/types.go#L25)? If not then I think it should be moving to v1beta1 as well.

GenericControllerManagerConfiguration is extracted from common part of corresponding structures of KCM and CCM during the "make staging/controller-manager easier to consume task by @cici37". It is still needed unless we want to rollback any changes during the task. However, GenericControllerManagerConfiguration should not go to v1beta because:

The stability is unknown, but it is definitely less than beta

We are keeping v1alpha package following convention of how we graduate resources with feature, it would not hurt to keep GenericControllerManagerConfiguration there. There are examples of some resources in a group graduate later than other resources. CronJob goes GA only in 1.21 while others of group batch are GA long before 1.21

If we are to graduate GenericControllerManagerConfiguration to beta, it would be far out of scope of this PR. We can do this in a separate PR to avoid scope creeping.

jiahuif · 2021-07-07T19:35:51Z

/assign @cheftako

jiahuif · 2021-07-07T22:21:50Z

/cc @andrewsykim

jiahuif · 2021-07-08T21:05:12Z

Fixing feature conflicts, 2nd time.

andrewsykim

/approve

for pkg/features

jpbetz · 2021-07-08T21:39:53Z

/lgtm

BenTheElder · 2021-07-08T21:43:50Z

/approve
for vendor/modules.txt

k8s-ci-robot · 2021-07-08T21:44:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewsykim, BenTheElder, cheftako, jiahuif

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/features/OWNERS~~ [andrewsykim]
~~staging/src/k8s.io/controller-manager/OWNERS~~ [andrewsykim,cheftako]
~~vendor/OWNERS~~ [BenTheElder]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cheftako · 2021-07-08T22:15:00Z

/milestone v1.22

jiahuif · 2021-07-08T22:15:03Z

/retest

This feature was in staging/src/k8s.io/controller-manager/pkg/features/kube_features.go but missing here.

jiahuif · 2021-07-09T04:20:33Z

fixing conflicts for the 3rd time.

jiahuif · 2021-07-09T14:54:16Z

/retest

jpbetz · 2021-07-09T15:41:37Z

/lgtm

Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - NodeSwap kubernetes/kubernetes#102823, kubernetes/kubernetes#103553 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.

Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DelegateFSGroupToCSIDriver kubernetes/kubernetes#103244 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.

Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - MemoryQoS kubernetes/kubernetes#102970 - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - StatefulSetAutoDeletePVC kubernetes/kubernetes#99378 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.

Here is a second batch for feature gate updates in 1.22. - CPUManagerPolicyOptions kubernetes/kubernetes#101432 - ControllerManagerLeaderMigration kubernetes/kubernetes#103533 - DynamicKubeletConfig kubernetes/kubernetes#102966 - EndpointSliceProxying kubernetes/kubernetes#103451 - EndpointSliceTerminatingCondition kubernetes/kubernetes#103596 - HugePageStorageMediumSize kubernetes/kubernetes#99144 - JobTrackingWithFinalizers kubernetes/kubernetes#98817 (also tracked in kubernetes#28841, can rebase). - ServiceInternalTrafficPolicy kubernetes/kubernetes#103462 - WindowsEndpointSliceProxying kubernetes/kubernetes#103451 Some of these needs more detailed documentation.

k8s-ci-robot requested review from deads2k and wojtek-t July 7, 2021 05:23

k8s-ci-robot assigned jpbetz Jul 7, 2021

jiahuif changed the title ~~Leader Migration: to beta~~ KEP-2436 Leader Migration: to beta Jul 7, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2021

cheftako reviewed Jul 7, 2021

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 7, 2021

k8s-ci-robot assigned cheftako Jul 7, 2021

k8s-ci-robot requested a review from andrewsykim July 7, 2021 22:21

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 8, 2021

andrewsykim reviewed Jul 8, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 8, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 8, 2021

k8s-ci-robot added this to the v1.22 milestone Jul 8, 2021

jiahuif added 3 commits July 8, 2021 21:20

add ControllerManagerLeaderMigration as beta

995278c

This feature was in staging/src/k8s.io/controller-manager/pkg/features/kube_features.go but missing here.

promote ControllerManagerLeaderMigration to beta.

40b2155

create LeaderMigrationConfiguration v1beta1

b14c10a

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 9, 2021

jiahuif mentioned this pull request Jul 9, 2021

KEP-2436 Leader Migration: to beta kubernetes/website#28882

Merged

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 9, 2021

k8s-ci-robot merged commit 6075621 into kubernetes:master Jul 9, 2021

jiahuif deleted the feature/leader-migration/to-beta branch July 9, 2021 19:15

tengqm mentioned this pull request Jul 11, 2021

Batch update feature gates for 1.22 kubernetes/website#28849

Merged

jiahuif mentioned this pull request Aug 2, 2021

Leader Migration for Controller Managers kubernetes/enhancements#2436

Closed

12 tasks

reylejano mentioned this pull request Mar 25, 2022

Fix feature state for ControllerLeaderMigration kubernetes/website#32486

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KEP-2436 Leader Migration: to beta #103533

KEP-2436 Leader Migration: to beta #103533

jiahuif commented Jul 7, 2021 •

edited

jiahuif commented Jul 7, 2021

jpbetz commented Jul 7, 2021

jiahuif commented Jul 7, 2021

jpbetz commented Jul 7, 2021

cheftako Jul 7, 2021

jiahuif Jul 7, 2021 •

edited

cheftako Jul 7, 2021

jiahuif Jul 7, 2021

cheftako Jul 7, 2021

jiahuif Jul 7, 2021 •

edited

jiahuif commented Jul 7, 2021

jiahuif commented Jul 7, 2021

jiahuif commented Jul 8, 2021

andrewsykim left a comment

jpbetz commented Jul 8, 2021

BenTheElder commented Jul 8, 2021

k8s-ci-robot commented Jul 8, 2021

cheftako commented Jul 8, 2021

jiahuif commented Jul 8, 2021

jiahuif commented Jul 9, 2021

jiahuif commented Jul 9, 2021

jpbetz commented Jul 9, 2021

KEP-2436 Leader Migration: to beta #103533

KEP-2436 Leader Migration: to beta #103533

Conversation

jiahuif commented Jul 7, 2021 • edited

What type of PR is this?

What this PR does / why we need it:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

jiahuif commented Jul 7, 2021

jpbetz commented Jul 7, 2021

jiahuif commented Jul 7, 2021

jpbetz commented Jul 7, 2021

cheftako Jul 7, 2021

Choose a reason for hiding this comment

jiahuif Jul 7, 2021 • edited

Choose a reason for hiding this comment

cheftako Jul 7, 2021

Choose a reason for hiding this comment

jiahuif Jul 7, 2021

Choose a reason for hiding this comment

cheftako Jul 7, 2021

Choose a reason for hiding this comment

jiahuif Jul 7, 2021 • edited

Choose a reason for hiding this comment

jiahuif commented Jul 7, 2021

jiahuif commented Jul 7, 2021

jiahuif commented Jul 8, 2021

andrewsykim left a comment

Choose a reason for hiding this comment

jpbetz commented Jul 8, 2021

BenTheElder commented Jul 8, 2021

k8s-ci-robot commented Jul 8, 2021

cheftako commented Jul 8, 2021

jiahuif commented Jul 8, 2021

jiahuif commented Jul 9, 2021

jiahuif commented Jul 9, 2021

jpbetz commented Jul 9, 2021

jiahuif commented Jul 7, 2021 •

edited

jiahuif Jul 7, 2021 •

edited

jiahuif Jul 7, 2021 •

edited