Merge pull request #6491 from Miciah/revert-revert-6478-nlb-field-and…

…-fix-OCPBUGS-2436 OCPBUGS-2436: Revert "Revert "[AWS] Add LB Type in the infrastructure cluster object via install-config yaml"" and fix OCPBUGS-2436
openshift · Oct 19, 2022 · 5a9ad4c · 5a9ad4c
2 parents 19d917c + 1a71334
commit 5a9ad4c
Show file tree

Hide file tree

Showing 7 changed files with 120 additions and 19 deletions.
diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml
@@ -1718,6 +1718,20 @@ spec:
                       the subnets. Leave the hosted zone unset to have the installer
                       create the hosted zone on your behalf.
                     type: string
+                  lbType:
+                    description: "LBType is an optional field to specify a load balancer
+                      type. \n When this field is specified, the default ingresscontroller
+                      will be created using the specified load-balancer type. \n Following
+                      are the accepted values: \n * \"Classic\": A Classic Load Balancer
+                      that makes routing decisions at either the transport layer (TCP/SSL)
+                      or the application layer (HTTP/HTTPS). See the following for
+                      additional details: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb
+                      \n * \"NLB\": A Network Load Balancer that makes routing decisions
+                      at the transport layer (TCP/SSL). See the following for additional
+                      details: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb
+                      \n If this field is not set explicitly, it defaults to \"Classic\".
+                      \ This default is subject to change over time."
+                    type: string
                   propagateUserTags:
                     description: PropagateUserTags is a flag that directs in-cluster
                       operators to include the specified user tags in the tags of

diff --git a/pkg/asset/installconfig/vsphere/mock/authmanager_generated.go b/pkg/asset/installconfig/vsphere/mock/authmanager_generated.go
diff --git a/pkg/asset/manifests/infrastructure_test.go b/pkg/asset/manifests/infrastructure_test.go
@@ -38,7 +38,8 @@ func TestGenerateInfrastructe(t *testing.T) {
 			infraBuild.forPlatform(configv1.AWSPlatformType),
 			infraBuild.withServiceEndpoint("service", "https://endpoint"),
 		),
-	}}
+	},
+	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			parents := asset.Parents{}
@@ -121,6 +122,13 @@ func (b icBuildNamespace) withServiceEndpoint(name, url string) icOption {
 	}
 }
 
+func (b icBuildNamespace) withLBType(lbType configv1.AWSLBType) icOption {
+	return func(ic *types.InstallConfig) {
+		b.forAWS()(ic)
+		ic.Platform.AWS.LBType = lbType
+	}
+}
+
 type infraOption func(*configv1.Infrastructure)
 
 type infraBuildNamespace struct{}

diff --git a/pkg/asset/manifests/ingress.go b/pkg/asset/manifests/ingress.go
@@ -14,6 +14,7 @@ import (
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/asset/installconfig"
 	"github.com/openshift/installer/pkg/types"
+	"github.com/openshift/installer/pkg/types/aws"
 )
 
 var (
@@ -115,6 +116,22 @@ func (ing *Ingress) generateClusterConfig(config *types.InstallConfig) ([]byte,
 			DefaultPlacement: defaultPlacement,
 		},
 	}
+
+	switch config.Platform.Name() {
+	case aws.Name:
+		lbType := configv1.Classic
+		if config.AWS.LBType == configv1.NLB {
+			lbType = configv1.NLB
+		}
+		obj.Spec.LoadBalancer = configv1.LoadBalancer{
+			Platform: configv1.IngressPlatformSpec{
+				AWS: &configv1.AWSIngressSpec{
+					Type: lbType,
+				},
+				Type: configv1.AWSPlatformType,
+			},
+		}
+	}
 	return yaml.Marshal(obj)
 }
 

diff --git a/pkg/asset/manifests/ingress_test.go b/pkg/asset/manifests/ingress_test.go
@@ -53,11 +53,13 @@ func installConfigFromTopologies(t *testing.T, options []icOption,
 
 func TestGenerateIngerssDefaultPlacement(t *testing.T) {
 	cases := []struct {
-		name                      string
-		installConfigBuildOptions []icOption
-		controlPlaneTopology      configv1.TopologyMode
-		infrastructureTopology    configv1.TopologyMode
-		expectedIngressPlacement  configv1.DefaultPlacement
+		name                        string
+		installConfigBuildOptions   []icOption
+		controlPlaneTopology        configv1.TopologyMode
+		infrastructureTopology      configv1.TopologyMode
+		expectedIngressPlacement    configv1.DefaultPlacement
+		expectedIngressAWSLBType    configv1.AWSLBType
+		expectedIngressPlatformType configv1.PlatformType
 	}{
 		{
 			// AWS currently uses a load balancer even on single-node, so the
@@ -66,13 +68,15 @@ func TestGenerateIngerssDefaultPlacement(t *testing.T) {
 			installConfigBuildOptions: []icOption{icBuild.forAWS()},
 			controlPlaneTopology:      configv1.SingleReplicaTopologyMode,
 			infrastructureTopology:    configv1.SingleReplicaTopologyMode,
+			expectedIngressAWSLBType:  configv1.Classic,
 			expectedIngressPlacement:  configv1.DefaultPlacementWorkers,
 		},
 		{
 			name:                      "aws multi-node with 1 day-1 worker",
 			installConfigBuildOptions: []icOption{icBuild.forAWS()},
 			controlPlaneTopology:      configv1.HighlyAvailableTopologyMode,
 			infrastructureTopology:    configv1.SingleReplicaTopologyMode,
+			expectedIngressAWSLBType:  configv1.Classic,
 			expectedIngressPlacement:  configv1.DefaultPlacementWorkers,
 		},
 		{
@@ -82,15 +86,35 @@ func TestGenerateIngerssDefaultPlacement(t *testing.T) {
 			installConfigBuildOptions: []icOption{icBuild.forAWS()},
 			controlPlaneTopology:      configv1.SingleReplicaTopologyMode,
 			infrastructureTopology:    configv1.HighlyAvailableTopologyMode,
+			expectedIngressAWSLBType:  configv1.Classic,
 			expectedIngressPlacement:  configv1.DefaultPlacementWorkers,
 		},
 		{
 			name:                      "vanilla aws",
 			installConfigBuildOptions: []icOption{icBuild.forAWS()},
 			controlPlaneTopology:      configv1.HighlyAvailableTopologyMode,
 			infrastructureTopology:    configv1.HighlyAvailableTopologyMode,
+			expectedIngressAWSLBType:  configv1.Classic,
 			expectedIngressPlacement:  configv1.DefaultPlacementWorkers,
 		},
+		{
+			name:                        "test setting of aws lb type to NLB",
+			installConfigBuildOptions:   []icOption{icBuild.withLBType(configv1.NLB)},
+			controlPlaneTopology:        configv1.HighlyAvailableTopologyMode,
+			infrastructureTopology:      configv1.HighlyAvailableTopologyMode,
+			expectedIngressPlacement:    configv1.DefaultPlacementWorkers,
+			expectedIngressAWSLBType:    configv1.NLB,
+			expectedIngressPlatformType: configv1.AWSPlatformType,
+		},
+		{
+			name:                        "test setting of aws lb type to Classic",
+			installConfigBuildOptions:   []icOption{icBuild.withLBType(configv1.Classic)},
+			controlPlaneTopology:        configv1.HighlyAvailableTopologyMode,
+			infrastructureTopology:      configv1.HighlyAvailableTopologyMode,
+			expectedIngressPlacement:    configv1.DefaultPlacementWorkers,
+			expectedIngressAWSLBType:    configv1.Classic,
+			expectedIngressPlatformType: configv1.AWSPlatformType,
+		},
 		{
 			name:                      "none-platform single node with 0 or 1 day-1 workers",
 			installConfigBuildOptions: []icOption{icBuild.forNone()},
@@ -154,6 +178,10 @@ func TestGenerateIngerssDefaultPlacement(t *testing.T) {
 				return
 			}
 			assert.Equal(t, tc.expectedIngressPlacement, actualIngress.Status.DefaultPlacement)
+			if len(tc.expectedIngressPlatformType) != 0 {
+				assert.Equal(t, tc.expectedIngressAWSLBType, actualIngress.Spec.LoadBalancer.Platform.AWS.Type)
+				assert.Equal(t, tc.expectedIngressPlatformType, actualIngress.Spec.LoadBalancer.Platform.Type)
+			}
 		})
 	}
 }
diff --git a/pkg/explain/printer_test.go b/pkg/explain/printer_test.go
@@ -147,6 +147,14 @@ func Test_PrintFields(t *testing.T) {
     hostedZone <string>
       HostedZone is the ID of an existing hosted zone into which to add DNS records for the cluster's internal API. An existing hosted zone can only be used when also using existing subnets. The hosted zone must be associated with the VPC containing the subnets. Leave the hosted zone unset to have the installer create the hosted zone on your behalf.
 
+    lbType <string>
+      LBType is an optional field to specify a load balancer type. 
+ When this field is specified, the default ingresscontroller will be created using the specified load-balancer type. 
+ Following are the accepted values: 
+ * "Classic": A Classic Load Balancer that makes routing decisions at either the transport layer (TCP/SSL) or the application layer (HTTP/HTTPS). See the following for additional details: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb 
+ * "NLB": A Network Load Balancer that makes routing decisions at the transport layer (TCP/SSL). See the following for additional details: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb 
+ If this field is not set explicitly, it defaults to "Classic".  This default is subject to change over time.
+
     propagateUserTags <boolean>
       PropagateUserTags is a flag that directs in-cluster operators to include the specified user tags in the tags of the AWS resources that the operators create.
 

diff --git a/pkg/types/aws/platform.go b/pkg/types/aws/platform.go
@@ -1,6 +1,9 @@
 package aws
 
-import "github.com/aws/aws-sdk-go/aws/endpoints"
+import (
+	"github.com/aws/aws-sdk-go/aws/endpoints"
+	configv1 "github.com/openshift/api/config/v1"
+)
 
 // Platform stores all the global configuration that all machinesets
 // use.
@@ -59,6 +62,28 @@ type Platform struct {
 	// AWS resources that the operators create.
 	// +optional
 	PropagateUserTag bool `json:"propagateUserTags,omitempty"`
+
+	// LBType is an optional field to specify a load balancer type.
+	//
+	// When this field is specified, the default ingresscontroller will be
+	// created using the specified load-balancer type.
+	//
+	// Following are the accepted values:
+	//
+	// * "Classic": A Classic Load Balancer that makes routing decisions at
+	// either the transport layer (TCP/SSL) or the application layer
+	// (HTTP/HTTPS). See the following for additional details:
+	// https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#clb
+	//
+	// * "NLB": A Network Load Balancer that makes routing decisions at the
+	// transport layer (TCP/SSL). See the following for additional details:
+	// https://docs.aws.amazon.com/AmazonECS/latest/developerguide/load-balancer-types.html#nlb
+	//
+	// If this field is not set explicitly, it defaults to "Classic".  This
+	// default is subject to change over time.
+	//
+	// +optional
+	LBType configv1.AWSLBType `json:"lbType,omitempty"`
 }
 
 // ServiceEndpoint store the configuration for services to