Skip to content

Commit

Permalink
OCPBUGS-43692: Fix LB Type Defaulting with ProviderParameters Set
Browse files Browse the repository at this point in the history
Most of the logic in `setDefaultProviderParameters` didn't execute if
`alreadyAdmitted` is true. However, in the case `alreadyAdmitted` was
true and the IngressController's spec had `providerParameters` set,
`setDefaultProviderParameters` incorrectly forced the LB type
to `Classic`.

There is no clear advantage to running
`setDefaultProviderParameters` when `alreadyAdmitted` is true. This
fix removes `alreadyAdmitted` as a parameter from
`setDefaultProviderParameters` and instead prevents it from being
invoked entirely when `alreadyAdmitted` is true.
  • Loading branch information
gcs278 committed Oct 22, 2024
1 parent 8bf1b36 commit 8935f7a
Showing 1 changed file with 7 additions and 5 deletions.
12 changes: 7 additions & 5 deletions pkg/operator/controller/ingress/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,10 @@ func setDefaultPublishingStrategy(ic *operatorv1.IngressController, platformStat
}
}

// Set provider parameters based on the cluster ingress config.
setDefaultProviderParameters(effectiveStrategy.LoadBalancer, ingressConfig, alreadyAdmitted)
// Set provider parameters based on the cluster ingress config if not admitted already.
if !alreadyAdmitted {
setDefaultProviderParameters(effectiveStrategy.LoadBalancer, ingressConfig)
}

case operatorv1.NodePortServiceStrategyType:
if effectiveStrategy.NodePort == nil {
Expand Down Expand Up @@ -681,12 +683,12 @@ func setDefaultPublishingStrategy(ic *operatorv1.IngressController, platformStat
// setDefaultProviderParameters mutates the given LoadBalancerStrategy by
// defaulting its ProviderParameters field based on the defaults in the provided
// ingress config object.
func setDefaultProviderParameters(lbs *operatorv1.LoadBalancerStrategy, ingressConfig *configv1.Ingress, alreadyAdmitted bool) {
func setDefaultProviderParameters(lbs *operatorv1.LoadBalancerStrategy, ingressConfig *configv1.Ingress) {
var provider operatorv1.LoadBalancerProviderType
if lbs.ProviderParameters != nil {
provider = lbs.ProviderParameters.Type
}
if len(provider) == 0 && !alreadyAdmitted {
if len(provider) == 0 {
// Infer the LB type from the cluster ingress config, but only
// if the ingresscontroller isn't already admitted.
switch ingressConfig.Spec.LoadBalancer.Platform.Type {
Expand All @@ -701,7 +703,7 @@ func setDefaultProviderParameters(lbs *operatorv1.LoadBalancerStrategy, ingressC
}
lbs.ProviderParameters.Type = provider
defaultLBType := operatorv1.AWSClassicLoadBalancer
if p := ingressConfig.Spec.LoadBalancer.Platform; !alreadyAdmitted && p.Type == configv1.AWSPlatformType && p.AWS != nil {
if p := ingressConfig.Spec.LoadBalancer.Platform; p.Type == configv1.AWSPlatformType && p.AWS != nil {
if p.AWS.Type == configv1.NLB {
defaultLBType = operatorv1.AWSNetworkLoadBalancer
}
Expand Down

0 comments on commit 8935f7a

Please sign in to comment.