Skip to content

Deprecate --enable-readiness-reflector, consider always enabled #2891

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

Closed
wants to merge 3 commits into from
Closed

Deprecate --enable-readiness-reflector, consider always enabled #2891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8a5724
Deprecate --enable-delete-unused-frontends, consider always enabled
panslava May 22, 2025
08081f3
Deprecate --enable-frontend-config, consider always enabled
panslava May 22, 2025
58bd3fc
Deprecate --enable-readiness-reflector, consider always enabled
panslava May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions cmd/glbc/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,16 +147,14 @@ func main() {
}

var frontendConfigClient frontendconfigclient.Interface
if flags.F.EnableFrontendConfig {
frontendConfigCRDMeta := frontendconfig.CRDMeta()
if _, err := crdHandler.EnsureCRD(frontendConfigCRDMeta, true); err != nil {
klog.Fatalf("Failed to ensure FrontendConfig CRD: %v", err)
}
frontendConfigCRDMeta := frontendconfig.CRDMeta()
if _, err := crdHandler.EnsureCRD(frontendConfigCRDMeta, true); err != nil {
klog.Fatalf("Failed to ensure FrontendConfig CRD: %v", err)
}

frontendConfigClient, err = frontendconfigclient.NewForConfig(kubeConfig)
if err != nil {
klog.Fatalf("Failed to create FrontendConfig client: %v", err)
}
frontendConfigClient, err = frontendconfigclient.NewForConfig(kubeConfig)
if err != nil {
klog.Fatalf("Failed to create FrontendConfig client: %v", err)
}

var firewallCRClient firewallcrclient.Interface
Expand Down Expand Up @@ -336,7 +334,6 @@ func main() {
NumL4NetLBWorkers: flags.F.NumL4NetLBWorkers,
DefaultBackendSvcPort: defaultBackendServicePort,
HealthCheckPath: flags.F.HealthCheckPath,
FrontendConfigEnabled: flags.F.EnableFrontendConfig,
MaxIGSize: flags.F.MaxIGSize,
EnableL4ILBDualStack: flags.F.EnableL4ILBDualStack,
EnableL4NetLBDualStack: flags.F.EnableL4NetLBDualStack,
Expand Down Expand Up @@ -677,7 +674,6 @@ func createNEGController(ctx *ingctx.ControllerContext, systemHealth *systemheal
flags.F.ResyncPeriod,
flags.F.NegGCPeriod,
flags.F.NumNegGCWorkers,
flags.F.EnableReadinessReflector,
flags.F.EnableL4NEG,
flags.F.EnableNonGCPMode,
flags.F.EnableDualStackNEG,
Expand Down
5 changes: 1 addition & 4 deletions pkg/context/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ type ControllerContextConfig struct {
// DefaultBackendSvcPortID is the ServicePort for the system default backend.
DefaultBackendSvcPort utils.ServicePort
HealthCheckPath string
FrontendConfigEnabled bool
MaxIGSize int
EnableL4ILBDualStack bool
EnableL4NetLBDualStack bool
Expand Down Expand Up @@ -191,9 +190,7 @@ func NewControllerContext(
if firewallClient != nil {
context.FirewallInformer = informerfirewall.NewGCPFirewallInformer(firewallClient, config.ResyncPeriod, utils.NewNamespaceIndexer())
}
if config.FrontendConfigEnabled {
context.FrontendConfigInformer = informerfrontendconfig.NewFrontendConfigInformer(frontendConfigClient, config.Namespace, config.ResyncPeriod, utils.NewNamespaceIndexer())
}
context.FrontendConfigInformer = informerfrontendconfig.NewFrontendConfigInformer(frontendConfigClient, config.Namespace, config.ResyncPeriod, utils.NewNamespaceIndexer())

if saClient != nil {
context.SAInformer = informerserviceattachment.NewServiceAttachmentInformer(saClient, config.Namespace, config.ResyncPeriod, utils.NewNamespaceIndexer())
Expand Down
88 changes: 41 additions & 47 deletions pkg/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,45 +276,43 @@ func NewLoadBalancerController(
})

// FrontendConfig event handlers.
if ctx.FrontendConfigEnabled {
ctx.FrontendConfigInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) {
feConfig := obj.(*frontendconfigv1beta1.FrontendConfig)
ctx.FrontendConfigInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) {
feConfig := obj.(*frontendconfigv1beta1.FrontendConfig)
ings := operator.Ingresses(ctx.Ingresses().List()).ReferencesFrontendConfig(feConfig).AsList()
lbc.ingQueue.Enqueue(convert(ings)...)
},
UpdateFunc: func(old, cur interface{}) {
if !reflect.DeepEqual(old, cur) {
feConfig := cur.(*frontendconfigv1beta1.FrontendConfig)
logger.Info("FrontendConfig updated", "feConfigName", klog.KRef(feConfig.Namespace, feConfig.Name))
ings := operator.Ingresses(ctx.Ingresses().List()).ReferencesFrontendConfig(feConfig).AsList()
lbc.ingQueue.Enqueue(convert(ings)...)
},
UpdateFunc: func(old, cur interface{}) {
if !reflect.DeepEqual(old, cur) {
feConfig := cur.(*frontendconfigv1beta1.FrontendConfig)
logger.Info("FrontendConfig updated", "feConfigName", klog.KRef(feConfig.Namespace, feConfig.Name))
ings := operator.Ingresses(ctx.Ingresses().List()).ReferencesFrontendConfig(feConfig).AsList()
lbc.ingQueue.Enqueue(convert(ings)...)
}
},
DeleteFunc: func(obj interface{}) {
var feConfig *frontendconfigv1beta1.FrontendConfig
var ok, feOk bool
feConfig, ok = obj.(*frontendconfigv1beta1.FrontendConfig)
if !ok {
// This can happen if the watch is closed and misses the delete event
state, stateOk := obj.(cache.DeletedFinalStateUnknown)
if !stateOk {
logger.Error(nil, "Wanted cache.DeleteFinalStateUnknown of frontendconfig obj", "got", fmt.Sprintf("%+v", obj), "gotType", fmt.Sprintf("%T", obj))
return
}
},
DeleteFunc: func(obj interface{}) {
var feConfig *frontendconfigv1beta1.FrontendConfig
var ok, feOk bool
feConfig, ok = obj.(*frontendconfigv1beta1.FrontendConfig)
if !ok {
// This can happen if the watch is closed and misses the delete event
state, stateOk := obj.(cache.DeletedFinalStateUnknown)
if !stateOk {
logger.Error(nil, "Wanted cache.DeleteFinalStateUnknown of frontendconfig obj", "got", fmt.Sprintf("%+v", obj), "gotType", fmt.Sprintf("%T", obj))
return
}

feConfig, feOk = state.Obj.(*frontendconfigv1beta1.FrontendConfig)
if !feOk {
logger.Error(nil, "Wanted frontendconfig obj", "got", fmt.Sprintf("%+v", state.Obj), "gotType", fmt.Sprintf("%T", state.Obj))
return
}

feConfig, feOk = state.Obj.(*frontendconfigv1beta1.FrontendConfig)
if !feOk {
logger.Error(nil, "Wanted frontendconfig obj", "got", fmt.Sprintf("%+v", state.Obj), "gotType", fmt.Sprintf("%T", state.Obj))
return
}
}

ings := operator.Ingresses(ctx.Ingresses().List()).ReferencesFrontendConfig(feConfig).AsList()
lbc.ingQueue.Enqueue(convert(ings)...)
},
})
}
ings := operator.Ingresses(ctx.Ingresses().List()).ReferencesFrontendConfig(feConfig).AsList()
lbc.ingQueue.Enqueue(convert(ings)...)
},
})

if enableMultiSubnetClusterPhase1 {
// SvcNeg event handlers.
Expand Down Expand Up @@ -724,11 +722,9 @@ func (lbc *LoadBalancerController) sync(key string) error {
} else {
// Insert/update the ingress state for metrics after successful sync.
var fc *frontendconfigv1beta1.FrontendConfig
if flags.F.EnableFrontendConfig {
fc, err = frontendconfig.FrontendConfigForIngress(lbc.ctx.FrontendConfigs().List(), ing)
if err != nil {
return err
}
fc, err = frontendconfig.FrontendConfigForIngress(lbc.ctx.FrontendConfigs().List(), ing)
if err != nil {
return err
}
lbc.metrics.SetIngress(key, metrics.NewIngressState(ing, fc, urlMap.AllServicePorts()))
}
Expand Down Expand Up @@ -811,15 +807,13 @@ func (lbc *LoadBalancerController) toRuntimeInfo(ing *v1.Ingress, urlMap *utils.
}

var feConfig *frontendconfigv1beta1.FrontendConfig
if lbc.ctx.FrontendConfigEnabled {
feConfig, err = frontendconfig.FrontendConfigForIngress(lbc.ctx.FrontendConfigs().List(), ing)
if err != nil {
lbc.ctx.Recorder(ing.Namespace).Eventf(ing, apiv1.EventTypeWarning, events.SyncIngress, "Error: %v", err)
}
// Object in cache could be changed in-flight. Deepcopy to
// reduce race conditions.
feConfig = feConfig.DeepCopy()
feConfig, err = frontendconfig.FrontendConfigForIngress(lbc.ctx.FrontendConfigs().List(), ing)
if err != nil {
lbc.ctx.Recorder(ing.Namespace).Eventf(ing, apiv1.EventTypeWarning, events.SyncIngress, "Error: %v", err)
}
// Object in cache could be changed in-flight. Deepcopy to
// reduce race conditions.
feConfig = feConfig.DeepCopy()

staticIPName, err := annotations.StaticIPName()
if err != nil {
Expand Down
16 changes: 11 additions & 5 deletions pkg/flags/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,7 @@ var F = struct {
KubeClientBurst int

// Feature flags should be named Enablexxx.
EnableDeleteUnusedFrontends bool
EnableFrontendConfig bool
EnableNonGCPMode bool
EnableReadinessReflector bool
EnableV2FrontendNamer bool
FinalizerAdd bool // Should have been named Enablexxx.
FinalizerRemove bool // Should have been named Enablexxx.
Expand Down Expand Up @@ -145,10 +142,19 @@ var F = struct {
EnableIPV6OnlyNEG bool
MultiProjectOwnerLabelKey string

// ===============================
// DEPRECATED FLAGS
// ===============================
// DEPRECATED: ASM ConfigMap based config is no longer used and will be removed in a future release.
EnableASMConfigMapBasedConfig bool
ASMConfigMapBasedConfigCMName string
ASMConfigMapBasedConfigNamespace string
// DEPRECATED: EnableDeleteUnusedFrontends is on by default and will be removed in a future release.
EnableDeleteUnusedFrontends bool
// DEPRECATED: EnableFrontendConfig is on by default and will be removed in a future release.
EnableFrontendConfig bool
// DEPRECATED: EnableReadinessReflector is on by default and will be removed in a future release.
EnableReadinessReflector bool
}{
GCERateLimitScale: 1.0,
}
Expand Down Expand Up @@ -210,7 +216,7 @@ form namespace/name.`)
flag.StringVar(&F.DefaultSvcPortName, "default-backend-service-port", "http",
`Specify the default service's port used to serve a 404 page for the default backend. Takes
only the port's name - not its number.`)
flag.BoolVar(&F.EnableFrontendConfig, "enable-frontend-config", false,
flag.BoolVar(&F.EnableFrontendConfig, "enable-frontend-config", true,
`Optional, whether or not to enable FrontendConfig.`)
flag.Var(&F.GCERateLimit, "gce-ratelimit",
`Optional, can be used to rate limit certain GCE API calls. Example usage:
Expand Down Expand Up @@ -284,7 +290,7 @@ L7 load balancing. CSV values accepted. Example: -node-port-ranges=80,8080,400-5
flag.StringVar(&F.ASMConfigMapBasedConfigNamespace, "asm-configmap-based-config-namespace", "kube-system", "DEPRECATED: ASM Configmap based config: configmap namespace")
flag.StringVar(&F.ASMConfigMapBasedConfigCMName, "asm-configmap-based-config-cmname", "ingress-controller-asm-cm-config", "DEPRECATED: ASM Configmap based config: configmap name")
flag.BoolVar(&F.EnableNonGCPMode, "enable-non-gcp-mode", false, "Set to true when running on a non-GCP cluster.")
flag.BoolVar(&F.EnableDeleteUnusedFrontends, "enable-delete-unused-frontends", false, "Enable deleting unused gce frontend resources.")
flag.BoolVar(&F.EnableDeleteUnusedFrontends, "enable-delete-unused-frontends", true, "Enable deleting unused gce frontend resources.")
flag.BoolVar(&F.EnableV2FrontendNamer, "enable-v2-frontend-namer", false, "Enable v2 ingress frontend naming policy.")
flag.BoolVar(&F.RunIngressController, "run-ingress-controller", true, `Optional, if enabled then the ingress controller will be run.`)
flag.BoolVar(&F.RunL4Controller, "run-l4-controller", false, `Optional, whether or not to run L4 Service Controller as part of glbc. If set to true, services of Type:LoadBalancer with Internal annotation will be processed by this controller.`)
Expand Down
9 changes: 3 additions & 6 deletions pkg/loadbalancers/addresses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
"k8s.io/ingress-gce/pkg/annotations"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/flags"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/namer"
)
Expand All @@ -41,11 +40,9 @@ func (l7 *L7) checkStaticIP() (err error) {
}
if !manageStaticIP {
l7.logger.V(3).Info("Not managing user specified static IP", "ip", address)
if flags.F.EnableDeleteUnusedFrontends {
// Delete ingress controller managed static ip if exists.
if ip, ok := l7.ingress.Annotations[annotations.StaticIPKey]; ok && ip == managedStaticIPName {
return l7.deleteStaticIP()
}
// Delete ingress controller managed static ip if exists.
if ip, ok := l7.ingress.Annotations[annotations.StaticIPKey]; ok && ip == managedStaticIPName {
return l7.deleteStaticIP()
}
return nil
}
Expand Down
47 changes: 20 additions & 27 deletions pkg/loadbalancers/l7.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"fmt"
"strings"

"k8s.io/ingress-gce/pkg/flags"
"k8s.io/ingress-gce/pkg/translator"

"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
Expand Down Expand Up @@ -156,17 +155,15 @@ func (l7 *L7) edgeHop() error {
return err
}

if flags.F.EnableFrontendConfig {
if err := l7.ensureRedirectURLMap(); err != nil {
return fmt.Errorf("ensureRedirectUrlMap() = %v", err)
}
if err := l7.ensureRedirectURLMap(); err != nil {
return fmt.Errorf("ensureRedirectUrlMap() = %v", err)
}

if l7.runtimeInfo.AllowHTTP {
if err := l7.edgeHopHttp(); err != nil {
return err
}
} else if flags.F.EnableDeleteUnusedFrontends && requireDeleteFrontend(l7.ingress, namer.HTTPProtocol, l7.logger) {
} else if requireDeleteFrontend(l7.ingress, namer.HTTPProtocol, l7.logger) {
if err := l7.deleteHttp(features.VersionsFromIngress(&l7.ingress)); err != nil {
return err
}
Expand All @@ -184,7 +181,7 @@ func (l7 *L7) edgeHop() error {
if err := l7.edgeHopHttps(); err != nil {
return err
}
} else if flags.F.EnableDeleteUnusedFrontends && requireDeleteFrontend(l7.ingress, namer.HTTPSProtocol, l7.logger) {
} else if requireDeleteFrontend(l7.ingress, namer.HTTPSProtocol, l7.logger) {
if err := l7.deleteHttps(features.VersionsFromIngress(&l7.ingress)); err != nil {
return err
}
Expand Down Expand Up @@ -385,20 +382,18 @@ func (l7 *L7) Cleanup(versions *features.ResourceVersions) error {
}

// Delete RedirectUrlMap if exists
if flags.F.EnableFrontendConfig {
umName, supported := l7.namer.RedirectUrlMap()
if !supported {
// Skip deletion
return nil
}
l7.logger.V(2).Info("Deleting Redirect URL Map", "urlMapName", umName)
key, err := l7.CreateKey(umName)
if err != nil {
return err
}
if err := utils.IgnoreHTTPNotFound(composite.DeleteUrlMap(l7.cloud, key, versions.UrlMap, l7.logger)); err != nil {
return err
}
umName, supported := l7.namer.RedirectUrlMap()
if !supported {
// Skip deletion
return nil
}
l7.logger.V(2).Info("Deleting Redirect URL Map", "urlMapName", umName)
key, err = l7.CreateKey(umName)
if err != nil {
return err
}
if err := utils.IgnoreHTTPNotFound(composite.DeleteUrlMap(l7.cloud, key, versions.UrlMap, l7.logger)); err != nil {
return err
}
return nil
}
Expand Down Expand Up @@ -438,12 +433,10 @@ func (l7 *L7) getFrontendAnnotations(existing map[string]string) map[string]stri
}

// Handle Https Redirect Map
if flags.F.EnableFrontendConfig {
if l7.redirectUm != nil {
existing[annotations.RedirectUrlMapKey] = l7.redirectUm.Name
} else {
delete(existing, annotations.RedirectUrlMapKey)
}
if l7.redirectUm != nil {
existing[annotations.RedirectUrlMapKey] = l7.redirectUm.Name
} else {
delete(existing, annotations.RedirectUrlMapKey)
}

// Note that ingress IP annotation is not deleted when user disables one of http/https.
Expand Down
8 changes: 0 additions & 8 deletions pkg/loadbalancers/loadbalancers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
frontendconfigv1beta1 "k8s.io/ingress-gce/pkg/apis/frontendconfig/v1beta1"
"k8s.io/ingress-gce/pkg/composite"
"k8s.io/ingress-gce/pkg/events"
"k8s.io/ingress-gce/pkg/flags"
"k8s.io/ingress-gce/pkg/loadbalancers/features"
"k8s.io/ingress-gce/pkg/translator"
"k8s.io/ingress-gce/pkg/utils"
Expand Down Expand Up @@ -1096,9 +1095,6 @@ func TestStaticIP(t *testing.T) {

// Test setting frontendconfig Ssl policy
func TestFrontendConfigSslPolicy(t *testing.T) {
flags.F.EnableFrontendConfig = true
defer func() { flags.F.EnableFrontendConfig = false }()

j := newTestJig(t)

gceUrlMap := utils.NewGCEURLMap(klog.TODO())
Expand Down Expand Up @@ -1134,9 +1130,6 @@ func TestFrontendConfigSslPolicy(t *testing.T) {
}

func TestFrontendConfigRedirects(t *testing.T) {
flags.F.EnableFrontendConfig = true
defer func() { flags.F.EnableFrontendConfig = false }()

j := newTestJig(t)
ing := newIngress()

Expand Down Expand Up @@ -1764,7 +1757,6 @@ func TestSecretBasedToPreSharedCertUpdateWithErrors(t *testing.T) {
// on updating ingress configuration to disable http/https traffic.
func TestResourceDeletionWithProtocol(t *testing.T) {
// TODO(smatti): Add flag saver to capture current value and reset back.
flags.F.EnableDeleteUnusedFrontends = true
j := newTestJig(t)

gceUrlMap := utils.NewGCEURLMap(klog.TODO())
Expand Down
Loading