Merge branch 'kubernetes-sigs:main' into main

shraddhabang · Jan 31, 2024 · c58f04d · c58f04d
2 parents c2ae6ba + a95e471
commit c58f04d
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 3 deletions.
diff --git a/helm/aws-load-balancer-controller/README.md b/helm/aws-load-balancer-controller/README.md
@@ -246,6 +246,7 @@ The default values set by the application itself can be confirmed [here](https:/
 | `extraVolumes`                                 | Extra volumes for the pod                                                                                                                                                                                              | `[]`                                              |
 | `defaultTags`                                  | Default tags to apply to all AWS resources managed by this controller                                                                                                                                                  | `{}`                                              |
 | `replicaCount`                                 | Number of controller pods to run, only one will be active due to leader election                                                                                                                                       | `2`                                               |
+| `revisionHistoryLimit`                         | Number of revisions to keep                                                                                                                                                                                            | `10`                                              |
 | `podDisruptionBudget`                          | Limit the disruption for controller pods. Require at least 2 controller replicas and 3 worker nodes                                                                                                                    | `{}`                                              |
 | `updateStrategy`                               | Defines the update strategy for the deployment                                                                                                                                                                         | `{}`                                              |
 | `enableCertManager`                            | If enabled, cert-manager issues the webhook certificates instead of the helm template, requires cert-manager and it's CRDs to be installed                                                                             | `false`                                           |

diff --git a/helm/aws-load-balancer-controller/templates/deployment.yaml b/helm/aws-load-balancer-controller/templates/deployment.yaml
@@ -11,6 +11,7 @@ metadata:
     {{- include "aws-load-balancer-controller.labels" . | nindent 4 }}
 spec:
   replicas: {{ .Values.replicaCount }}
+  revisionHistoryLimit: {{ .Values.revisionHistoryLimit }}
   selector:
     matchLabels:
       {{- include "aws-load-balancer-controller.selectorLabels" . | nindent 6 }}

diff --git a/helm/aws-load-balancer-controller/values.yaml b/helm/aws-load-balancer-controller/values.yaml
@@ -4,6 +4,8 @@
 
 replicaCount: 2
 
+revisionHistoryLimit: 10
+
 image:
   repository: public.ecr.aws/eks/aws-load-balancer-controller
   tag: v2.6.1

diff --git a/pkg/networking/subnet_resolver.go b/pkg/networking/subnet_resolver.go
@@ -198,7 +198,10 @@ func (r *defaultSubnetsResolver) ResolveViaSelector(ctx context.Context, selecto
 				},
 			},
 		}
+
+		targetTagKeys := []string{}
 		for key, values := range selector.Tags {
+			targetTagKeys = append(targetTagKeys, key)
 			req.Filters = append(req.Filters, &ec2sdk.Filter{
 				Name:   awssdk.String("tag:" + key),
 				Values: awssdk.StringSlice(values),
@@ -209,7 +212,8 @@ func (r *defaultSubnetsResolver) ResolveViaSelector(ctx context.Context, selecto
 		if err != nil {
 			return nil, err
 		}
-		explanation = fmt.Sprintf("%d match VPC and tags", len(allSubnets))
+		explanation = fmt.Sprintf("%d match VPC and tags: %s", len(allSubnets), targetTagKeys)
+
 		var subnets []*ec2sdk.Subnet
 		taggedOtherCluster := 0
 		for _, subnet := range allSubnets {

diff --git a/pkg/networking/subnet_resolver_test.go b/pkg/networking/subnet_resolver_test.go
@@ -203,7 +203,7 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
 			},
 		},
 		{
-			name: "ALB with no matching subnets",
+			name: "ALB with no matching subnets (internal)",
 			fields: fields{
 				vpcID:       "vpc-1",
 				clusterName: "kube-cluster",
@@ -231,7 +231,38 @@ func Test_defaultSubnetsResolver_ResolveViaDiscovery(t *testing.T) {
 					WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternal),
 				},
 			},
-			wantErr: errors.New("unable to resolve at least one subnet (0 match VPC and tags)"),
+			wantErr: errors.New("unable to resolve at least one subnet (0 match VPC and tags: [kubernetes.io/role/internal-elb])"),
+		},
+		{
+			name: "ALB with no matching subnets (internet-facing)",
+			fields: fields{
+				vpcID:       "vpc-1",
+				clusterName: "kube-cluster",
+				describeSubnetsAsListCalls: []describeSubnetsAsListCall{
+					{
+						input: &ec2sdk.DescribeSubnetsInput{
+							Filters: []*ec2sdk.Filter{
+								{
+									Name:   awssdk.String("vpc-id"),
+									Values: awssdk.StringSlice([]string{"vpc-1"}),
+								},
+								{
+									Name:   awssdk.String("tag:kubernetes.io/role/elb"),
+									Values: awssdk.StringSlice([]string{"", "1"}),
+								},
+							},
+						},
+						output: nil,
+					},
+				},
+			},
+			args: args{
+				opts: []SubnetsResolveOption{
+					WithSubnetsResolveLBType(elbv2model.LoadBalancerTypeApplication),
+					WithSubnetsResolveLBScheme(elbv2model.LoadBalancerSchemeInternetFacing),
+				},
+			},
+			wantErr: errors.New("unable to resolve at least one subnet (0 match VPC and tags: [kubernetes.io/role/elb])"),
 		},
 		{
 			name: "NLB with one matching subnet",