Merge pull request #85023 from MrHohn/svc-lb-ga

Promote service load balancer finalizer to GA
This commit is contained in:
Kubernetes Prow Robot 2019-11-13 17:28:21 -08:00 committed by GitHub
commit 072cf5bd58
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 34 additions and 52 deletions

View File

@ -79,11 +79,6 @@ const (
// originated from: https://github.com/kubernetes/kubernetes/blob/28e800245e/pkg/features/kube_features.go#L178
serviceNodeExclusionFeature = "ServiceNodeExclusion"
// serviceLoadBalancerFinalizerFeature is the feature gate name that
// enables Finalizer Protection for Service LoadBalancers.
// originated from: https://github.com/kubernetes/kubernetes/blob/28e800245e/pkg/features/kube_features.go#L433
serviceLoadBalancerFinalizerFeature = "ServiceLoadBalancerFinalizer"
// legacyNodeRoleBehaviro is the feature gate name that enables legacy
// behavior to vary cluster functionality on the node-role.kubernetes.io
// labels.
@ -103,12 +98,13 @@ type serviceCache struct {
// Controller keeps cloud provider service resources
// (like load balancers) in sync with the registry.
type Controller struct {
cloud cloudprovider.Interface
knownHosts []*v1.Node
servicesToUpdate []*v1.Service
kubeClient clientset.Interface
clusterName string
balancer cloudprovider.LoadBalancer
cloud cloudprovider.Interface
knownHosts []*v1.Node
servicesToUpdate []*v1.Service
kubeClient clientset.Interface
clusterName string
balancer cloudprovider.LoadBalancer
// TODO(#85155): Stop relying on this and remove the cache completely.
cache *serviceCache
serviceLister corelisters.ServiceLister
serviceListerSynced cache.InformerSynced
@ -170,15 +166,8 @@ func New(
s.enqueueService(cur)
}
},
DeleteFunc: func(old interface{}) {
if utilfeature.DefaultFeatureGate.Enabled(serviceLoadBalancerFinalizerFeature) {
// No need to handle deletion event if finalizer feature gate is
// enabled. Because the deletion would be handled by the update
// path when the deletion timestamp is added.
return
}
s.enqueueService(old)
},
// No need to handle deletion event because the deletion would be handled by
// the update path when the deletion timestamp is added.
},
serviceSyncPeriod,
)
@ -335,10 +324,8 @@ func (s *Controller) syncLoadBalancerIfNeeded(service *v1.Service, key string) (
return op, fmt.Errorf("failed to delete load balancer: %v", err)
}
}
// Always try to remove finalizer when load balancer is deleted.
// It will be a no-op if finalizer does not exist.
// Note this also clears up finalizer if the cluster is downgraded
// from a version that attaches finalizer to a version that doesn't.
// Always remove finalizer when load balancer is deleted, this ensures Services
// can be deleted after all corresponding load balancer resources are deleted.
if err := s.removeFinalizer(service); err != nil {
return op, fmt.Errorf("failed to remove load balancer cleanup finalizer: %v", err)
}
@ -348,15 +335,10 @@ func (s *Controller) syncLoadBalancerIfNeeded(service *v1.Service, key string) (
op = ensureLoadBalancer
klog.V(2).Infof("Ensuring load balancer for service %s", key)
s.eventRecorder.Event(service, v1.EventTypeNormal, "EnsuringLoadBalancer", "Ensuring load balancer")
if utilfeature.DefaultFeatureGate.Enabled(serviceLoadBalancerFinalizerFeature) {
// Always try to add finalizer prior to load balancer creation.
// It will be a no-op if finalizer already exists.
// Note this also retrospectively puts on finalizer if the cluster
// is upgraded from a version that doesn't attach finalizer to a
// version that does.
if err := s.addFinalizer(service); err != nil {
return op, fmt.Errorf("failed to add load balancer cleanup finalizer: %v", err)
}
// Always add a finalizer prior to creating load balancers, this ensures Services
// can't be deleted until all corresponding load balancer resources are also deleted.
if err := s.addFinalizer(service); err != nil {
return op, fmt.Errorf("failed to add load balancer cleanup finalizer: %v", err)
}
newStatus, err = s.ensureLoadBalancer(service)
if err != nil {

View File

@ -93,7 +93,6 @@ func newController() (*Controller, *fakecloud.Cloud, *fake.Clientset) {
func TestSyncLoadBalancerIfNeeded(t *testing.T) {
testCases := []struct {
desc string
enableFeatureGate bool
service *v1.Service
lbExists bool
expectOp loadBalancerOperation
@ -155,9 +154,10 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) {
Type: v1.ServiceTypeLoadBalancer,
},
},
expectOp: ensureLoadBalancer,
expectCreateAttempt: true,
expectPatchStatus: true,
expectOp: ensureLoadBalancer,
expectCreateAttempt: true,
expectPatchStatus: true,
expectPatchFinalizer: true,
},
{
desc: "tcp service that wants LB",
@ -175,9 +175,10 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) {
Type: v1.ServiceTypeLoadBalancer,
},
},
expectOp: ensureLoadBalancer,
expectCreateAttempt: true,
expectPatchStatus: true,
expectOp: ensureLoadBalancer,
expectCreateAttempt: true,
expectPatchStatus: true,
expectPatchFinalizer: true,
},
{
desc: "sctp service that wants LB",
@ -195,9 +196,10 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) {
Type: v1.ServiceTypeLoadBalancer,
},
},
expectOp: ensureLoadBalancer,
expectCreateAttempt: true,
expectPatchStatus: true,
expectOp: ensureLoadBalancer,
expectCreateAttempt: true,
expectPatchStatus: true,
expectPatchFinalizer: true,
},
// Finalizer test cases below.
{
@ -259,8 +261,7 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) {
expectPatchFinalizer: true,
},
{
desc: "service without finalizer that wants LB",
enableFeatureGate: true,
desc: "service without finalizer that wants LB",
service: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-service1",
@ -281,8 +282,7 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) {
expectPatchFinalizer: true,
},
{
desc: "service with finalizer that wants LB",
enableFeatureGate: true,
desc: "service with finalizer that wants LB",
service: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-service1",
@ -307,8 +307,6 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, serviceLoadBalancerFinalizerFeature, tc.enableFeatureGate)()
controller, cloud, client := newController()
cloud.Exists = tc.lbExists
key := fmt.Sprintf("%s/%s", tc.service.Namespace, tc.service.Name)
@ -392,7 +390,7 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) {
}
}
if numPatches != expectNumPatches {
t.Errorf("Expected %d patches, got %d instead. Actions: %v", numPatches, expectNumPatches, actions)
t.Errorf("Got %d patches, expect %d instead. Actions: %v", numPatches, expectNumPatches, actions)
}
})
}

View File

@ -434,7 +434,9 @@ const (
CSIMigrationOpenStack featuregate.Feature = "CSIMigrationOpenStack"
// owner: @MrHohn
// beta: v1.16
// alpha: v1.15
// beta: v1.16
// GA: v1.17
//
// Enables Finalizer Protection for Service LoadBalancers.
ServiceLoadBalancerFinalizer featuregate.Feature = "ServiceLoadBalancerFinalizer"
@ -570,7 +572,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
KubeletPodResources: {Default: true, PreRelease: featuregate.Beta},
WindowsGMSA: {Default: true, PreRelease: featuregate.Beta},
WindowsRunAsUserName: {Default: true, PreRelease: featuregate.Beta},
ServiceLoadBalancerFinalizer: {Default: true, PreRelease: featuregate.Beta},
ServiceLoadBalancerFinalizer: {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
LocalStorageCapacityIsolationFSQuotaMonitoring: {Default: false, PreRelease: featuregate.Alpha},
NonPreemptingPriority: {Default: false, PreRelease: featuregate.Alpha},
VolumePVCDataSource: {Default: true, PreRelease: featuregate.Beta},