Skip to content

OCPBUGS-55673: Remove IngressControllerLBSubnetsAWS featuregate #1242

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

Merged
openshift-merge-bot merged 1 commit into from
Sep 18, 2025
Merged

OCPBUGS-55673: Remove IngressControllerLBSubnetsAWS featuregate #1242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion pkg/operator/controller/ingress/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ type Config struct {
Namespace string
IngressControllerImage string
RouteExternalCertificateEnabled bool
IngressControllerLBSubnetsAWSEnabled bool
IngressControllerEIPAllocationsAWSEnabled bool
IngressControllerDCMEnabled bool
}
Expand Down
28 changes: 12 additions & 16 deletions pkg/operator/controller/ingress/load_balancer_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ var (
// Always returns the current LB service if one exists (whether it already
// existed or was created during the course of the function).
func (r *reconciler) ensureLoadBalancerService(ci *operatorv1.IngressController, deploymentRef metav1.OwnerReference, platformStatus *configv1.PlatformStatus) (bool, *corev1.Service, error) {
wantLBS, desiredLBService, err := desiredLoadBalancerService(ci, deploymentRef, platformStatus, r.config.IngressControllerLBSubnetsAWSEnabled, r.config.IngressControllerEIPAllocationsAWSEnabled)
wantLBS, desiredLBService, err := desiredLoadBalancerService(ci, deploymentRef, platformStatus, r.config.IngressControllerEIPAllocationsAWSEnabled)
if err != nil {
return false, nil, err
}
Expand Down Expand Up @@ -336,7 +336,7 @@ func isServiceOwnedByIngressController(service *corev1.Service, ic *operatorv1.I
// ingresscontroller, or nil if an LB service isn't desired. An LB service is
// desired if the high availability type is Cloud. An LB service will declare an
// owner reference to the given deployment.
func desiredLoadBalancerService(ci *operatorv1.IngressController, deploymentRef metav1.OwnerReference, platform *configv1.PlatformStatus, subnetsAWSEnabled bool, eipAllocationsAWSEnabled bool) (bool, *corev1.Service, error) {
func desiredLoadBalancerService(ci *operatorv1.IngressController, deploymentRef metav1.OwnerReference, platform *configv1.PlatformStatus, eipAllocationsAWSEnabled bool) (bool, *corev1.Service, error) {
if ci.Status.EndpointPublishingStrategy.Type != operatorv1.LoadBalancerServiceStrategyType {
return false, nil, nil
}
Expand Down Expand Up @@ -408,11 +408,9 @@ func desiredLoadBalancerService(ci *operatorv1.IngressController, deploymentRef
// See <https://bugzilla.redhat.com/show_bug.cgi?id=1908758>.
service.Annotations[awsLBHealthCheckIntervalAnnotation] = awsLBHealthCheckIntervalNLB

if subnetsAWSEnabled {
nlbParams := getAWSNetworkLoadBalancerParametersInSpec(ci)
if nlbParams != nil && awsSubnetsExist(nlbParams.Subnets) {
service.Annotations[awsLBSubnetsAnnotation] = JoinAWSSubnets(nlbParams.Subnets, ",")
}
nlbParams := getAWSNetworkLoadBalancerParametersInSpec(ci)
if nlbParams != nil && awsSubnetsExist(nlbParams.Subnets) {
service.Annotations[awsLBSubnetsAnnotation] = JoinAWSSubnets(nlbParams.Subnets, ",")
}

if eipAllocationsAWSEnabled {
Expand All @@ -429,11 +427,9 @@ func desiredLoadBalancerService(ci *operatorv1.IngressController, deploymentRef
}
}

if subnetsAWSEnabled {
clbParams := getAWSClassicLoadBalancerParametersInSpec(ci)
if clbParams != nil && awsSubnetsExist(clbParams.Subnets) {
service.Annotations[awsLBSubnetsAnnotation] = JoinAWSSubnets(clbParams.Subnets, ",")
}
clbParams := getAWSClassicLoadBalancerParametersInSpec(ci)
if clbParams != nil && awsSubnetsExist(clbParams.Subnets) {
service.Annotations[awsLBSubnetsAnnotation] = JoinAWSSubnets(clbParams.Subnets, ",")
}
}
}
Expand Down Expand Up @@ -760,8 +756,8 @@ func IsServiceInternal(service *corev1.Service) bool {
// return value is nil. Otherwise, if something or someone else has modified
// the service, then the return value is a non-nil error indicating that the
// modification must be reverted before upgrading is allowed.
func loadBalancerServiceIsUpgradeable(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference, current *corev1.Service, platform *configv1.PlatformStatus, subnetsAWSEnabled bool, eipAllocationsAWSEnabled bool) error {
want, desired, err := desiredLoadBalancerService(ic, deploymentRef, platform, subnetsAWSEnabled, eipAllocationsAWSEnabled)
func loadBalancerServiceIsUpgradeable(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference, current *corev1.Service, platform *configv1.PlatformStatus, eipAllocationsAWSEnabled bool) error {
want, desired, err := desiredLoadBalancerService(ic, deploymentRef, platform, eipAllocationsAWSEnabled)
if err != nil {
return err
}
Expand All @@ -785,7 +781,7 @@ func loadBalancerServiceIsUpgradeable(ic *operatorv1.IngressController, deployme

// loadBalancerServiceIsProgressing returns an error value indicating if the
// load balancer service is in progressing status.
func loadBalancerServiceIsProgressing(ic *operatorv1.IngressController, service *corev1.Service, platform *configv1.PlatformStatus, subnetsAWSEnabled bool, eipAllocationsAWSEnabled bool) error {
func loadBalancerServiceIsProgressing(ic *operatorv1.IngressController, service *corev1.Service, platform *configv1.PlatformStatus, eipAllocationsAWSEnabled bool) error {
var errs []error
wantScope := ic.Status.EndpointPublishingStrategy.LoadBalancer.Scope
haveScope := operatorv1.ExternalLoadBalancer
Expand All @@ -800,7 +796,7 @@ func loadBalancerServiceIsProgressing(ic *operatorv1.IngressController, service
errs = append(errs, err)
}

if platform.Type == configv1.AWSPlatformType && subnetsAWSEnabled {
if platform.Type == configv1.AWSPlatformType {
var (
wantSubnets, haveSubnets *operatorv1.AWSSubnets
paramsFieldName string
Expand Down
44 changes: 3 additions & 41 deletions pkg/operator/controller/ingress/load_balancer_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ func Test_desiredLoadBalancerService(t *testing.T) {
expectedServiceAnnotations map[string]annotationExpectation
expectedExternalTrafficPolicy corev1.ServiceExternalTrafficPolicy
platformStatus *configv1.PlatformStatus
subnetsAWSFeatureEnabled bool
eipAllocationsAWSFeatureEnabled bool
expectedFloatingIP string
}{
Expand Down Expand Up @@ -305,39 +304,6 @@ func Test_desiredLoadBalancerService(t *testing.T) {
awsLBSubnetsAnnotation: {false, ""},
},
},
{
description: "network load balancer with subnets for aws platform, but feature gate is disabled",
platformStatus: platformStatus(configv1.AWSPlatformType),
strategySpec: awsLbWithSubnets(
operatorv1.AWSNetworkLoadBalancer,
&operatorv1.AWSSubnets{
IDs: []operatorv1.AWSSubnetID{"subnet-00000000000000001", "subnet-00000000000000002"},
Names: []operatorv1.AWSSubnetName{"subnetA", "subnetB"},
},
),
strategyStatus: awsLbWithSubnets(
operatorv1.AWSNetworkLoadBalancer,
&operatorv1.AWSSubnets{
IDs: []operatorv1.AWSSubnetID{"subnet-00000000000000001", "subnet-00000000000000002"},
Names: []operatorv1.AWSSubnetName{"subnetA", "subnetB"},
},
),
proxyNeeded: false,
expectService: true,
subnetsAWSFeatureEnabled: false,
expectedExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal,
expectedServiceAnnotations: map[string]annotationExpectation{
awsInternalLBAnnotation: {false, ""},
awsLBAdditionalResourceTags: {false, ""},
awsLBHealthCheckHealthyThresholdAnnotation: {true, awsLBHealthCheckHealthyThresholdDefault},
awsLBHealthCheckIntervalAnnotation: {true, awsLBHealthCheckIntervalNLB},
awsLBHealthCheckTimeoutAnnotation: {true, awsLBHealthCheckTimeoutDefault},
awsLBHealthCheckUnhealthyThresholdAnnotation: {true, awsLBHealthCheckUnhealthyThresholdDefault},
awsLBProxyProtocolAnnotation: {false, ""},
AWSLBTypeAnnotation: {true, AWSNLBAnnotation},
localWithFallbackAnnotation: {true, ""},
},
},
{
description: "network load balancer with subnets for aws platform using both IDs and names",
platformStatus: platformStatus(configv1.AWSPlatformType),
Expand All @@ -357,7 +323,6 @@ func Test_desiredLoadBalancerService(t *testing.T) {
),
proxyNeeded: false,
expectService: true,
subnetsAWSFeatureEnabled: true,
expectedExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal,
expectedServiceAnnotations: map[string]annotationExpectation{
awsInternalLBAnnotation: {false, ""},
Expand Down Expand Up @@ -389,7 +354,6 @@ func Test_desiredLoadBalancerService(t *testing.T) {
),
proxyNeeded: false,
expectService: true,
subnetsAWSFeatureEnabled: true,
expectedExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal,
expectedServiceAnnotations: map[string]annotationExpectation{
awsInternalLBAnnotation: {false, ""},
Expand Down Expand Up @@ -421,7 +385,6 @@ func Test_desiredLoadBalancerService(t *testing.T) {
),
proxyNeeded: false,
expectService: true,
subnetsAWSFeatureEnabled: true,
expectedExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal,
expectedServiceAnnotations: map[string]annotationExpectation{
awsInternalLBAnnotation: {false, ""},
Expand Down Expand Up @@ -455,7 +418,6 @@ func Test_desiredLoadBalancerService(t *testing.T) {
),
proxyNeeded: true,
expectService: true,
subnetsAWSFeatureEnabled: true,
expectedExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyLocal,
expectedServiceAnnotations: map[string]annotationExpectation{
awsInternalLBAnnotation: {false, ""},
Expand Down Expand Up @@ -753,7 +715,7 @@ func Test_desiredLoadBalancerService(t *testing.T) {
t.Errorf("expected IsProxyProtocolNeeded to return %v, got %v", tc.proxyNeeded, proxyNeeded)
}

haveSvc, svc, err := desiredLoadBalancerService(ic, deploymentRef, infraConfig.Status.PlatformStatus, tc.subnetsAWSFeatureEnabled, tc.eipAllocationsAWSFeatureEnabled)
haveSvc, svc, err := desiredLoadBalancerService(ic, deploymentRef, infraConfig.Status.PlatformStatus, tc.eipAllocationsAWSFeatureEnabled)
switch {
case err != nil:
t.Error(err)
Expand Down Expand Up @@ -938,7 +900,7 @@ func TestDesiredLoadBalancerServiceAWSIdleTimeout(t *testing.T) {
},
},
}
haveSvc, svc, err := desiredLoadBalancerService(ic, deploymentRef, infraConfig.Status.PlatformStatus, true, true)
haveSvc, svc, err := desiredLoadBalancerService(ic, deploymentRef, infraConfig.Status.PlatformStatus, true)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -1552,7 +1514,7 @@ func TestUpdateLoadBalancerServiceSourceRanges(t *testing.T) {
},
},
}
wantSvc, desired, err := desiredLoadBalancerService(ic, deploymentRef, infraConfig.Status.PlatformStatus, true, true)
wantSvc, desired, err := desiredLoadBalancerService(ic, deploymentRef, infraConfig.Status.PlatformStatus, true)
if err != nil {
t.Fatal(err)
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/operator/controller/ingress/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (r *reconciler) syncIngressControllerStatus(ic *operatorv1.IngressControlle
if updated.Status.EndpointPublishingStrategy != nil && updated.Status.EndpointPublishingStrategy.LoadBalancer != nil {
updated.Status.EndpointPublishingStrategy.LoadBalancer.AllowedSourceRanges = computeAllowedSourceRanges(service)
}
if platformStatus.Type == configv1.AWSPlatformType && r.config.IngressControllerLBSubnetsAWSEnabled {
if platformStatus.Type == configv1.AWSPlatformType {
updateIngressControllerAWSSubnetStatus(updated, service)
}
if platformStatus.Type == configv1.AWSPlatformType && r.config.IngressControllerEIPAllocationsAWSEnabled {
Expand All @@ -89,14 +89,14 @@ func (r *reconciler) syncIngressControllerStatus(ic *operatorv1.IngressControlle
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeDeploymentReplicasAllAvailableCondition(deployment))
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeDeploymentRollingOutCondition(deployment))
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeLoadBalancerStatus(ic, service, operandEvents)...)
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeLoadBalancerProgressingStatus(updated, service, platformStatus, r.config.IngressControllerLBSubnetsAWSEnabled, r.config.IngressControllerEIPAllocationsAWSEnabled))
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeLoadBalancerProgressingStatus(updated, service, platformStatus, r.config.IngressControllerEIPAllocationsAWSEnabled))
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeDNSStatus(ic, wildcardRecord, platformStatus, dnsConfig)...)
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeIngressAvailableCondition(updated.Status.Conditions))
degradedCondition, err := computeIngressDegradedCondition(updated.Status.Conditions, updated.Name)
errs = append(errs, err)
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeIngressProgressingCondition(updated.Status.Conditions))
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, degradedCondition)
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeIngressUpgradeableCondition(ic, deploymentRef, service, platformStatus, secret, r.config.IngressControllerLBSubnetsAWSEnabled, r.config.IngressControllerEIPAllocationsAWSEnabled))
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeIngressUpgradeableCondition(ic, deploymentRef, service, platformStatus, secret, r.config.IngressControllerEIPAllocationsAWSEnabled))
updated.Status.Conditions = MergeConditions(updated.Status.Conditions, computeIngressEvaluationConditionsDetectedCondition(ic, service))

updated.Status.Conditions = PruneConditions(updated.Status.Conditions)
Expand Down Expand Up @@ -646,13 +646,13 @@ func computeIngressDegradedCondition(conditions []operatorv1.OperatorCondition,
}

// computeIngressUpgradeableCondition computes the IngressController's "Upgradeable" status condition.
func computeIngressUpgradeableCondition(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference, service *corev1.Service, platform *configv1.PlatformStatus, secret *corev1.Secret, subnetsAWSEnabled bool, eipAllocationsAWSEnabled bool) operatorv1.OperatorCondition {
func computeIngressUpgradeableCondition(ic *operatorv1.IngressController, deploymentRef metav1.OwnerReference, service *corev1.Service, platform *configv1.PlatformStatus, secret *corev1.Secret, eipAllocationsAWSEnabled bool) operatorv1.OperatorCondition {
var errs []error

errs = append(errs, checkDefaultCertificate(secret, "*."+ic.Status.Domain))

if service != nil {
errs = append(errs, loadBalancerServiceIsUpgradeable(ic, deploymentRef, service, platform, subnetsAWSEnabled, eipAllocationsAWSEnabled))
errs = append(errs, loadBalancerServiceIsUpgradeable(ic, deploymentRef, service, platform, eipAllocationsAWSEnabled))
}

if err := kerrors.NewAggregate(errs); err != nil {
Expand Down Expand Up @@ -912,7 +912,7 @@ func computeLoadBalancerStatus(ic *operatorv1.IngressController, service *corev1
// computeLoadBalancerProgressingStatus returns the LoadBalancerProgressing
// conditions for the given ingress controller. These conditions subsequently determine
// the ingress controller's Progressing status.
func computeLoadBalancerProgressingStatus(ic *operatorv1.IngressController, service *corev1.Service, platform *configv1.PlatformStatus, subnetsAWSEnabled bool, eipAllocationsAWSEnabled bool) operatorv1.OperatorCondition {
func computeLoadBalancerProgressingStatus(ic *operatorv1.IngressController, service *corev1.Service, platform *configv1.PlatformStatus, eipAllocationsAWSEnabled bool) operatorv1.OperatorCondition {
// Compute the IngressControllerLoadBalancerProgressingConditionType condition for the LoadBalancer
if ic.Status.EndpointPublishingStrategy.Type == operatorv1.LoadBalancerServiceStrategyType {
switch {
Expand All @@ -931,7 +931,7 @@ func computeLoadBalancerProgressingStatus(ic *operatorv1.IngressController, serv
Message: "LoadBalancer Service not created.",
}
default:
if err := loadBalancerServiceIsProgressing(ic, service, platform, subnetsAWSEnabled, eipAllocationsAWSEnabled); err != nil {
if err := loadBalancerServiceIsProgressing(ic, service, platform, eipAllocationsAWSEnabled); err != nil {
return operatorv1.OperatorCondition{
Type: IngressControllerLoadBalancerProgressingConditionType,
Status: operatorv1.ConditionTrue,
Expand Down
Loading